| Message ID | 20190110020259.8492-2-ehabkost@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | Fix virtio-*(-non)-transitional crash on 2.6 machine-types | expand |
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>
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 > >
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>
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;
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(-)