[Qemu-devel] [PATCHv4 2/6] seccomp: add obsolete argument to command line

Eduardo Otubo posted 6 patches 8 years, 2 months ago
[Qemu-devel] [PATCHv4 2/6] seccomp: add obsolete argument to command line
Posted by Eduardo Otubo 8 years, 2 months ago
This patch introduces the argument [,obsolete=allow] to the `-sandbox on'
option. It allows Qemu to run safely on old system that still relies on
old system calls.

Signed-off-by: Eduardo Otubo <otubo@redhat.com>
---
 include/sysemu/seccomp.h |  3 ++-
 qemu-options.hx          | 12 ++++++++++--
 qemu-seccomp.c           | 23 ++++++++++++++++++++++-
 vl.c                     | 22 +++++++++++++++++++++-
 4 files changed, 55 insertions(+), 5 deletions(-)

diff --git a/include/sysemu/seccomp.h b/include/sysemu/seccomp.h
index 23b9c3c789..215138a372 100644
--- a/include/sysemu/seccomp.h
+++ b/include/sysemu/seccomp.h
@@ -16,8 +16,9 @@
 #define QEMU_SECCOMP_H
 
 #define QEMU_SECCOMP_SET_DEFAULT     (1 << 0)
+#define QEMU_SECCOMP_SET_OBSOLETE    (1 << 1)
 
 #include <seccomp.h>
 
-int seccomp_start(void);
+int seccomp_start(uint32_t seccomp_opts);
 #endif
diff --git a/qemu-options.hx b/qemu-options.hx
index 9f6e2adfff..72150c6b84 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -4017,13 +4017,21 @@ Old param mode (ARM only).
 ETEXI
 
 DEF("sandbox", HAS_ARG, QEMU_OPTION_sandbox, \
-    "-sandbox <arg>  Enable seccomp mode 2 system call filter (default 'off').\n",
+    "-sandbox on[,obsolete=allow|deny]\n" \
+    "                Enable seccomp mode 2 system call filter (default 'off').\n" \
+    "                use 'obsolete' to allow obsolete system calls that are provided\n" \
+    "                    by the kernel, but typically no longer used by modern\n" \
+    "                    C library implementations.\n",
     QEMU_ARCH_ALL)
 STEXI
-@item -sandbox @var{arg}
+@item -sandbox @var{arg}[,obsolete=@var{string}]
 @findex -sandbox
 Enable Seccomp mode 2 system call filter. 'on' will enable syscall filtering and 'off' will
 disable it.  The default is 'off'.
+@table @option
+@item obsolete=@var{string}
+Enable Obsolete system calls
+@end table
 ETEXI
 
 DEF("readconfig", HAS_ARG, QEMU_OPTION_readconfig,
diff --git a/qemu-seccomp.c b/qemu-seccomp.c
index 585de42a97..3e3f15cc08 100644
--- a/qemu-seccomp.c
+++ b/qemu-seccomp.c
@@ -56,9 +56,22 @@ static const struct QemuSeccompSyscall blacklist[] = {
     { SCMP_SYS(tuxcall),               1, QEMU_SECCOMP_SET_DEFAULT },
     { SCMP_SYS(ulimit),                1, QEMU_SECCOMP_SET_DEFAULT },
     { SCMP_SYS(vserver),               1, QEMU_SECCOMP_SET_DEFAULT },
+    /* obsolete */
+    { SCMP_SYS(readdir),               2, QEMU_SECCOMP_SET_OBSOLETE },
+    { SCMP_SYS(_sysctl),               2, QEMU_SECCOMP_SET_OBSOLETE },
+    { SCMP_SYS(bdflush),               2, QEMU_SECCOMP_SET_OBSOLETE },
+    { SCMP_SYS(create_module),         2, QEMU_SECCOMP_SET_OBSOLETE },
+    { SCMP_SYS(get_kernel_syms),       2, QEMU_SECCOMP_SET_OBSOLETE },
+    { SCMP_SYS(query_module),          2, QEMU_SECCOMP_SET_OBSOLETE },
+    { SCMP_SYS(sgetmask),              2, QEMU_SECCOMP_SET_OBSOLETE },
+    { SCMP_SYS(ssetmask),              2, QEMU_SECCOMP_SET_OBSOLETE },
+    { SCMP_SYS(sysfs),                 2, QEMU_SECCOMP_SET_OBSOLETE },
+    { SCMP_SYS(uselib),                2, QEMU_SECCOMP_SET_OBSOLETE },
+    { SCMP_SYS(ustat),                 2, QEMU_SECCOMP_SET_OBSOLETE },
 };
 
-int seccomp_start(void)
+
+int seccomp_start(uint32_t seccomp_opts)
 {
     int rc = 0;
     unsigned int i = 0;
@@ -72,6 +85,14 @@ int seccomp_start(void)
 
     for (i = 0; i < ARRAY_SIZE(blacklist); i++) {
         switch (blacklist[i].set) {
+        case QEMU_SECCOMP_SET_OBSOLETE:
+            if (!(seccomp_opts & QEMU_SECCOMP_SET_OBSOLETE)) {
+                goto add_syscall;
+            } else {
+                continue;
+            }
+
+            break;
         default:
             goto add_syscall;
         }
diff --git a/vl.c b/vl.c
index 305531aba8..ca267f9918 100644
--- a/vl.c
+++ b/vl.c
@@ -271,6 +271,10 @@ static QemuOptsList qemu_sandbox_opts = {
             .name = "enable",
             .type = QEMU_OPT_BOOL,
         },
+        {
+            .name = "obsolete",
+            .type = QEMU_OPT_STRING,
+        },
         { /* end of list */ }
     },
 };
@@ -1032,7 +1036,23 @@ static int parse_sandbox(void *opaque, QemuOpts *opts, Error **errp)
 {
     if (qemu_opt_get_bool(opts, "enable", false)) {
 #ifdef CONFIG_SECCOMP
-        if (seccomp_start() < 0) {
+        uint32_t seccomp_opts = 0x00000;
+        const char *value = NULL;
+
+        value = qemu_opt_get(opts, "obsolete");
+        if (value) {
+            if (strcmp(value, "allow") == 0) {
+                seccomp_opts |= QEMU_SECCOMP_SET_OBSOLETE;
+            } else if (strcmp(value, "deny")) {
+                /* this is the default option, this if is here
+		 * to provide a little bit of consistency for
+		 * the command line */
+	    } else {
+		error_report("invalid argument for obsolete");
+	    }
+        }
+
+        if (seccomp_start(seccomp_opts) < 0) {
             error_report("failed to install seccomp syscall filter "
                          "in the kernel");
             return -1;
-- 
2.13.5


Re: [Qemu-devel] [PATCHv4 2/6] seccomp: add obsolete argument to command line
Posted by Daniel P. Berrange 8 years, 2 months ago
On Fri, Sep 01, 2017 at 12:58:14PM +0200, Eduardo Otubo wrote:
> This patch introduces the argument [,obsolete=allow] to the `-sandbox on'
> option. It allows Qemu to run safely on old system that still relies on
> old system calls.
> 
> Signed-off-by: Eduardo Otubo <otubo@redhat.com>
> ---
>  include/sysemu/seccomp.h |  3 ++-
>  qemu-options.hx          | 12 ++++++++++--
>  qemu-seccomp.c           | 23 ++++++++++++++++++++++-
>  vl.c                     | 22 +++++++++++++++++++++-
>  4 files changed, 55 insertions(+), 5 deletions(-)
> 

> @@ -72,6 +85,14 @@ int seccomp_start(void)
>  
>      for (i = 0; i < ARRAY_SIZE(blacklist); i++) {
>          switch (blacklist[i].set) {
> +        case QEMU_SECCOMP_SET_OBSOLETE:
> +            if (!(seccomp_opts & QEMU_SECCOMP_SET_OBSOLETE)) {
> +                goto add_syscall;
> +            } else {
> +                continue;
> +            }
> +
> +            break;

THis can be simplified:

            if ((seccomp_opts & QEMU_SECCOMP_SET_OBSOLETE)) {
                continue;
            }

            break;

thus avoiding need to 'goto'

Likewise for all following patches

>          default:
>              goto add_syscall;
>          }

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCHv4 2/6] seccomp: add obsolete argument to command line
Posted by Eduardo Otubo 8 years, 1 month ago
On Fri, Sep 01, 2017 at 12:05:41PM +0100, Daniel P. Berrange wrote:
> On Fri, Sep 01, 2017 at 12:58:14PM +0200, Eduardo Otubo wrote:
> > This patch introduces the argument [,obsolete=allow] to the `-sandbox on'
> > option. It allows Qemu to run safely on old system that still relies on
> > old system calls.
> > 
> > Signed-off-by: Eduardo Otubo <otubo@redhat.com>
> > ---
> >  include/sysemu/seccomp.h |  3 ++-
> >  qemu-options.hx          | 12 ++++++++++--
> >  qemu-seccomp.c           | 23 ++++++++++++++++++++++-
> >  vl.c                     | 22 +++++++++++++++++++++-
> >  4 files changed, 55 insertions(+), 5 deletions(-)
> > 
> 
> > @@ -72,6 +85,14 @@ int seccomp_start(void)
> >  
> >      for (i = 0; i < ARRAY_SIZE(blacklist); i++) {
> >          switch (blacklist[i].set) {
> > +        case QEMU_SECCOMP_SET_OBSOLETE:
> > +            if (!(seccomp_opts & QEMU_SECCOMP_SET_OBSOLETE)) {
> > +                goto add_syscall;
> > +            } else {
> > +                continue;
> > +            }
> > +
> > +            break;
> 
> THis can be simplified:
> 
>             if ((seccomp_opts & QEMU_SECCOMP_SET_OBSOLETE)) {
>                 continue;
>             }
> 
>             break;
> 
> thus avoiding need to 'goto'
> 
> Likewise for all following patches

Do you think there's anything else to fix on this series? if nothing
else emerges, I'll send the v5 tomorrow (also with the style fixes).

-- 
Eduardo Otubo
Senior Software Engineer @ RedHat

Re: [Qemu-devel] [PATCHv4 2/6] seccomp: add obsolete argument to command line
Posted by Daniel P. Berrange 8 years, 1 month ago
On Thu, Sep 07, 2017 at 11:31:04AM +0200, Eduardo Otubo wrote:
> On Fri, Sep 01, 2017 at 12:05:41PM +0100, Daniel P. Berrange wrote:
> > On Fri, Sep 01, 2017 at 12:58:14PM +0200, Eduardo Otubo wrote:
> > > This patch introduces the argument [,obsolete=allow] to the `-sandbox on'
> > > option. It allows Qemu to run safely on old system that still relies on
> > > old system calls.
> > > 
> > > Signed-off-by: Eduardo Otubo <otubo@redhat.com>
> > > ---
> > >  include/sysemu/seccomp.h |  3 ++-
> > >  qemu-options.hx          | 12 ++++++++++--
> > >  qemu-seccomp.c           | 23 ++++++++++++++++++++++-
> > >  vl.c                     | 22 +++++++++++++++++++++-
> > >  4 files changed, 55 insertions(+), 5 deletions(-)
> > > 
> > 
> > > @@ -72,6 +85,14 @@ int seccomp_start(void)
> > >  
> > >      for (i = 0; i < ARRAY_SIZE(blacklist); i++) {
> > >          switch (blacklist[i].set) {
> > > +        case QEMU_SECCOMP_SET_OBSOLETE:
> > > +            if (!(seccomp_opts & QEMU_SECCOMP_SET_OBSOLETE)) {
> > > +                goto add_syscall;
> > > +            } else {
> > > +                continue;
> > > +            }
> > > +
> > > +            break;
> > 
> > THis can be simplified:
> > 
> >             if ((seccomp_opts & QEMU_SECCOMP_SET_OBSOLETE)) {
> >                 continue;
> >             }
> > 
> >             break;
> > 
> > thus avoiding need to 'goto'
> > 
> > Likewise for all following patches
> 
> Do you think there's anything else to fix on this series? if nothing
> else emerges, I'll send the v5 tomorrow (also with the style fixes).

I just sent one more comment, but apart from the that & the style fixes
it looks good to me.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCHv4 2/6] seccomp: add obsolete argument to command line
Posted by Daniel P. Berrange 8 years, 1 month ago
On Fri, Sep 01, 2017 at 12:58:14PM +0200, Eduardo Otubo wrote:
> This patch introduces the argument [,obsolete=allow] to the `-sandbox on'
> option. It allows Qemu to run safely on old system that still relies on
> old system calls.
> 
> Signed-off-by: Eduardo Otubo <otubo@redhat.com>
> ---
>  include/sysemu/seccomp.h |  3 ++-
>  qemu-options.hx          | 12 ++++++++++--
>  qemu-seccomp.c           | 23 ++++++++++++++++++++++-
>  vl.c                     | 22 +++++++++++++++++++++-
>  4 files changed, 55 insertions(+), 5 deletions(-)
> 

> @@ -1032,7 +1036,23 @@ static int parse_sandbox(void *opaque, QemuOpts *opts, Error **errp)
>  {
>      if (qemu_opt_get_bool(opts, "enable", false)) {
>  #ifdef CONFIG_SECCOMP
> -        if (seccomp_start() < 0) {
> +        uint32_t seccomp_opts = 0x00000;
> +        const char *value = NULL;
> +
> +        value = qemu_opt_get(opts, "obsolete");
> +        if (value) {
> +            if (strcmp(value, "allow") == 0) {

I would have a slight preference for g_str_equal(value, "allow")

> +                seccomp_opts |= QEMU_SECCOMP_SET_OBSOLETE;
> +            } else if (strcmp(value, "deny")) {

and  !g_str_equal(value, "deny")

> +                /* this is the default option, this if is here
> +		 * to provide a little bit of consistency for
> +		 * the command line */
> +	    } else {
> +		error_report("invalid argument for obsolete");
> +	    }

There seem to be tabs for indent here too

> +        }
> +
> +        if (seccomp_start(seccomp_opts) < 0) {
>              error_report("failed to install seccomp syscall filter "
>                           "in the kernel");
>              return -1;
> -- 
> 2.13.5
> 

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|