Skip to contentSkip to conversation

convert_fxsr_from_user

DISCUSSION Dave Jones 2 participants 3 messages Apr 9, 2003

lore
Share & tools
lore

Dave Jones

author
#0 lore raw

Andrew,
A while back you optimised this routine to not do lots of memory
copies. I've noticed it does no checking on the validity of the addresses it dereferences from userspace.

Looks like we need to...

Dave

Unable to handle kernel paging request at virtual address 08048514
 printing eip:
c0114490
*pde = 05ad8067
*pte = 0586a025
Oops: 0003 [#1]
CPU:    0
EIP:    0060:[<c0114490>]    Not tainted
EFLAGS: 00010246

EIP is at convert_fxsr_from_user+0xe0/0x150
eax: e9000000 ebx: 08048514 ecx: c5be1d40 edx: 00000000
esi: c56d4000 edi: 00000000 ebp: c56d5f1c esp: c56d5ee4
ds: 007b es: 007b ss: 0068
Process a.out (pid: 638, threadinfo=c56d4000 task=c5be1980)

Stack: c56d5ef0 080484f8 0000001c 83e58955 51e808ec e8000001 000001ac 0007cbe8 
       00c3c900 90f435ff c6165d70 c5be1980 c5be1d20 080484f8 c56d5f3c c0114711 
       c5be1d20 080484f8 00000200 080484f8 00000000 00000000 c56d5f54 c01147a0 
Call Trace:
 [<c0114711>] restore_i387_fxsave+0x81/0x90
 [<c01147a0>] restore_i387+0x80/0x90
 [<c01092b8>] restore_sigcontext+0x128/0x140
 [<c010972c>] sys_rt_sigreturn+0x1bc/0x2e0
 [<c016ba65>] sys_write+0x45/0x60
 [<c010a457>] syscall_call+0x7/0xb

Code: 89 03 85 d2 75 2e 8b 41 04 89 43 04 85 d2 75 24 66 8b 41 08
#1 lore raw
Quoted 5 lines · Dave Jones

Andrew,
A while back you optimised this routine to not do lots of memory
copies. I've noticed it does no checking on the validity of the addresses it dereferences from userspace.

It never has performed those checks. The check is in the caller,
arch/i386/kernel/signal.c:restore_i387.

Bless you for merging Jon's uaccess.h documentation patch.

My __get_user()'s are arse-about.

arch/i386/kernel/i387.c renamed from arch/i386/kernel/i387.c~convert_fxsr_from_user-get_user-fixes +3 -3
diff -puN arch/i386/kernel/i387.c~convert_fxsr_from_user-get_user-fixes arch/i386/kernel/i387.c
@@ -275,9 +275,9 @@ static int convert_fxsr_from_user( struc
 		unsigned long *t = (unsigned long *)to;
 		unsigned long *f = (unsigned long *)from;
 
-		if (__get_user(*f, t) ||
-				__get_user(*(f + 1), t + 1) ||
-				__get_user(from->exponent, &to->exponent))
+		if (__get_user(*t, f) ||
+				__get_user(*(t + 1), f + 1) ||
+				__get_user(to->exponent, &from->exponent))
 			return 1;
 	}
 	return 0;

_

author
#2 lore raw
Quoted 2 lines · Andrew Morton

It never has performed those checks. The check is in the caller,
arch/i386/kernel/signal.c:restore_i387.

Ah, mixing my __'s up.

Quoted 2 lines

Bless you for merging Jon's uaccess.h documentation patch. My __get_user()'s are arse-about.

Yup, that's the bugger. Fixes the problem here.

Dave