qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH] sandbox: Report error on forbidden system call


From: Daniel P. Berrange
Subject: Re: [Qemu-devel] [PATCH] sandbox: Report error on forbidden system call
Date: Wed, 6 Feb 2013 11:25:10 +0000
User-agent: Mutt/1.5.21 (2010-09-15)

On Tue, Feb 05, 2013 at 09:28:51AM -0500, Corey Bryant wrote:
> 
> On 02/05/2013 06:02 AM, Michal Privoznik wrote:
> >Currently, it we call a not white listed system call, we get killed
> >immediately without reporting any error. It would be far more useful,
> >if we can at least shout something on stderr just before dying, so
> >users know it is because of sandbox, not just random quit.
> >
> >Signed-off-by: Michal Privoznik <address@hidden>
> >---
> >  os-posix.c     | 8 ++++++++
> >  qemu-seccomp.c | 4 +++-
> >  2 files changed, 11 insertions(+), 1 deletion(-)
> >
> >diff --git a/os-posix.c b/os-posix.c
> >index 5c64518..1d52306 100644
> >--- a/os-posix.c
> >+++ b/os-posix.c
> >@@ -62,6 +62,12 @@ void os_setup_early_signal_handling(void)
> >      sigaction(SIGPIPE, &act, NULL);
> >  }
> >
> >+static void syssig_handler(int signal, siginfo_t *info, void *c)
> >+{
> >+    fprintf(stderr, "Bad system call\n");
> >+    exit(1);
> >+}
> >+
> >  static void termsig_handler(int signal, siginfo_t *info, void *c)
> >  {
> >      qemu_system_killed(info->si_signo, info->si_pid);
> >@@ -77,6 +83,8 @@ void os_setup_signal_handling(void)
> >      sigaction(SIGINT,  &act, NULL);
> >      sigaction(SIGHUP,  &act, NULL);
> >      sigaction(SIGTERM, &act, NULL);
> >+    act.sa_sigaction = syssig_handler;
> >+    sigaction(SIGSYS,  &act, NULL);
> >  }
> >
> >  /* Find a likely location for support files using the location of the 
> > binary.
> >diff --git a/qemu-seccomp.c b/qemu-seccomp.c
> >index 031da1d..897d9b3 100644
> >--- a/qemu-seccomp.c
> >+++ b/qemu-seccomp.c
> >@@ -2,9 +2,11 @@
> >   * QEMU seccomp mode 2 support with libseccomp
> >   *
> >   * Copyright IBM, Corp. 2012
> >+ * Copyright (C) 2013 Red Hat, Inc.
> >   *
> >   * Authors:
> >   *  Eduardo Otubo    <address@hidden>
> >+ *  Michal Privoznik <address@hidden>
> >   *
> >   * This work is licensed under the terms of the GNU GPL, version 2.  See
> >   * the COPYING file in the top-level directory.
> >@@ -238,7 +240,7 @@ int seccomp_start(void)
> >      unsigned int i = 0;
> >      scmp_filter_ctx ctx;
> >
> >-    ctx = seccomp_init(SCMP_ACT_KILL);
> >+    ctx = seccomp_init(SCMP_ACT_TRAP);
> >      if (ctx == NULL) {
> >          goto seccomp_return;
> >      }
> >
> 
> I think this is going to be better solved in the kernel.  I have a
> kernel patch sitting out there at:
> https://lkml.org/lkml/2013/1/7/313
> Any public support of this patch could be useful to help get it in.

While that "learning mode" is useful, I don't think that really
solves the problem that Michael is looking at. We're running a QEMU
guest in production mode and it gets killed with no indication of
why. It is definitely desirable that we get a log message in QEMU's
stderr for this scenario.

> Something is definitely needed to learn the syscall that is killing
> QEMU.  But I don't think the signal handler approach is going to
> work. We tried that and ran into too many situations where signals
> were being blocked by libraries (spice is one example).  And we
> didn't want to get in the business of patching third party libraries
> to allow SIGSYS.

We absolutely should be fixing libraries not to screw up signal handling
in applications. I see that the SPICE worker thread blocks every single
signal except SEGV/FPE/ILL, which is just completely bogus. There's no
acceptable reason for SPICE to block so many signals.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|



reply via email to

[Prev in Thread] Current Thread [Next in Thread]