lkml.org 
[lkml]   [2006]   [May]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [patch 03/24] make vm86 call audit_syscall_exit

On Sat, 29 Apr 2006, Sergey Vlasov wrote:

> On Thu, 27 Apr 2006 17:16:52 -0700 Greg KH wrote:
>
> > -stable review patch. If anyone has any objections, please let us know.
> >
> > ------------------
> > hi,
> >
> > The motivation behind the patch below was to address messages in
> > /var/log/messages such as:
> >
> > Jan 31 10:54:15 mets kernel: audit(:0): major=252 name_count=0: freeing
> > multiple contexts (1)
> > Jan 31 10:54:15 mets kernel: audit(:0): major=113 name_count=0: freeing
> > multiple contexts (2)
> >
> > I can reproduce by running 'get-edid' from:
> > http://john.fremlin.de/programs/linux/read-edid/.
> >
> > These messages come about in the log b/c the vm86 calls do not exit via
> > the normal system call exit paths and thus do not call
> > 'audit_syscall_exit'. The next system call will then free the context for
> > itself and for the vm86 context, thus generating the above messages. This
> > patch addresses the issue by simply adding a call to 'audit_syscall_exit'
> > from the vm86 code.
> >
> > Besides fixing the above error messages the patch also now allows vm86
> > system calls to become auditable. This is useful since strace does not
> > appear to properly record the return values from sys_vm86.
>
> I don't understand how this is supposed to log the sys_vm86 return value
> properly - the return value for userspace would be known only in
> return_to_32bit(), and here audit_syscall_exit() is called in
> do_sys_vm86(), before the VM86-mode code has even started to run.
>
> > I think this patch is also a step in the right direction in terms of
> > cleaning up some core auditing code. If we can correct any other paths
> > that do not properly call the audit exit and entries points, then we can
> > also eliminate the notion of context chaining.
> >
> > I've tested this patch by verifying that the log messages no longer
> > appear, and that the audit records for sys_vm86 appear to be correct.
>
> And what return values are logged?
>

the xorl of eax with itself produces 0.

> > Also, 'read_edid' produces itentical output.
> >
> > thanks,
> >
> > -Jason
> >
> > Signed-off-by: Jason Baron <jbaron@redhat.com>
> > Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> > Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
> >
> > ---
> > ---
> > arch/i386/kernel/vm86.c | 12 ++++++++++--
> > kernel/auditsc.c | 5 -----
> > 2 files changed, 10 insertions(+), 7 deletions(-)
> >
> > --- linux-2.6.16.11.orig/arch/i386/kernel/vm86.c
> > +++ linux-2.6.16.11/arch/i386/kernel/vm86.c
> > @@ -43,6 +43,7 @@
> > #include <linux/smp_lock.h>
> > #include <linux/highmem.h>
> > #include <linux/ptrace.h>
> > +#include <linux/audit.h>
> >
> > #include <asm/uaccess.h>
> > #include <asm/io.h>
> > @@ -252,6 +253,7 @@ out:
> > static void do_sys_vm86(struct kernel_vm86_struct *info, struct task_struct *tsk)
> > {
> > struct tss_struct *tss;
> > + long eax;
> > /*
> > * make sure the vm86() system call doesn't try to do anything silly
> > */
> > @@ -305,13 +307,19 @@ static void do_sys_vm86(struct kernel_vm
> > tsk->thread.screen_bitmap = info->screen_bitmap;
> > if (info->flags & VM86_SCREEN_BITMAP)
> > mark_screen_rdonly(tsk->mm);
> > + __asm__ __volatile__("xorl %eax,%eax; movl %eax,%fs; movl %eax,%gs\n\t");
> > + __asm__ __volatile__("movl %%eax, %0\n" :"=r"(eax));
>
> This fetches whatever value happened to be in the EAX register at this
> point, and stuffs it into the eax variable. Most likely EAX will
> contain 0 from the previous asm commands. I'm not sure whether gcc
> could insert some code of its own between two "asm volatile" statements,
> but this asm statement looks like a bug by itself.
>
> > +
> > + /*call audit_syscall_exit since we do not exit via the normal paths */
> > + if (unlikely(current->audit_context))
> > + audit_syscall_exit(current, AUDITSC_RESULT(eax), eax);
>
> Then this probably always records 0 as the syscall result.
>
> However, looks like moving audit_syscall_exit() into return_to_32bit()
> (where the syscall result is known) would not work because of issues
> with signal handling... So maybe we are stuck with partially wrong
> audit records for vm86, but at least the broken asm should be removed.
>

agreed. I've been meaning to post a patch to clean this up...how does this
look?

--- linux-2.6/arch/i386/kernel/vm86.c.bak 2006-04-30 21:00:21.000000000 -0400
+++ linux-2.6/arch/i386/kernel/vm86.c 2006-04-30 21:08:32.000000000 -0400
@@ -253,7 +253,6 @@ out:
static void do_sys_vm86(struct kernel_vm86_struct *info, struct task_struct *tsk)
{
struct tss_struct *tss;
- long eax;
/*
* make sure the vm86() system call doesn't try to do anything silly
*/
@@ -307,19 +306,18 @@ static void do_sys_vm86(struct kernel_vm
tsk->thread.screen_bitmap = info->screen_bitmap;
if (info->flags & VM86_SCREEN_BITMAP)
mark_screen_rdonly(tsk->mm);
- __asm__ __volatile__("xorl %eax,%eax; movl %eax,%fs; movl %eax,%gs\n\t");
- __asm__ __volatile__("movl %%eax, %0\n" :"=r"(eax));

/*call audit_syscall_exit since we do not exit via the normal paths */
if (unlikely(current->audit_context))
- audit_syscall_exit(current, AUDITSC_RESULT(eax), eax);
+ audit_syscall_exit(current, AUDITSC_RESULT(0), 0);

__asm__ __volatile__(
+ "xorl %%eax,%%eax; movl %%eax,%%fs; movl %%eax,%%gs\n\t"
"movl %0,%%esp\n\t"
"movl %1,%%ebp\n\t"
"jmp resume_userspace"
: /* no outputs */
- :"r" (&info->regs), "r" (task_thread_info(tsk)));
+ :"r" (&info->regs), "r" (task_thread_info(tsk)) : "ax");
/* we never return here */
}

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
\
 
 \ /
  Last update: 2006-05-01 19:45    [W:0.051 / U:0.392 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site