| Message ID | 20170901105818.31956-3-otubo@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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).
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
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
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;
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(-)