1. Patchew
  2. QEMU
  3. View series

[Qemu-devel] [PATCH] vl: fix -sandbox parsing crash when seccomp support is disabled

Marc-André Lureau posted 1 patch 6 years, 6 months ago
Test asan failed
Test docker-mingw@fedora failed
Test docker-clang@ubuntu failed
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20190429134757.13570-1-marcandre.lureau@redhat.com
Maintainers: Paolo Bonzini <pbonzini@redhat.com>
vl.c | 18 ++++++++++--------
1 file changed, 10 insertions(+), 8 deletions(-)
[Qemu-devel] [PATCH] vl: fix -sandbox parsing crash when seccomp support is disabled
Posted by Marc-André Lureau 6 years, 6 months ago 
$ ./x86_64-softmmu/qemu-system-x86_64 -sandbox off
qemu-system-x86_64: -sandbox off: There is no option group 'sandbox'
Segmentation fault

Commit 5780760f5e ("seccomp: check TSYNC host capability") 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
    at util/qemu-option.c:829
 #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

Fixes: 5780760f5ea6163939a5dabe7427318b4f07d1a2
Cc: david@gibson.dropbear.id.au
Cc: otubo@redhat.com
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 vl.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/vl.c b/vl.c
index 4019a4387d..5fc4994d3c 100644
--- a/vl.c
+++ b/vl.c
@@ -3866,17 +3866,19 @@ int main(int argc, char **argv, char **envp)
                 qtest_log = optarg;
                 break;
             case QEMU_OPTION_sandbox:
-#ifdef CONFIG_SECCOMP
-                opts = qemu_opts_parse_noisily(qemu_find_opts("sandbox"),
-                                               optarg, true);
+                olist = qemu_find_opts("sandbox");
+                if (!olist) {
+#ifndef CONFIG_SECCOMP
+                    error_report("-sandbox support is not enabled "
+                                 "in this QEMU binary");
+#endif
+                    exit(1);
+                }
+
+                opts = qemu_opts_parse_noisily(olist, optarg, true);
                 if (!opts) {
                     exit(1);
                 }
-#else
-                error_report("-sandbox support is not enabled "
-                             "in this QEMU binary");
-                exit(1);
-#endif
                 break;
             case QEMU_OPTION_add_fd:
 #ifndef _WIN32
-- 
2.21.0.777.g83232e3864
Re: [Qemu-devel] [PATCH] vl: fix -sandbox parsing crash when seccomp support is disabled
Posted by Philippe Mathieu-Daudé 6 years, 6 months ago 
On 4/29/19 3:47 PM, Marc-André Lureau wrote:
> $ ./x86_64-softmmu/qemu-system-x86_64 -sandbox off
> qemu-system-x86_64: -sandbox off: There is no option group 'sandbox'
> Segmentation fault
> 
> Commit 5780760f5e ("seccomp: check TSYNC host capability") 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
>     at util/qemu-option.c:829
>  #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
> 
> Fixes: 5780760f5ea6163939a5dabe7427318b4f07d1a2
> Cc: david@gibson.dropbear.id.au
> Cc: otubo@redhat.com
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>  vl.c | 18 ++++++++++--------
>  1 file changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/vl.c b/vl.c
> index 4019a4387d..5fc4994d3c 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -3866,17 +3866,19 @@ int main(int argc, char **argv, char **envp)
>                  qtest_log = optarg;
>                  break;
>              case QEMU_OPTION_sandbox:
> -#ifdef CONFIG_SECCOMP
> -                opts = qemu_opts_parse_noisily(qemu_find_opts("sandbox"),
> -                                               optarg, true);
> +                olist = qemu_find_opts("sandbox");
> +                if (!olist) {
> +#ifndef CONFIG_SECCOMP
> +                    error_report("-sandbox support is not enabled "
> +                                 "in this QEMU binary");
> +#endif
> +                    exit(1);
> +                }
> +
> +                opts = qemu_opts_parse_noisily(olist, optarg, true);
>                  if (!opts) {
>                      exit(1);
>                  }
> -#else
> -                error_report("-sandbox support is not enabled "
> -                             "in this QEMU binary");
> -                exit(1);
> -#endif
>                  break;
>              case QEMU_OPTION_add_fd:
>  #ifndef _WIN32
>
Re: [Qemu-devel] [PATCH] vl: fix -sandbox parsing crash when seccomp support is disabled
Posted by Laurent Vivier 6 years, 6 months ago 
On 29/04/2019 15:47, Marc-André Lureau wrote:
> $ ./x86_64-softmmu/qemu-system-x86_64 -sandbox off
> qemu-system-x86_64: -sandbox off: There is no option group 'sandbox'
> Segmentation fault
> 
> Commit 5780760f5e ("seccomp: check TSYNC host capability") 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
>     at util/qemu-option.c:829
>  #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
> 
> Fixes: 5780760f5ea6163939a5dabe7427318b4f07d1a2
> Cc: david@gibson.dropbear.id.au
> Cc: otubo@redhat.com
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  vl.c | 18 ++++++++++--------
>  1 file changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/vl.c b/vl.c
> index 4019a4387d..5fc4994d3c 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -3866,17 +3866,19 @@ int main(int argc, char **argv, char **envp)
>                  qtest_log = optarg;
>                  break;
>              case QEMU_OPTION_sandbox:
> -#ifdef CONFIG_SECCOMP
> -                opts = qemu_opts_parse_noisily(qemu_find_opts("sandbox"),
> -                                               optarg, true);
> +                olist = qemu_find_opts("sandbox");
> +                if (!olist) {
> +#ifndef CONFIG_SECCOMP

Why do you move the #ifdef? We have two separate error cases here.
And it seems better no to check for "-sandbox" when seccomp is disabled.

Thanks,
Laurent
Re: [Qemu-devel] [PATCH] vl: fix -sandbox parsing crash when seccomp support is disabled
Posted by Marc-André Lureau 6 years, 6 months ago 
Hi

On Mon, Apr 29, 2019 at 4:26 PM Laurent Vivier <lvivier@redhat.com> wrote:
>
> On 29/04/2019 15:47, Marc-André Lureau wrote:
> > $ ./x86_64-softmmu/qemu-system-x86_64 -sandbox off
> > qemu-system-x86_64: -sandbox off: There is no option group 'sandbox'
> > Segmentation fault
> >
> > Commit 5780760f5e ("seccomp: check TSYNC host capability") 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
> >     at util/qemu-option.c:829
> >  #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
> >
> > Fixes: 5780760f5ea6163939a5dabe7427318b4f07d1a2
> > Cc: david@gibson.dropbear.id.au
> > Cc: otubo@redhat.com
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> >  vl.c | 18 ++++++++++--------
> >  1 file changed, 10 insertions(+), 8 deletions(-)
> >
> > diff --git a/vl.c b/vl.c
> > index 4019a4387d..5fc4994d3c 100644
> > --- a/vl.c
> > +++ b/vl.c
> > @@ -3866,17 +3866,19 @@ int main(int argc, char **argv, char **envp)
> >                  qtest_log = optarg;
> >                  break;
> >              case QEMU_OPTION_sandbox:
> > -#ifdef CONFIG_SECCOMP
> > -                opts = qemu_opts_parse_noisily(qemu_find_opts("sandbox"),
> > -                                               optarg, true);
> > +                olist = qemu_find_opts("sandbox");
> > +                if (!olist) {
> > +#ifndef CONFIG_SECCOMP
>
> Why do you move the #ifdef? We have two separate error cases here.
> And it seems better no to check for "-sandbox" when seccomp is disabled.

I tried to remove the #ifdef altogether to simplify the code, then
realized the error message could be useful.

I don't think it's a problem to lookup "-sandbox" when seccomp is disabled.

-- 
Marc-André Lureau
Re: [Qemu-devel] [PATCH] vl: fix -sandbox parsing crash when seccomp support is disabled
Posted by Laurent Vivier 6 years, 6 months ago 
On 29/04/2019 16:46, Marc-André Lureau wrote:
> Hi
> 
> On Mon, Apr 29, 2019 at 4:26 PM Laurent Vivier <lvivier@redhat.com> wrote:
>>
>> On 29/04/2019 15:47, Marc-André Lureau wrote:
>>> $ ./x86_64-softmmu/qemu-system-x86_64 -sandbox off
>>> qemu-system-x86_64: -sandbox off: There is no option group 'sandbox'
>>> Segmentation fault
>>>
>>> Commit 5780760f5e ("seccomp: check TSYNC host capability") 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
>>>     at util/qemu-option.c:829
>>>  #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
>>>
>>> Fixes: 5780760f5ea6163939a5dabe7427318b4f07d1a2
>>> Cc: david@gibson.dropbear.id.au
>>> Cc: otubo@redhat.com
>>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>>> ---
>>>  vl.c | 18 ++++++++++--------
>>>  1 file changed, 10 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/vl.c b/vl.c
>>> index 4019a4387d..5fc4994d3c 100644
>>> --- a/vl.c
>>> +++ b/vl.c
>>> @@ -3866,17 +3866,19 @@ int main(int argc, char **argv, char **envp)
>>>                  qtest_log = optarg;
>>>                  break;
>>>              case QEMU_OPTION_sandbox:
>>> -#ifdef CONFIG_SECCOMP
>>> -                opts = qemu_opts_parse_noisily(qemu_find_opts("sandbox"),
>>> -                                               optarg, true);
>>> +                olist = qemu_find_opts("sandbox");
>>> +                if (!olist) {
>>> +#ifndef CONFIG_SECCOMP
>>
>> Why do you move the #ifdef? We have two separate error cases here.
>> And it seems better no to check for "-sandbox" when seccomp is disabled.
> 
> I tried to remove the #ifdef altogether to simplify the code, then
> realized the error message could be useful.
> 
> I don't think it's a problem to lookup "-sandbox" when seccomp is disabled.
> 

ok, so:

Reviewed-by: Laurent Vivier <lvivier@redhat.com>
Re: [Qemu-devel] [PATCH] vl: fix -sandbox parsing crash when seccomp support is disabled
Posted by Andrea Bolognani 6 years, 6 months ago 
On Mon, 2019-04-29 at 15:47 +0200, Marc-André Lureau wrote:
> $ ./x86_64-softmmu/qemu-system-x86_64 -sandbox off
> qemu-system-x86_64: -sandbox off: There is no option group 'sandbox'
> Segmentation fault
> 
> Commit 5780760f5e ("seccomp: check TSYNC host capability") 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
>     at util/qemu-option.c:829
>  #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
> 
> Fixes: 5780760f5ea6163939a5dabe7427318b4f07d1a2
> Cc: david@gibson.dropbear.id.au
> Cc: otubo@redhat.com
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  vl.c | 18 ++++++++++--------
>  1 file changed, 10 insertions(+), 8 deletions(-)

This fixes the crash for me, so

  Tested-by: Andrea Bolognani <abologna@redhat.com>

-- 
Andrea Bolognani / Red Hat / Virtualization
Re: [Qemu-devel] [PATCH] vl: fix -sandbox parsing crash when seccomp support is disabled
Posted by Paolo Bonzini 6 years, 6 months ago 
On 29/04/19 15:47, Marc-André Lureau wrote:
> $ ./x86_64-softmmu/qemu-system-x86_64 -sandbox off
> qemu-system-x86_64: -sandbox off: There is no option group 'sandbox'
> Segmentation fault
> 
> Commit 5780760f5e ("seccomp: check TSYNC host capability") 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
>     at util/qemu-option.c:829
>  #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
> 
> Fixes: 5780760f5ea6163939a5dabe7427318b4f07d1a2
> Cc: david@gibson.dropbear.id.au
> Cc: otubo@redhat.com
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  vl.c | 18 ++++++++++--------
>  1 file changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/vl.c b/vl.c
> index 4019a4387d..5fc4994d3c 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -3866,17 +3866,19 @@ int main(int argc, char **argv, char **envp)
>                  qtest_log = optarg;
>                  break;
>              case QEMU_OPTION_sandbox:
> -#ifdef CONFIG_SECCOMP
> -                opts = qemu_opts_parse_noisily(qemu_find_opts("sandbox"),
> -                                               optarg, true);
> +                olist = qemu_find_opts("sandbox");
> +                if (!olist) {
> +#ifndef CONFIG_SECCOMP
> +                    error_report("-sandbox support is not enabled "
> +                                 "in this QEMU binary");
> +#endif
> +                    exit(1);
> +                }
> +
> +                opts = qemu_opts_parse_noisily(olist, optarg, true);
>                  if (!opts) {
>                      exit(1);
>                  }
> -#else
> -                error_report("-sandbox support is not enabled "
> -                             "in this QEMU binary");
> -                exit(1);
> -#endif
>                  break;
>              case QEMU_OPTION_add_fd:
>  #ifndef _WIN32
> 

Queued, thanks.

Paolo
Re: [Qemu-devel] [PATCH] vl: fix -sandbox parsing crash when seccomp support is disabled
Posted by no-reply@patchew.org 6 years, 6 months ago 
Patchew URL: https://patchew.org/QEMU/20190429134757.13570-1-marcandre.lureau@redhat.com/



Hi,

This series failed the docker-mingw@fedora build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
time make docker-test-mingw@fedora SHOW_ENV=1 J=14 NETWORK=1
=== TEST SCRIPT END ===




The full log is available at
http://patchew.org/logs/20190429134757.13570-1-marcandre.lureau@redhat.com/testing.docker-mingw@fedora/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
Re: [Qemu-devel] [PATCH] vl: fix -sandbox parsing crash when seccomp support is disabled
Posted by no-reply@patchew.org 6 years, 6 months ago 
Patchew URL: https://patchew.org/QEMU/20190429134757.13570-1-marcandre.lureau@redhat.com/



Hi,

This series failed the asan build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
time make docker-test-debug@fedora TARGET_LIST=x86_64-softmmu J=14 NETWORK=1
=== TEST SCRIPT END ===




The full log is available at
http://patchew.org/logs/20190429134757.13570-1-marcandre.lureau@redhat.com/testing.asan/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.