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;
}
_
* Jeremy Fitzhardinge ([email protected]) wrote:
> 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.
Looks good, but you missed sys32_vm86_warning.
Signed-off-by: Chris Wright <[email protected]>
===== 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;
}
On Tue, 2004-11-30 at 10:31 -0800, Chris Wright wrote:
> * Jeremy Fitzhardinge ([email protected]) wrote:
> > 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.
>
> Looks good, but you missed sys32_vm86_warning.
Hadn't got that far. Should we be worried that task_struct.comm might
not be \0-terminated, and therefore use ("... %.*s ...",
sizeof(lastcomm), lastcomm) in the printk's?
J
* Jeremy Fitzhardinge ([email protected]) wrote:
> On Tue, 2004-11-30 at 10:31 -0800, Chris Wright wrote:
> > * Jeremy Fitzhardinge ([email protected]) wrote:
> > > 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.
> >
> > Looks good, but you missed sys32_vm86_warning.
>
> Hadn't got that far. Should we be worried that task_struct.comm might
> not be \0-terminated, and therefore use ("... %.*s ...",
> sizeof(lastcomm), lastcomm) in the printk's?
It gets NULL terminated during exec or prctl.
thanks,
-chris
--
Linux Security Modules http://lsm.immunix.org http://lsm.bkbits.net