[v2,1/3] qom: Don't keep error value between object_property_parse() calls
diff mbox series

Message ID 20190110020259.8492-2-ehabkost@redhat.com
State New
Headers show
Series
  • Fix virtio-*(-non)-transitional crash on 2.6 machine-types
Related show

Commit Message

Eduardo Habkost Jan. 10, 2019, 2:02 a.m. UTC
When handling errp==NULL at object_apply_global_props(), we are
leaving the old error value in `err` after printing a warning.
This makes QEMU crash if two global properties generate warnings:

  $ echo device_add rtl8139 | qemu-system-x86_64 -monitor stdio -global rtl8139.xxx=yyy -global rtl8139.xxx=zzz
  warning: can't apply global rtl8139.xxx=yyy: Property '.xxx' not found
  qemu-system-x86_64: util/error.c:57: error_setv: Assertion `*errp == NULL' failed.
  Aborted (core dumped)

Fix that by making `err` go out of scope immediately after the
warn_report_err() call.

Fixes: 50545b2cc029 "qdev-props: call object_apply_global_props()"
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 qom/object.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Eric Blake Jan. 10, 2019, 2:43 a.m. UTC | #1
On 1/9/19 8:02 PM, Eduardo Habkost wrote:
> When handling errp==NULL at object_apply_global_props(), we are
> leaving the old error value in `err` after printing a warning.
> This makes QEMU crash if two global properties generate warnings:
> 
>   $ echo device_add rtl8139 | qemu-system-x86_64 -monitor stdio -global rtl8139.xxx=yyy -global rtl8139.xxx=zzz
>   warning: can't apply global rtl8139.xxx=yyy: Property '.xxx' not found
>   qemu-system-x86_64: util/error.c:57: error_setv: Assertion `*errp == NULL' failed.
>   Aborted (core dumped)
> 
> Fix that by making `err` go out of scope immediately after the
> warn_report_err() call.
> 
> Fixes: 50545b2cc029 "qdev-props: call object_apply_global_props()"
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  qom/object.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/qom/object.c b/qom/object.c
> index aa6f3a2a71..4e5226ca12 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -372,7 +372,6 @@ static void object_post_init_with_type(Object *obj, TypeImpl *ti)
>  
>  void object_apply_global_props(Object *obj, const GPtrArray *props, Error **errp)
>  {
> -    Error *err = NULL;

Could also have been fixed by leaving this line at this scope,...

>      int i;
>  
>      if (!props) {
> @@ -381,6 +380,7 @@ void object_apply_global_props(Object *obj, const GPtrArray *props, Error **errp
>  
>      for (i = 0; i < props->len; i++) {
>          GlobalProperty *p = g_ptr_array_index(props, i);
> +        Error *err = NULL;
>  
>          if (object_dynamic_cast(obj, p->driver) == NULL) {
>              continue;
> 

...and doing 'err = NULL;' after warn_report_err().  That is, it's not
the going out of scope that fixes it per se, but the fact that you
changed to resetting it to NULL on each loop invocation rather than
leaving it pointing at freed memory.  Whether you set to NULL by a
tighter scope initializer or by an assignment doesn't matter, so no need
to respin since your way works.

Reviewed-by: Eric Blake <eblake@redhat.com>
Marc-André Lureau Jan. 10, 2019, 8:30 a.m. UTC | #2
On Thu, Jan 10, 2019 at 6:04 AM Eduardo Habkost <ehabkost@redhat.com> wrote:
>
> When handling errp==NULL at object_apply_global_props(), we are
> leaving the old error value in `err` after printing a warning.
> This makes QEMU crash if two global properties generate warnings:
>
>   $ echo device_add rtl8139 | qemu-system-x86_64 -monitor stdio -global rtl8139.xxx=yyy -global rtl8139.xxx=zzz
>   warning: can't apply global rtl8139.xxx=yyy: Property '.xxx' not found
>   qemu-system-x86_64: util/error.c:57: error_setv: Assertion `*errp == NULL' failed.
>   Aborted (core dumped)
>
> Fix that by making `err` go out of scope immediately after the
> warn_report_err() call.
>
> Fixes: 50545b2cc029 "qdev-props: call object_apply_global_props()"
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

> ---
>  qom/object.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/qom/object.c b/qom/object.c
> index aa6f3a2a71..4e5226ca12 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -372,7 +372,6 @@ static void object_post_init_with_type(Object *obj, TypeImpl *ti)
>
>  void object_apply_global_props(Object *obj, const GPtrArray *props, Error **errp)
>  {
> -    Error *err = NULL;
>      int i;
>
>      if (!props) {
> @@ -381,6 +380,7 @@ void object_apply_global_props(Object *obj, const GPtrArray *props, Error **errp
>
>      for (i = 0; i < props->len; i++) {
>          GlobalProperty *p = g_ptr_array_index(props, i);
> +        Error *err = NULL;
>
>          if (object_dynamic_cast(obj, p->driver) == NULL) {
>              continue;
> --
> 2.18.0.rc1.1.g3f1ff2140
>
>
Cornelia Huck Jan. 10, 2019, 10:42 a.m. UTC | #3
On Thu, 10 Jan 2019 00:02:57 -0200
Eduardo Habkost <ehabkost@redhat.com> wrote:

> When handling errp==NULL at object_apply_global_props(), we are
> leaving the old error value in `err` after printing a warning.
> This makes QEMU crash if two global properties generate warnings:
> 
>   $ echo device_add rtl8139 | qemu-system-x86_64 -monitor stdio -global rtl8139.xxx=yyy -global rtl8139.xxx=zzz
>   warning: can't apply global rtl8139.xxx=yyy: Property '.xxx' not found
>   qemu-system-x86_64: util/error.c:57: error_setv: Assertion `*errp == NULL' failed.
>   Aborted (core dumped)
> 
> Fix that by making `err` go out of scope immediately after the
> warn_report_err() call.
> 
> Fixes: 50545b2cc029 "qdev-props: call object_apply_global_props()"
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  qom/object.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Cornelia Huck <cohuck@redhat.com>

Patch
diff mbox series

diff --git a/qom/object.c b/qom/object.c
index aa6f3a2a71..4e5226ca12 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -372,7 +372,6 @@  static void object_post_init_with_type(Object *obj, TypeImpl *ti)
 
 void object_apply_global_props(Object *obj, const GPtrArray *props, Error **errp)
 {
-    Error *err = NULL;
     int i;
 
     if (!props) {
@@ -381,6 +380,7 @@  void object_apply_global_props(Object *obj, const GPtrArray *props, Error **errp
 
     for (i = 0; i < props->len; i++) {
         GlobalProperty *p = g_ptr_array_index(props, i);
+        Error *err = NULL;
 
         if (object_dynamic_cast(obj, p->driver) == NULL) {
             continue;