All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] seccomp: check TSYNC host capability
@ 2018-08-30 14:33 Marc-André Lureau
  2018-09-10  8:36 ` Eduardo Otubo
  2018-12-13  3:24 ` David Gibson
  0 siblings, 2 replies; 6+ messages in thread
From: Marc-André Lureau @ 2018-08-30 14:33 UTC (permalink / raw)
  To: qemu-devel; +Cc: berrange, otubo, Marc-André Lureau

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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH] seccomp: check TSYNC host capability
  2018-08-30 14:33 [Qemu-devel] [PATCH] seccomp: check TSYNC host capability Marc-André Lureau
@ 2018-09-10  8:36 ` Eduardo Otubo
  2018-12-13  3:24 ` David Gibson
  1 sibling, 0 replies; 6+ messages in thread
From: Eduardo Otubo @ 2018-09-10  8:36 UTC (permalink / raw)
  To: Marc-André Lureau; +Cc: qemu-devel, berrange

[-- Attachment #1: Type: text/plain, Size: 1918 bytes --]

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>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH] seccomp: check TSYNC host capability
  2018-08-30 14:33 [Qemu-devel] [PATCH] seccomp: check TSYNC host capability Marc-André Lureau
  2018-09-10  8:36 ` Eduardo Otubo
@ 2018-12-13  3:24 ` David Gibson
  2019-04-29 13:21   ` Thomas Huth
  1 sibling, 1 reply; 6+ messages in thread
From: David Gibson @ 2018-12-13  3:24 UTC (permalink / raw)
  To: Marc-André Lureau; +Cc: qemu-devel, otubo

[-- Attachment #1: Type: text/plain, Size: 1905 bytes --]

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).

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH] seccomp: check TSYNC host capability
  2018-12-13  3:24 ` David Gibson
@ 2019-04-29 13:21   ` Thomas Huth
  2019-04-29 13:49       ` Marc-André Lureau
  0 siblings, 1 reply; 6+ messages in thread
From: Thomas Huth @ 2019-04-29 13:21 UTC (permalink / raw)
  To: David Gibson, Marc-André Lureau; +Cc: otubo, qemu-devel, Andrea Bolognani

[-- Attachment #1: Type: text/plain, Size: 1952 bytes --]

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



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH] seccomp: check TSYNC host capability
@ 2019-04-29 13:49       ` Marc-André Lureau
  0 siblings, 0 replies; 6+ messages in thread
From: Marc-André Lureau @ 2019-04-29 13:49 UTC (permalink / raw)
  To: Thomas Huth; +Cc: David Gibson, Eduardo Otubo, QEMU, Andrea Bolognani

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


-- 
Marc-André Lureau

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH] seccomp: check TSYNC host capability
@ 2019-04-29 13:49       ` Marc-André Lureau
  0 siblings, 0 replies; 6+ messages in thread
From: Marc-André Lureau @ 2019-04-29 13:49 UTC (permalink / raw)
  To: Thomas Huth; +Cc: Eduardo Otubo, QEMU, Andrea Bolognani, David Gibson

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


-- 
Marc-André Lureau


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2019-04-29 13:53 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-30 14:33 [Qemu-devel] [PATCH] seccomp: check TSYNC host capability Marc-André Lureau
2018-09-10  8:36 ` Eduardo Otubo
2018-12-13  3:24 ` David Gibson
2019-04-29 13:21   ` Thomas Huth
2019-04-29 13:49     ` Marc-André Lureau
2019-04-29 13:49       ` Marc-André Lureau

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.