From 50571883f6dad098e1eecfb717ed509451a7a476 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Tue, 21 Nov 2023 18:34:15 +0100 Subject: [PATCH] qdev: Fix crash in array property getter Passing an uninitialised list to visit_start_list() happens to work for the QObject output visitor because it treats the pointer as an opaque value and never dereferences it, but the string output visitor expects a valid list to check if it has more than one element. The existing code crashes with the string output visitor if the uninitialised value is non-NULL. Passing an explicit NULL would fix the crash, but still result in wrong output. Rework get_prop_array() so that it conforms to the expectations that the string output visitor has. This includes building a real list first and using visit_next_list() to iterate it. Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1993 Reported-by: Thomas Huth Signed-off-by: Kevin Wolf Tested-by: Dan Hoffman Tested-by: Thomas Huth Signed-off-by: Stefan Hajnoczi Message-ID: <20231121173416.346610-2-kwolf@redhat.com> --- hw/core/qdev-properties.c | 33 ++++++++++++++++++++++++++------- 1 file changed, 26 insertions(+), 7 deletions(-) diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c index 91632f7be9..840006e953 100644 --- a/hw/core/qdev-properties.c +++ b/hw/core/qdev-properties.c @@ -689,23 +689,36 @@ static void get_prop_array(Object *obj, Visitor *v, const char *name, Property *prop = opaque; uint32_t *alenptr = object_field_prop_ptr(obj, prop); void **arrayptr = (void *)obj + prop->arrayoffset; - char *elem = *arrayptr; - GenericList *list; - const size_t list_elem_size = sizeof(*list) + prop->arrayfieldsize; + char *elemptr = *arrayptr; + ArrayElementList *list = NULL, *elem; + ArrayElementList **tail = &list; + const size_t size = sizeof(*list); int i; bool ok; - if (!visit_start_list(v, name, &list, list_elem_size, errp)) { + /* At least the string output visitor needs a real list */ + for (i = 0; i < *alenptr; i++) { + elem = g_new0(ArrayElementList, 1); + elem->value = elemptr; + elemptr += prop->arrayfieldsize; + + *tail = elem; + tail = &elem->next; + } + + if (!visit_start_list(v, name, (GenericList **) &list, size, errp)) { return; } - for (i = 0; i < *alenptr; i++) { - Property elem_prop = array_elem_prop(obj, prop, name, elem); + elem = list; + while (elem) { + Property elem_prop = array_elem_prop(obj, prop, name, elem->value); prop->arrayinfo->get(obj, v, NULL, &elem_prop, errp); if (*errp) { goto out_obj; } - elem += prop->arrayfieldsize; + elem = (ArrayElementList *) visit_next_list(v, (GenericList*) elem, + size); } /* visit_check_list() can only fail for input visitors */ @@ -714,6 +727,12 @@ static void get_prop_array(Object *obj, Visitor *v, const char *name, out_obj: visit_end_list(v, (void**) &list); + + while (list) { + elem = list; + list = elem->next; + g_free(elem); + } } static void default_prop_array(ObjectProperty *op, const Property *prop) -- 2.30.2