Bugzilla Bug 73000

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 an attachment (id=46359) [details]
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.