| Message ID | 1453993346-26423-1-git-send-email-ehabkost@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Jan 28, 2016 at 01:02:26PM -0200, Eduardo Habkost wrote: > If the same GlobalProperty struct is registered twice, the list > entry gets corrupted, making tqe_next points to itself, and > qdev_prop_set_globals() gets stuck in a loop. The bug can be > easily reproduced by running: > > $ qemu-system-x86_64 -rtc-td-hack -rtc-td-hack > > Change global_props to use GList instead of queue.h, making the > code simpler and able to deal with properties being registered > twice. > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> > --- > hw/core/qdev-properties.c | 17 ++++++++++------- > include/hw/qdev-core.h | 1 - > 2 files changed, 10 insertions(+), 8 deletions(-) > > diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c > index aacad66..180e0da 100644 > --- a/hw/core/qdev-properties.c > +++ b/hw/core/qdev-properties.c > @@ -1,3 +1,5 @@ > +#include <glib.h> > + > #include "net/net.h" > #include "hw/qdev.h" > #include "qapi/qmp/qerror.h" > @@ -1009,12 +1011,11 @@ void qdev_prop_set_ptr(DeviceState *dev, const char *name, void *value) > *ptr = value; > } > > -static QTAILQ_HEAD(, GlobalProperty) global_props = > - QTAILQ_HEAD_INITIALIZER(global_props); > +static GList *global_props; > > void qdev_prop_register_global(GlobalProperty *prop) > { > - QTAILQ_INSERT_TAIL(&global_props, prop, next); > + global_props = g_list_append(global_props, prop); > } > > void qdev_prop_register_global_list(GlobalProperty *props) > @@ -1028,10 +1029,11 @@ void qdev_prop_register_global_list(GlobalProperty *props) > > int qdev_prop_check_globals(void) > { > - GlobalProperty *prop; > + GList *l; > int ret = 0; > > - QTAILQ_FOREACH(prop, &global_props, next) { > + for (l = global_props; l; l = l->next) { > + GlobalProperty *prop = l->data; > ObjectClass *oc; > DeviceClass *dc; > if (prop->used) { > @@ -1062,9 +1064,10 @@ int qdev_prop_check_globals(void) > static void qdev_prop_set_globals_for_type(DeviceState *dev, > const char *typename) > { > - GlobalProperty *prop; > + GList *l; > > - QTAILQ_FOREACH(prop, &global_props, next) { > + for (l = global_props; l; l = l->next) { > + GlobalProperty *prop = l->data; > Error *err = NULL; > > if (strcmp(typename, prop->driver) != 0) { > diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h > index abcdee8..1fdbaaa 100644 > --- a/include/hw/qdev-core.h > +++ b/include/hw/qdev-core.h > @@ -268,7 +268,6 @@ typedef struct GlobalProperty { > const char *value; > bool user_provided; > bool used; > - QTAILQ_ENTRY(GlobalProperty) next; > } GlobalProperty; > > /*** Board API. This should go away once we have a machine config file. ***/ > -- > 2.1.0
On Thu, Jan 28, 2016 at 07:01:12PM +0200, Michael S. Tsirkin wrote: > On Thu, Jan 28, 2016 at 01:02:26PM -0200, Eduardo Habkost wrote: > > If the same GlobalProperty struct is registered twice, the list > > entry gets corrupted, making tqe_next points to itself, and > > qdev_prop_set_globals() gets stuck in a loop. The bug can be > > easily reproduced by running: > > > > $ qemu-system-x86_64 -rtc-td-hack -rtc-td-hack > > > > Change global_props to use GList instead of queue.h, making the > > code simpler and able to deal with properties being registered > > twice. > > > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > > Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Thanks! It seems we don't have a maintainer for hw/core/qdev*. Who should merge this?
On Fri, Jan 29, 2016 at 12:15:12PM -0200, Eduardo Habkost wrote: > On Thu, Jan 28, 2016 at 07:01:12PM +0200, Michael S. Tsirkin wrote: > > On Thu, Jan 28, 2016 at 01:02:26PM -0200, Eduardo Habkost wrote: > > > If the same GlobalProperty struct is registered twice, the list > > > entry gets corrupted, making tqe_next points to itself, and > > > qdev_prop_set_globals() gets stuck in a loop. The bug can be > > > easily reproduced by running: > > > > > > $ qemu-system-x86_64 -rtc-td-hack -rtc-td-hack > > > > > > Change global_props to use GList instead of queue.h, making the > > > code simpler and able to deal with properties being registered > > > twice. > > > > > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > > > > Reviewed-by: Michael S. Tsirkin <mst@redhat.com> > > Thanks! It seems we don't have a maintainer for hw/core/qdev*. > Who should merge this? I've just noticed that this didn't get included. Andreas, is it OK if I merge this through the machine core tree?
On Thu, May 12, 2016 at 06:22:36PM +0200, Andreas Färber wrote: > Am 12.05.2016 um 14:26 schrieb Eduardo Habkost: > > On Fri, Jan 29, 2016 at 12:15:12PM -0200, Eduardo Habkost wrote: > >> On Thu, Jan 28, 2016 at 07:01:12PM +0200, Michael S. Tsirkin wrote: > >>> On Thu, Jan 28, 2016 at 01:02:26PM -0200, Eduardo Habkost wrote: > >>>> If the same GlobalProperty struct is registered twice, the list > >>>> entry gets corrupted, making tqe_next points to itself, and > >>>> qdev_prop_set_globals() gets stuck in a loop. The bug can be > >>>> easily reproduced by running: > >>>> > >>>> $ qemu-system-x86_64 -rtc-td-hack -rtc-td-hack > >>>> > >>>> Change global_props to use GList instead of queue.h, making the > >>>> code simpler and able to deal with properties being registered > >>>> twice. > >>>> > >>>> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > >>> > >>> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> > >> > >> Thanks! It seems we don't have a maintainer for hw/core/qdev*. > >> Who should merge this? > > > > I've just noticed that this didn't get included. Andreas, is it > > OK if I merge this through the machine core tree? > > In general I don't mind about those lists, but I still have the bus > refactoring sitting in my tree that didn't make it into 2.6 because > someone made last-minute changes to osdep.h that resulted in hundreds of > errors in my tree... If you make sure it doesn't conflict, feel free to > take it. Which branch should I look at, to ensure it doesn't conflict?
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c index aacad66..180e0da 100644 --- a/hw/core/qdev-properties.c +++ b/hw/core/qdev-properties.c @@ -1,3 +1,5 @@ +#include <glib.h> + #include "net/net.h" #include "hw/qdev.h" #include "qapi/qmp/qerror.h" @@ -1009,12 +1011,11 @@ void qdev_prop_set_ptr(DeviceState *dev, const char *name, void *value) *ptr = value; } -static QTAILQ_HEAD(, GlobalProperty) global_props = - QTAILQ_HEAD_INITIALIZER(global_props); +static GList *global_props; void qdev_prop_register_global(GlobalProperty *prop) { - QTAILQ_INSERT_TAIL(&global_props, prop, next); + global_props = g_list_append(global_props, prop); } void qdev_prop_register_global_list(GlobalProperty *props) @@ -1028,10 +1029,11 @@ void qdev_prop_register_global_list(GlobalProperty *props) int qdev_prop_check_globals(void) { - GlobalProperty *prop; + GList *l; int ret = 0; - QTAILQ_FOREACH(prop, &global_props, next) { + for (l = global_props; l; l = l->next) { + GlobalProperty *prop = l->data; ObjectClass *oc; DeviceClass *dc; if (prop->used) { @@ -1062,9 +1064,10 @@ int qdev_prop_check_globals(void) static void qdev_prop_set_globals_for_type(DeviceState *dev, const char *typename) { - GlobalProperty *prop; + GList *l; - QTAILQ_FOREACH(prop, &global_props, next) { + for (l = global_props; l; l = l->next) { + GlobalProperty *prop = l->data; Error *err = NULL; if (strcmp(typename, prop->driver) != 0) { diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h index abcdee8..1fdbaaa 100644 --- a/include/hw/qdev-core.h +++ b/include/hw/qdev-core.h @@ -268,7 +268,6 @@ typedef struct GlobalProperty { const char *value; bool user_provided; bool used; - QTAILQ_ENTRY(GlobalProperty) next; } GlobalProperty; /*** Board API. This should go away once we have a machine config file. ***/
If the same GlobalProperty struct is registered twice, the list entry gets corrupted, making tqe_next points to itself, and qdev_prop_set_globals() gets stuck in a loop. The bug can be easily reproduced by running: $ qemu-system-x86_64 -rtc-td-hack -rtc-td-hack Change global_props to use GList instead of queue.h, making the code simpler and able to deal with properties being registered twice. Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- hw/core/qdev-properties.c | 17 ++++++++++------- include/hw/qdev-core.h | 1 - 2 files changed, 10 insertions(+), 8 deletions(-)