Skip to contentSkip to conversation

Modutils 2.3.14 / Kernel 2.4.0-test11 incompatibility

DISCUSSION Russell King 2 participants 3 messages Nov 22, 2000 – Nov 23, 2000

lore
Share & tools
lore
#0 lore raw

Hi,

There appears to be an incompatibility between modutils 2.3.14 and kernel 2.4.0-test11.

This occurs because modutils knows only of an 84-byte struct module, but the kernel knows about a 96-byte struct module.

Unfortunately, the kernel "forgets" about the 12 bytes, which are part of the loading module text because in sys_init_module():

        if ((error = get_user(mod_user_size, &mod_user->size_of_struct)) != 0)
                goto err1;
/* mod_user_size == 84 */
	/* ... */
        error = copy_from_user(mod, mod_user, mod_user_size);
/* we copy byte 0 to byte 83 of struct module */
	/* ... */
        if (copy_from_user(mod+1, mod_user+1, mod->size-sizeof(*mod))) {

/* we copy byte 96 to <end of module> */

And guess what? We missed bytes 84 to 95! Oh dear, the result is one of these:

Unable to handle kernel NULL pointer dereference at virtual address 00000000
pgd = c1e90000
*pgd = 01e94001, *pmd = 01e94001, *pte = 0000308b, *ppte = 0000300a
Internal error: Oops: 0
CPU: 0
pc : [<c280007c>]    lr : [<c00251f8>]

sp : c1e97f08 ip : c1e97ec0 fp : c1e97f14
r10: c1e96000 r9 : 00000004 r8 : ffffffea
r7 : 02029220 r6 : c1e37000 r5 : c2800000 r4 : 00000000 r3 : ef9f0000 r2 : 00000000 r1 : 00000000 r0 : 000001db
Flags: nZCv IRQs on FIQs on Mode SVC_32 Segment user
Control: 1E9117D Table: 01E9117D DAC: 00000015 Process insmod (pid: 9, stackpage=c1e97000)

Stack:
c1e97ee0:                                                        c00251f8 c280007c
c1e97f00: 60000013 ffffffff c1e97fac c1e97f18  c0026194 c280006c c1e37000 c1e38000
c1e97f20: c01f3680 00000060 c011d48c c2800060  00005e00 00000000 00000000 00000000
c1e97f40: 00000000 00000000 00000000 00000000  00000000 00000000 00000000 00000000
c1e97f60: 00000000 00000000 00000000 00000000  00000000 00000000 00000000 00000000
c1e97f80: 00000000 bfffec64 02021928 60000010  c2800000 c00169e4 00000080 0201bbe8
c1e97fa0: 00000000 c1e97fb0 c0016860 c0025acc  bfffec64 c001bb54 0201bbe8 02029220
c1e97fc0: 00000000 00000038 bfffec64 02021928  02019f10 c2800000 00005e00 0201bbe8
c1e97fe0: 0201bbe8 bffffe60 400e4c90 bfffec28  020031e8 400e4c9c 60000010 0201bbe8
Backtrace:
Function entered at [<c2800060>] from [<c0026194>]
Function entered at [<c0025ac0>] from [<c0016860>]
Code: e51f2024 e5923000 (e5813000) e3a00000 e51f3030

because both r2 and r1 are loaded from the first two words of module text which forgot to be copied. Therefore, please reverse the following change in 2.4.0-test11 because it is NOT a security problem but a much needed subtlety:

- error = copy_from_user(mod, mod_user, sizeof(struct module));

+       error = copy_from_user(mod, mod_user, mod_user_size);
        if (error) {

Better still, place a comment before it describing why it is so!
_____

  |_____| ------------------------------------------------- ---+---+-
  |   |         Russell King        [email protected]      --- ---
  | | | | http://www.arm.linux.org.uk/personal/aboutme.html   /  /  |
  | +-+-+                                                     --- -+-
  /   |               THE developer of ARM Linux              |+| /|\
 /  | | |                                                     ---  |
    +-+-+ -------------------------------------------------  /\\\  |

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

#1 lore raw
Quoted 1 line · Russell King
       if (copy_from_user(mod+1, mod_user+1, mod->size-sizeof(*mod))) {

Using sizeof(struct module) is a nono in sys_init_module(), the code has to use the user space size. Does this untested patch fix the problem? Against 2.4.0-test11-pre6 but should fit test11.

Index: 0-test11-pre6.1/kernel/module.c

kernel/module.c Thu, 23 Nov 2000 10:22:26 +1100 kaos (linux-2.4/j/28_module.c 1.1.2.1.1.1.7.1.1.1 644) renamed from kernel/module.c Wed, 08 Nov 2000 11:52:15 +1100 kaos (linux-2.4/j/28_module.c 1.1.2.1.1.1.7.1.1.1 644) +3 -1
@@ -480,7 +480,9 @@ sys_init_module(const char *name_user, s
 
 	/* Ok, that's about all the sanity we can stomach; copy the rest.  */
 
-	if (copy_from_user(mod+1, mod_user+1, mod->size-sizeof(*mod))) {
+	if (copy_from_user((char *)mod+mod_user_size,
+			   (char *)mod_user+mod_user_size,
+			   mod->size-mod_user_size)) {
 		error = -EFAULT;
 		goto err3;
 	}

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

#2 lore raw
Quoted 13 lines · Keith Owens
Quoted 1 line · Russell King
       if (copy_from_user(mod+1, mod_user+1, mod->size-sizeof(*mod))) {

Using sizeof(struct module) is a nono in sys_init_module(), the code has to use the user space size. Does this untested patch fix the problem? Against 2.4.0-test11-pre6 but should fit test11.

Index: 0-test11-pre6.1/kernel/module.c

kernel/module.c Thu, 23 Nov 2000 10:22:26 +1100 kaos (linux-2.4/j/28_module.c 1.1.2.1.1.1.7.1.1.1 644) sys_init_module(const char *name_user, s · line 488

Quoted 10 lines
kernel/module.c Thu, 23 Nov 2000 10:22:26 +1100 kaos (linux-2.4/j/28_module.c 1.1.2.1.1.1.7.1.1.1 644) renamed from kernel/module.c Wed, 08 Nov 2000 11:52:15 +1100 kaos (linux-2.4/j/28_module.c 1.1.2.1.1.1.7.1.1.1 644) +3 -1
@@ -480,7 +480,9 @@ sys_init_module(const char *name_user, s
 
 	/* Ok, that's about all the sanity we can stomach; copy the rest.  */
 
-	if (copy_from_user(mod+1, mod_user+1, mod->size-sizeof(*mod))) {
+	if (copy_from_user((char *)mod+mod_user_size,
+			   (char *)mod_user+mod_user_size,
+			   mod->size-mod_user_size)) {
 		error = -EFAULT;
 		goto err3;
 	}

This also works!
_____

  |_____| ------------------------------------------------- ---+---+-
  |   |         Russell King        [email protected]      --- ---
  | | | | http://www.arm.linux.org.uk/personal/aboutme.html   /  /  |
  | +-+-+                                                     --- -+-
  /   |               THE developer of ARM Linux              |+| /|\
 /  | | |                                                     ---  |
    +-+-+ -------------------------------------------------  /\\\  |

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/