v-s reports: struct task_struct.comm is defined to be 16 chars, but arch/x86_64/sys_ia32.c:sys32_ni_syscall() copies it into a static 8 byte buffer, which will surely cause problems.
v-s reports: struct task_struct.comm is defined to be 16 chars, but arch/x86_64/sys_ia32.c:sys32_ni_syscall() copies it into a static 8 byte buffer, which will surely cause problems. This patch makes lastcomm[] the right size, and makes sure it can't be overrun. Since the code also goes to the effort of getting a local copy of current in "me", we may as well use it for printing the message. Patch is against 2.6.10-rc2-mm3. J arch/x86_64/ia32/sys_ia32.c | 11 ++++++----- 1 files changed, 6 insertions(+), 5 deletions(-) diff -puN arch/x86_64/ia32/sys_ia32.c~short-comm-string arch/x86_64/ia32/sys_ia32.c --- local-2.6/arch/x86_64/ia32/sys_ia32.c~short-comm-string 2004-11-29 19:51:02.922621617 -0800 +++ local-2.6-jeremy/arch/x86_64/ia32/sys_ia32.c 2004-11-29 19:52:43.493561830 -0800 @@ -525,11 +525,12 @@ sys32_waitpid(compat_pid_t pid, unsigned int sys32_ni_syscall(int call) { struct task_struct *me = current; - static char lastcomm[8]; - if (strcmp(lastcomm, me->comm)) { - printk(KERN_INFO "IA32 syscall %d from %s not implemented\n", call, - current->comm); - strcpy(lastcomm, me->comm); + static char lastcomm[sizeof(me->comm)]; + + if (strncmp(lastcomm, me->comm, sizeof(lastcomm))) { + printk(KERN_INFO "IA32 syscall %d from %s not implemented\n", call, + me->comm); + strncpy(lastcomm, me->comm, sizeof(lastcomm)); } return -ENOSYS; } _ Another posting: > There's at least one more in the same file. Looking for any more. Only found the one extra in sys32_vm86_warning. ===== arch/x86_64/ia32/sys_ia32.c 1.74 vs edited ===== --- 1.74/arch/x86_64/ia32/sys_ia32.c 2004-11-02 06:40:37 -08:00 +++ edited/arch/x86_64/ia32/sys_ia32.c 2004-11-30 09:42:26 -08:00 @@ -525,11 +525,12 @@ sys32_waitpid(compat_pid_t pid, unsigned int sys32_ni_syscall(int call) { struct task_struct *me = current; - static char lastcomm[8]; - if (strcmp(lastcomm, me->comm)) { - printk(KERN_INFO "IA32 syscall %d from %s not implemented\n", call, - current->comm); - strcpy(lastcomm, me->comm); + static char lastcomm[sizeof(me->comm)]; + + if (strncmp(lastcomm, me->comm, sizeof(lastcomm))) { + printk(KERN_INFO "IA32 syscall %d from %s not implemented\n", + call, me->comm); + strncpy(lastcomm, me->comm, sizeof(lastcomm)); } return -ENOSYS; } @@ -1125,11 +1126,11 @@ long sys32_fadvise64_64(int fd, __u32 of long sys32_vm86_warning(void) { struct task_struct *me = current; - static char lastcomm[8]; - if (strcmp(lastcomm, me->comm)) { + static char lastcomm[sizeof(me->comm)]; + if (strncmp(lastcomm, me->comm, sizeof(lastcomm))) { printk(KERN_INFO "%s: vm86 mode not supported on 64 bit kernel\n", me->comm); - strcpy(lastcomm, me->comm); + strncpy(lastcomm, me->comm, sizeof(lastcomm)); } return -ENOSYS; }
This is public now. CAN number pending.
Assigned CAN-2004-1151
From Andi Kleen : ---------------------------- It overflows 8 bytes into the next, not the previous static variable. That would be ffffffff805362e8 b lastcomm.0 ffffffff805362f0 b lastcomm.1 ffffffff805362f8 b count.1 It's probably one of the count variables in fs/compat_ioctl.c overwriting these is completely harmless because they only control a printk. So in short I think it cannot be exploited in any ways. ----------------------------
Created attachment 46359 [details, diff] Patch
The following externally maintained sources need this applying: gentoo-dev-sources -- Adding dsd... rsbac-dev-sources -- Adding kang...
gentoo-dev-sources was already done :)
rsbac-dev-sources is ok now
Mass-Ccing kern-sec@gentoo.org to make sure Kernel Security guys know about all of these...
All fixed, resolving bug.
