Re: [PATCH] 2.0.35 insmod change appraisal request

B. James Phillippe (bryan@terran.org)
Wed, 20 Jan 1999 13:49:18 -0800 (PST)


On Wed, 20 Jan 1999, Björn Ekwall wrote:

> Hi!

Hello Björn, you're one of the folks I was hoping to speak to about this.
:)

> Your module problem caught my eye...
> The patch is not correct, since you actually miss the last longword of
> the module image. That is: if the value of codesize is correct...
> Also: the code for determining the actual number of pages is correct as is.

I will respond to each of your points to the best of my ability. First,
the easy part: the second hunk of the patch (the npages part) was a
cosmetic change which I believed made the code easier to read. If it is
not correct, it can easily be dismissed from the discussion.

However, the first change, the one which omits the final longword of the
module code, was an attempt to fix a problem I mentioned in a previous
post. I will summarize below:

> So, why are you having a problem?

The problem is that when I insmod a module that reports a codesize of 8192
(under modutils-2.1.85+), it causes a "unable to handle paging request"
Oops on 2.0.3X. 8192 is an even multiple of PAGE_SIZE on the x86, which I
believe is the root of the problem. If I omit the last longword, the
problem goes away.

My thought process was this:

1.) insmod copies the module code into memory and tells the kernel the
starting address and size. If the kernel is "old" (< 2.1.18), insmod
passes the starting address + sizeof(longword) to compensate for
mod_use_count. However, it does not also fudge the size of the module by 1
longword less.

2.) The kernel figures out how many pages it needs to store the module,
then vmallocs that much memory.

3.) The kernel starts to copy the object code from user space to the
freshly allocated kernel memory, but at an offset of 1 longword. If the
insmod has offset it's start address, everything lines up. HOWEVER, if
insmod has not adjusted the size downward, the kernel will copy too much
data; 1 extra longword.

4.) If the end of the module lands on a page boundary, or just a few bytes
before the end, the kernel will try to load the next page for those last
few bytes of the trailing longword. But there is no last page, so you get
an Oops. Now, interesting to note is that the kernel also does not do a
verify_area before copying, so the problem in insmod is allowed to tank the
kernel.

To that effect, assuming my logic is not flawed, here are new patches to
2.0.35 and insmod-2.1.121 which should fix the problem properly. PLEASE
continue to comment on this and let me know if I'm talking nonsense.

==== Patch to 2.0.25 kernel ====
--- kernel/module.c.orig Tue Jan 19 13:41:30 1999
+++ kernel/module.c Wed Jan 20 13:44:30 1999
@@ -166,6 +166,8 @@
}
if ((codesize + sizeof (long) + PAGE_SIZE - 1) / PAGE_SIZE >
mp->size)
return -EINVAL;
+ if ((error = verify_area(VERIFY_READ, code, codesize)))
+ return error;
memcpy_fromfs((char *)mp->addr + sizeof (long), code, codesize);
memset((char *)mp->addr + sizeof (long) + codesize, 0,
mp->size * PAGE_SIZE - (codesize + sizeof (long)));

==== Patch to insmod-2.1.121 ====
--- insmod.c 1998/11/13 00:26:57 1.1.1.1
+++ insmod.c 1999/01/20 21:32:03
@@ -726,7 +726,8 @@
mod_use_count. However the old module kernel support assume that
it is receiving something which does not contain mod_use_count. */
ret = old_sys_init_module(m_name, image+sizeof(long),
- m_size | (flag_autoclean ? OLD_MOD_AUTOCLEAN : 0),
+ (m_size-sizeof(long)) |
+ (flag_autoclean ? OLD_MOD_AUTOCLEAN : 0),
&routines, symtab);
if (ret)
error("init_module: %m");

With these patches applied, all my modules seem to work okay.

thanks,
-bp

--
B. James Phillippe	. bryan@terran.org
Linux Engineer/Admin	. http://www.terran.org/~bryan
Member since 1.1.59	. finger:bryan@earth.terran.org

- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.rutgers.edu Please read the FAQ at http://www.tux.org/lkml/