[PATCH-for-6.2 v2] qom/object: Ignore global properties with empty name

Philippe Mathieu-Daudé posted 1 patch 2 years, 5 months ago
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20211119122911.365036-1-philmd@redhat.com
There is a newer version of this series
qom/object.c | 3 +++
1 file changed, 3 insertions(+)
[PATCH-for-6.2 v2] qom/object: Ignore global properties with empty name
Posted by Philippe Mathieu-Daudé 2 years, 5 months ago
When using -global, properties might have empty name/value.

This fixes this legitimate use case:

  $ qemu-system-x86_64 -global driver=isa-fdc
  qemu-system-x86_64: ../../devel/qemu/qapi/string-input-visitor.c:394:
  string_input_visitor_new: Assertion `str' failed.
  Aborted (core dumped)

  (gdb) bt
  #4  0x5f6b8d5 in string_input_visitor_new (str=0x0) at qapi/string-input-visitor.c:394
  #5  0x5dd0f8d in object_property_parse (obj=0x6f33400, name=0x0, string=0x0, errp=0x7ffc9c8) at qom/object.c:1641
  #6  0x5dce131 in object_apply_global_props (obj=0x6f33400, props=0x6737360, errp=0x6611760 <error_fatal>) at qom/object.c:411
  #7  0x5dc5ee2 in qdev_prop_set_globals (dev=0x6f33400) at hw/core/qdev-properties.c:790
  #8  0x5dc89e8 in device_post_init (obj=0x6f33400) at hw/core/qdev.c:697
  #9  0x5dce02b in object_post_init_with_type (obj=0x6f33400, ti=0x672bd20) at qom/object.c:383
  #10 0x5dce059 in object_post_init_with_type (obj=0x6f33400, ti=0x66e9090) at qom/object.c:387
  #11 0x5dce059 in object_post_init_with_type (obj=0x6f33400, ti=0x66df730) at qom/object.c:387
  #12 0x5dce566 in object_initialize_with_type (obj=0x6f33400, size=848, type=0x66df730) at qom/object.c:519
  #13 0x5dcec78 in object_new_with_type (type=0x66df730) at qom/object.c:733
  #14 0x5dceccf in object_new (typename=0x60fcf81 "isa-fdc") at qom/object.c:748
  #15 0x5dc75fe in qdev_new (name=0x60fcf81 "isa-fdc") at hw/core/qdev.c:153
  #16 0x59eec58 in isa_new (name=0x60fcf81 "isa-fdc") at hw/isa/isa-bus.c:166
  #17 0x5bd3607 in pc_superio_init (isa_bus=0x66b42e0, create_fdctrl=true, no_vmport=false) at hw/i386/pc.c:1026
  (gdb) fr 6
  #6  0x5dce131 in object_apply_global_props (obj=0x6f33400, props=0x6737360, errp=0x6611760 <error_fatal>) at qom/object.c:411
  411             if (!object_property_parse(obj, p->property, p->value, &err)) {
  (gdb) p *p
  $1 = {driver = 0x6738250 "isa-fdc", property = 0x0, value = 0x0, used = true, optional = false}

Reported-by: Thomas Huth <thuth@redhat.com>
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/604
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
v2: s/55555555// for readability
---
 qom/object.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/qom/object.c b/qom/object.c
index 4f0677cca9e..45fa8561df6 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -401,6 +401,9 @@ bool object_apply_global_props(Object *obj, const GPtrArray *props,
         GlobalProperty *p = g_ptr_array_index(props, i);
         Error *err = NULL;
 
+        if (!p->property) {
+            continue;
+        }
         if (object_dynamic_cast(obj, p->driver) == NULL) {
             continue;
         }
-- 
2.31.1

Re: [PATCH-for-6.2 v2] qom/object: Ignore global properties with empty name
Posted by Markus Armbruster 2 years, 5 months ago
Philippe Mathieu-Daudé <philmd@redhat.com> writes:

> When using -global, properties might have empty name/value.
>
> This fixes this legitimate use case:
>
>   $ qemu-system-x86_64 -global driver=isa-fdc
>   qemu-system-x86_64: ../../devel/qemu/qapi/string-input-visitor.c:394:
>   string_input_visitor_new: Assertion `str' failed.
>   Aborted (core dumped)
>
>   (gdb) bt
>   #4  0x5f6b8d5 in string_input_visitor_new (str=0x0) at qapi/string-input-visitor.c:394
>   #5  0x5dd0f8d in object_property_parse (obj=0x6f33400, name=0x0, string=0x0, errp=0x7ffc9c8) at qom/object.c:1641
>   #6  0x5dce131 in object_apply_global_props (obj=0x6f33400, props=0x6737360, errp=0x6611760 <error_fatal>) at qom/object.c:411
>   #7  0x5dc5ee2 in qdev_prop_set_globals (dev=0x6f33400) at hw/core/qdev-properties.c:790
>   #8  0x5dc89e8 in device_post_init (obj=0x6f33400) at hw/core/qdev.c:697
>   #9  0x5dce02b in object_post_init_with_type (obj=0x6f33400, ti=0x672bd20) at qom/object.c:383
>   #10 0x5dce059 in object_post_init_with_type (obj=0x6f33400, ti=0x66e9090) at qom/object.c:387
>   #11 0x5dce059 in object_post_init_with_type (obj=0x6f33400, ti=0x66df730) at qom/object.c:387
>   #12 0x5dce566 in object_initialize_with_type (obj=0x6f33400, size=848, type=0x66df730) at qom/object.c:519
>   #13 0x5dcec78 in object_new_with_type (type=0x66df730) at qom/object.c:733
>   #14 0x5dceccf in object_new (typename=0x60fcf81 "isa-fdc") at qom/object.c:748
>   #15 0x5dc75fe in qdev_new (name=0x60fcf81 "isa-fdc") at hw/core/qdev.c:153
>   #16 0x59eec58 in isa_new (name=0x60fcf81 "isa-fdc") at hw/isa/isa-bus.c:166
>   #17 0x5bd3607 in pc_superio_init (isa_bus=0x66b42e0, create_fdctrl=true, no_vmport=false) at hw/i386/pc.c:1026
>   (gdb) fr 6
>   #6  0x5dce131 in object_apply_global_props (obj=0x6f33400, props=0x6737360, errp=0x6611760 <error_fatal>) at qom/object.c:411
>   411             if (!object_property_parse(obj, p->property, p->value, &err)) {
>   (gdb) p *p
>   $1 = {driver = 0x6738250 "isa-fdc", property = 0x0, value = 0x0, used = true, optional = false}
>
> Reported-by: Thomas Huth <thuth@redhat.com>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/604
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> v2: s/55555555// for readability
> ---
>  qom/object.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/qom/object.c b/qom/object.c
> index 4f0677cca9e..45fa8561df6 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -401,6 +401,9 @@ bool object_apply_global_props(Object *obj, const GPtrArray *props,
>          GlobalProperty *p = g_ptr_array_index(props, i);
>          Error *err = NULL;
>  
> +        if (!p->property) {
> +            continue;
> +        }
>          if (object_dynamic_cast(obj, p->driver) == NULL) {
>              continue;
>          }

Not a complete fix:

    $ qemu-system-x86_64 --global property=prop
    qemu-system-x86_64: warning: global (null).prop has invalid class name

    $ qemu-system-x86_64 --global value=val
    qemu-system-x86_64: warning: global (null).(null) has invalid class name

Glibc's printf() formats null pointers as "(null)", but some other
systems crash.

I think you should patch qemu_global_option() to require all three
parameters in the QemuOpts case.