[Qemu-devel] [PATCH 2/3] qemu-img: Use only string options in img_open_opts

Max Reitz posted 3 patches 7 years, 5 months ago
[Qemu-devel] [PATCH 2/3] qemu-img: Use only string options in img_open_opts
Posted by Max Reitz 7 years, 5 months ago
img_open_opts() takes a QemuOpts and converts them to a QDict, so all
values therein are strings.  Then it may try to call qdict_get_bool(),
however, which will fail with a segmentation fault every time:

$ ./qemu-img info -U --image-opts \
    driver=file,filename=/dev/null,force-share=off
[1]    27869 segmentation fault (core dumped)  ./qemu-img info -U
--image-opts driver=file,filename=/dev/null,force-share=off

Fix this by using qdict_get_str() and comparing the value as a string.
Also, when adding a force-share value to the QDict, add it as a string
so it fits the rest of the dict.

Cc: qemu-stable@nongnu.org
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 qemu-img.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index 855fa52514..42b60917b0 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -277,12 +277,12 @@ static BlockBackend *img_open_opts(const char *optstr,
     options = qemu_opts_to_qdict(opts, NULL);
     if (force_share) {
         if (qdict_haskey(options, BDRV_OPT_FORCE_SHARE)
-            && !qdict_get_bool(options, BDRV_OPT_FORCE_SHARE)) {
+            && strcmp(qdict_get_str(options, BDRV_OPT_FORCE_SHARE), "on")) {
             error_report("--force-share/-U conflicts with image options");
             QDECREF(options);
             return NULL;
         }
-        qdict_put_bool(options, BDRV_OPT_FORCE_SHARE, true);
+        qdict_put_str(options, BDRV_OPT_FORCE_SHARE, "on");
     }
     blk = blk_new_open(NULL, NULL, options, flags, &local_err);
     if (!blk) {
-- 
2.14.3


Re: [Qemu-devel] [PATCH 2/3] qemu-img: Use only string options in img_open_opts
Posted by Eric Blake 7 years, 5 months ago
On 05/02/2018 03:20 PM, Max Reitz wrote:
> img_open_opts() takes a QemuOpts and converts them to a QDict, so all
> values therein are strings.  Then it may try to call qdict_get_bool(),
> however, which will fail with a segmentation fault every time:

I have no idea if it's worth fixing qdict_get_bool() to at least not 
segfault when called on a non-bool Dict member (but what should it 
return, true or false? or should it abort() for at least a cleaner 
failure than a segfault?)

But in the meantime, your fix is correct.

> 
> $ ./qemu-img info -U --image-opts \
>      driver=file,filename=/dev/null,force-share=off
> [1]    27869 segmentation fault (core dumped)  ./qemu-img info -U
> --image-opts driver=file,filename=/dev/null,force-share=off
> 
> Fix this by using qdict_get_str() and comparing the value as a string.
> Also, when adding a force-share value to the QDict, add it as a string
> so it fits the rest of the dict.
> 
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>   qemu-img.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH 2/3] qemu-img: Use only string options in img_open_opts
Posted by Max Reitz 7 years, 5 months ago
On 2018-05-03 00:00, Eric Blake wrote:
> On 05/02/2018 03:20 PM, Max Reitz wrote:
>> img_open_opts() takes a QemuOpts and converts them to a QDict, so all
>> values therein are strings.  Then it may try to call qdict_get_bool(),
>> however, which will fail with a segmentation fault every time:
> 
> I have no idea if it's worth fixing qdict_get_bool() to at least not
> segfault when called on a non-bool Dict member (but what should it
> return, true or false? or should it abort() for at least a cleaner
> failure than a segfault?)

There's qdict_get_try_bool() which returns a default.  For testing
whether the member is a bool I suppose you can use
qdict_to(QBool, qdict_get(qdict, member)).

Max

> But in the meantime, your fix is correct.
> 
>>
>> $ ./qemu-img info -U --image-opts \
>>      driver=file,filename=/dev/null,force-share=off
>> [1]    27869 segmentation fault (core dumped)  ./qemu-img info -U
>> --image-opts driver=file,filename=/dev/null,force-share=off
>>
>> Fix this by using qdict_get_str() and comparing the value as a string.
>> Also, when adding a force-share value to the QDict, add it as a string
>> so it fits the rest of the dict.
>>
>> Cc: qemu-stable@nongnu.org
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>>   qemu-img.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
> 
> Reviewed-by: Eric Blake <eblake@redhat.com>
>