qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Kevin Wolf <kwolf@redhat.com>
To: qemu-devel@nongnu.org
Cc: kwolf@redhat.com, thuth@redhat.com, armbru@redhat.com,
	philmd@linaro.org, peter.maydell@linaro.org
Subject: [PATCH for-8.2 1/2] qdev: Fix crash in array property getter
Date: Tue, 21 Nov 2023 18:34:15 +0100	[thread overview]
Message-ID: <20231121173416.346610-2-kwolf@redhat.com> (raw)
In-Reply-To: <20231121173416.346610-1-kwolf@redhat.com>

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 <thuth@redhat.com>
Signed-off-by: Kevin Wolf <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.42.0



  reply	other threads:[~2023-11-21 17:35 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-11-21 17:34 [PATCH for-8.2 0/2] qdev array property fixes Kevin Wolf
2023-11-21 17:34 ` Kevin Wolf [this message]
2023-11-24 18:06   ` [PATCH for-8.2 1/2] qdev: Fix crash in array property getter Philippe Mathieu-Daudé
2023-11-21 17:34 ` [PATCH for-8.2 2/2] string-output-visitor: Support lists for non-integer types Kevin Wolf
2023-11-30 13:11   ` Markus Armbruster
2023-11-30 13:21     ` Stefan Hajnoczi
2023-11-30 13:41       ` Markus Armbruster
2023-11-30 14:00     ` Kevin Wolf
2023-11-30 14:35       ` Markus Armbruster
2023-11-21 18:48 ` [PATCH for-8.2 0/2] qdev array property fixes Thomas Huth
2023-11-28 16:23 ` Stefan Hajnoczi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20231121173416.346610-2-kwolf@redhat.com \
    --to=kwolf@redhat.com \
    --cc=armbru@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=philmd@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=thuth@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).