| Message ID | 20180830143348.10595-1-marcandre.lureau@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | seccomp: check TSYNC host capability | expand |
On 30/08/2018 - 16:33:48, Marc-André Lureau wrote: > Remove -sandbox option if the host is not capable of TSYNC, since the > sandbox will fail at setup time otherwise. This will help libvirt, for > ex, to figure out if -sandbox will work. > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> > --- > qemu-seccomp.c | 19 ++++++++++++++++++- > vl.c | 4 ++-- > 2 files changed, 20 insertions(+), 3 deletions(-) > > diff --git a/qemu-seccomp.c b/qemu-seccomp.c > index 4729eb107f..1baa5c69ed 100644 > --- a/qemu-seccomp.c > +++ b/qemu-seccomp.c > @@ -282,7 +282,24 @@ static QemuOptsList qemu_sandbox_opts = { > > static void seccomp_register(void) > { > - qemu_add_opts(&qemu_sandbox_opts); > + bool add = false; > + > + /* FIXME: use seccomp_api_get() >= 2 check when released */ > + > +#if defined(SECCOMP_FILTER_FLAG_TSYNC) > + int check; > + > + /* check host TSYNC capability, it returns errno == ENOSYS if unavailable */ > + check = qemu_seccomp(SECCOMP_SET_MODE_FILTER, > + SECCOMP_FILTER_FLAG_TSYNC, NULL); > + if (check < 0 && errno == EFAULT) { > + add = true; > + } > +#endif > + > + if (add) { > + qemu_add_opts(&qemu_sandbox_opts); > + } > } > opts_init(seccomp_register); > #endif > diff --git a/vl.c b/vl.c > index 408bff9c1e..70c2449b86 100644 > --- a/vl.c > +++ b/vl.c > @@ -4007,8 +4007,8 @@ int main(int argc, char **argv, char **envp) > } > > #ifdef CONFIG_SECCOMP > - if (qemu_opts_foreach(qemu_find_opts("sandbox"), > - parse_sandbox, NULL, NULL)) { > + olist = qemu_find_opts_err("sandbox", NULL); > + if (olist && qemu_opts_foreach(olist, parse_sandbox, NULL, NULL)) { > exit(1); > } > #endif > -- > 2.19.0.rc0.48.gb9dfa238d5 > Acked-by: Eduardo Otubo <otubo@redhat.com>
On Thu, Aug 30, 2018 at 04:33:48PM +0200, Marc-André Lureau wrote: > Remove -sandbox option if the host is not capable of TSYNC, since the > sandbox will fail at setup time otherwise. This will help libvirt, for > ex, to figure out if -sandbox will work. > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> This seems to have introduced a regression, which I found when preparing a ppc pull request. Specifically when running with RHEL7 on a POWER host, using "-sandbox off" which one of my tests did, causes a cryptic error followed by a SEGV: $ ./ppc64-softmmu/qemu-system-ppc64 -sandbox off qemu-system-ppc64: -sandbox off: There is no option group 'sandbox' Segmentation fault $ ./x86_64-softmmu/qemu-system-x86_64 -sandbox off qemu-system-x86_64: -sandbox off: There is no option group 'sandbox' Segmentation fault I think the problem is that while this wrapped one use of the sandbox option group to produce a sensible error, it didn't do the same for another call to qemu_opts_parse_noisily(): (gdb) bt #0 0x00000000105b36d8 in opts_parse (list=0x0, params=0x3ffffffffab5 "off", permit_abbrev=true, defaults=false, errp=0x3ffffffff080) at util/qemu-option.c:829 #1 0x00000000105b3b74 in qemu_opts_parse_noisily (list=<optimized out>, params=<optimized out>, permit_abbrev=<optimized out>) at util/qemu-option.c:890 #2 0x0000000010024964 in main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at vl.c:3589 I'm guessing RHEL7 triggers it because that has a version of libseccomp that doesn't support the feature needed to complete registration (maybe on ppc host only; I haven't had a chance to try on an x86 RHEL7 host).
On 13/12/2018 04.24, David Gibson wrote: > On Thu, Aug 30, 2018 at 04:33:48PM +0200, Marc-André Lureau wrote: >> Remove -sandbox option if the host is not capable of TSYNC, since the >> sandbox will fail at setup time otherwise. This will help libvirt, for >> ex, to figure out if -sandbox will work. >> >> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> > > This seems to have introduced a regression, which I found when > preparing a ppc pull request. Specifically when running with RHEL7 on > a POWER host, using "-sandbox off" which one of my tests did, causes a > cryptic error followed by a SEGV: > > $ ./ppc64-softmmu/qemu-system-ppc64 -sandbox off > qemu-system-ppc64: -sandbox off: There is no option group 'sandbox' > Segmentation fault > $ ./x86_64-softmmu/qemu-system-x86_64 -sandbox off > qemu-system-x86_64: -sandbox off: There is no option group 'sandbox' > Segmentation fault > > I think the problem is that while this wrapped one use of the sandbox > option group to produce a sensible error, it didn't do the same for > another call to qemu_opts_parse_noisily(): > > (gdb) bt > #0 0x00000000105b36d8 in opts_parse (list=0x0, params=0x3ffffffffab5 "off", permit_abbrev=true, defaults=false, errp=0x3ffffffff080) > at util/qemu-option.c:829 > #1 0x00000000105b3b74 in qemu_opts_parse_noisily (list=<optimized out>, params=<optimized out>, permit_abbrev=<optimized out>) at util/qemu-option.c:890 > #2 0x0000000010024964 in main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at vl.c:3589 > > I'm guessing RHEL7 triggers it because that has a version of > libseccomp that doesn't support the feature needed to complete > registration (maybe on ppc host only; I haven't had a chance to try on > an x86 RHEL7 host). Andrea reported the same issue again today with QEMU v4.0 ... Marc-André, have you ever had another look into this issue? Thomas
Hi On Mon, Apr 29, 2019 at 3:22 PM Thomas Huth <thuth@redhat.com> wrote: > > On 13/12/2018 04.24, David Gibson wrote: > > On Thu, Aug 30, 2018 at 04:33:48PM +0200, Marc-André Lureau wrote: > >> Remove -sandbox option if the host is not capable of TSYNC, since the > >> sandbox will fail at setup time otherwise. This will help libvirt, for > >> ex, to figure out if -sandbox will work. > >> > >> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> > > > > This seems to have introduced a regression, which I found when > > preparing a ppc pull request. Specifically when running with RHEL7 on > > a POWER host, using "-sandbox off" which one of my tests did, causes a > > cryptic error followed by a SEGV: > > > > $ ./ppc64-softmmu/qemu-system-ppc64 -sandbox off > > qemu-system-ppc64: -sandbox off: There is no option group 'sandbox' > > Segmentation fault > > $ ./x86_64-softmmu/qemu-system-x86_64 -sandbox off > > qemu-system-x86_64: -sandbox off: There is no option group 'sandbox' > > Segmentation fault > > > > I think the problem is that while this wrapped one use of the sandbox > > option group to produce a sensible error, it didn't do the same for > > another call to qemu_opts_parse_noisily(): > > > > (gdb) bt > > #0 0x00000000105b36d8 in opts_parse (list=0x0, params=0x3ffffffffab5 "off", permit_abbrev=true, defaults=false, errp=0x3ffffffff080) > > at util/qemu-option.c:829 > > #1 0x00000000105b3b74 in qemu_opts_parse_noisily (list=<optimized out>, params=<optimized out>, permit_abbrev=<optimized out>) at util/qemu-option.c:890 > > #2 0x0000000010024964 in main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at vl.c:3589 > > > > I'm guessing RHEL7 triggers it because that has a version of > > libseccomp that doesn't support the feature needed to complete > > registration (maybe on ppc host only; I haven't had a chance to try on > > an x86 RHEL7 host). > > Andrea reported the same issue again today with QEMU v4.0 ... > Marc-André, have you ever had another look into this issue? Sorry, I thought this was already fixed and I forgot about it. I just sent "[PATCH] vl: fix -sandbox parsing crash when seccomp support is disabled". thanks
diff --git a/qemu-seccomp.c b/qemu-seccomp.c index 4729eb107f..1baa5c69ed 100644 --- a/qemu-seccomp.c +++ b/qemu-seccomp.c @@ -282,7 +282,24 @@ static QemuOptsList qemu_sandbox_opts = { static void seccomp_register(void) { - qemu_add_opts(&qemu_sandbox_opts); + bool add = false; + + /* FIXME: use seccomp_api_get() >= 2 check when released */ + +#if defined(SECCOMP_FILTER_FLAG_TSYNC) + int check; + + /* check host TSYNC capability, it returns errno == ENOSYS if unavailable */ + check = qemu_seccomp(SECCOMP_SET_MODE_FILTER, + SECCOMP_FILTER_FLAG_TSYNC, NULL); + if (check < 0 && errno == EFAULT) { + add = true; + } +#endif + + if (add) { + qemu_add_opts(&qemu_sandbox_opts); + } } opts_init(seccomp_register); #endif diff --git a/vl.c b/vl.c index 408bff9c1e..70c2449b86 100644 --- a/vl.c +++ b/vl.c @@ -4007,8 +4007,8 @@ int main(int argc, char **argv, char **envp) } #ifdef CONFIG_SECCOMP - if (qemu_opts_foreach(qemu_find_opts("sandbox"), - parse_sandbox, NULL, NULL)) { + olist = qemu_find_opts_err("sandbox", NULL); + if (olist && qemu_opts_foreach(olist, parse_sandbox, NULL, NULL)) { exit(1); } #endif
Remove -sandbox option if the host is not capable of TSYNC, since the sandbox will fail at setup time otherwise. This will help libvirt, for ex, to figure out if -sandbox will work. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- qemu-seccomp.c | 19 ++++++++++++++++++- vl.c | 4 ++-- 2 files changed, 20 insertions(+), 3 deletions(-)