qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Kevin Wolf <kwolf@redhat.com>
To: qemu-block@nongnu.org
Cc: kwolf@redhat.com, aliang@redhat.com, stefanha@redhat.com,
	armbru@redhat.com, qemu-devel@nongnu.org
Subject: [PATCH] string-output-visitor: Fix (pseudo) struct handling
Date: Tue,  9 Jan 2024 19:17:17 +0100	[thread overview]
Message-ID: <20240109181717.42493-1-kwolf@redhat.com> (raw)

Commit ff32bb53 tried to get minimal struct support into the string
output visitor by just making it return "<omitted>". Unfortunately, it
forgot that the caller will still make more visitor calls for the
content of the struct.

If the struct is contained in a list, such as IOThreadVirtQueueMapping,
in the better case its fields show up as separate list entries. In the
worse case, it contains another list, and the string output visitor
doesn't support nested lists and asserts that this doesn't happen. So as
soon as the optional "vqs" field in IOThreadVirtQueueMapping is
specified, we get a crash.

This can be reproduced with the following command line:

  echo "info qtree" | ./qemu-system-x86_64 \
    -object iothread,id=t0 \
    -blockdev null-co,node-name=disk \
    -device '{"driver": "virtio-blk-pci", "drive": "disk",
              "iothread-vq-mapping": [{"iothread": "t0", "vqs": [0]}]}' \
    -monitor stdio

Fix the problem by counting the nesting level of structs and ignoring
any visitor calls for values (apart from start/end_struct) while we're
not on the top level.

Fixes: ff32bb53476539d352653f4ed56372dced73a388
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2069
Reported-by: Aihua Liang <aliang@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 qapi/string-output-visitor.c | 46 ++++++++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)

diff --git a/qapi/string-output-visitor.c b/qapi/string-output-visitor.c
index f0c1dea89e..5115536b15 100644
--- a/qapi/string-output-visitor.c
+++ b/qapi/string-output-visitor.c
@@ -65,6 +65,7 @@ struct StringOutputVisitor
     } range_start, range_end;
     GList *ranges;
     void *list; /* Only needed for sanity checking the caller */
+    unsigned int struct_nesting;
 };
 
 static StringOutputVisitor *to_sov(Visitor *v)
@@ -144,6 +145,10 @@ static bool print_type_int64(Visitor *v, const char *name, int64_t *obj,
     StringOutputVisitor *sov = to_sov(v);
     GList *l;
 
+    if (sov->struct_nesting) {
+        return true;
+    }
+
     switch (sov->list_mode) {
     case LM_NONE:
         string_output_append(sov, *obj);
@@ -231,6 +236,10 @@ static bool print_type_size(Visitor *v, const char *name, uint64_t *obj,
     uint64_t val;
     char *out, *psize;
 
+    if (sov->struct_nesting) {
+        return true;
+    }
+
     if (!sov->human) {
         out = g_strdup_printf("%"PRIu64, *obj);
         string_output_set(sov, out);
@@ -250,6 +259,11 @@ static bool print_type_bool(Visitor *v, const char *name, bool *obj,
                             Error **errp)
 {
     StringOutputVisitor *sov = to_sov(v);
+
+    if (sov->struct_nesting) {
+        return true;
+    }
+
     string_output_set(sov, g_strdup(*obj ? "true" : "false"));
     return true;
 }
@@ -260,6 +274,10 @@ static bool print_type_str(Visitor *v, const char *name, char **obj,
     StringOutputVisitor *sov = to_sov(v);
     char *out;
 
+    if (sov->struct_nesting) {
+        return true;
+    }
+
     if (sov->human) {
         out = *obj ? g_strdup_printf("\"%s\"", *obj) : g_strdup("<null>");
     } else {
@@ -273,6 +291,11 @@ static bool print_type_number(Visitor *v, const char *name, double *obj,
                               Error **errp)
 {
     StringOutputVisitor *sov = to_sov(v);
+
+    if (sov->struct_nesting) {
+        return true;
+    }
+
     string_output_set(sov, g_strdup_printf("%.17g", *obj));
     return true;
 }
@@ -283,6 +306,10 @@ static bool print_type_null(Visitor *v, const char *name, QNull **obj,
     StringOutputVisitor *sov = to_sov(v);
     char *out;
 
+    if (sov->struct_nesting) {
+        return true;
+    }
+
     if (sov->human) {
         out = g_strdup("<null>");
     } else {
@@ -295,6 +322,9 @@ static bool print_type_null(Visitor *v, const char *name, QNull **obj,
 static bool start_struct(Visitor *v, const char *name, void **obj,
                          size_t size, Error **errp)
 {
+    StringOutputVisitor *sov = to_sov(v);
+
+    sov->struct_nesting++;
     return true;
 }
 
@@ -302,6 +332,10 @@ static void end_struct(Visitor *v, void **obj)
 {
     StringOutputVisitor *sov = to_sov(v);
 
+    if (--sov->struct_nesting) {
+        return;
+    }
+
     /* TODO actually print struct fields */
     string_output_set(sov, g_strdup("<omitted>"));
 }
@@ -312,6 +346,10 @@ start_list(Visitor *v, const char *name, GenericList **list, size_t size,
 {
     StringOutputVisitor *sov = to_sov(v);
 
+    if (sov->struct_nesting) {
+        return true;
+    }
+
     /* we can't traverse a list in a list */
     assert(sov->list_mode == LM_NONE);
     /* We don't support visits without a list */
@@ -329,6 +367,10 @@ static GenericList *next_list(Visitor *v, GenericList *tail, size_t size)
     StringOutputVisitor *sov = to_sov(v);
     GenericList *ret = tail->next;
 
+    if (sov->struct_nesting) {
+        return ret;
+    }
+
     if (ret && !ret->next) {
         sov->list_mode = LM_END;
     }
@@ -339,6 +381,10 @@ static void end_list(Visitor *v, void **obj)
 {
     StringOutputVisitor *sov = to_sov(v);
 
+    if (sov->struct_nesting) {
+        return;
+    }
+
     assert(sov->list == obj);
     assert(sov->list_mode == LM_STARTED ||
            sov->list_mode == LM_END ||
-- 
2.43.0



             reply	other threads:[~2024-01-09 18:18 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-01-09 18:17 Kevin Wolf [this message]
2024-01-11 11:45 ` [PATCH] string-output-visitor: Fix (pseudo) struct handling Markus Armbruster
2024-01-15 15:10   ` Kevin Wolf
2024-01-17 12:32     ` Markus Armbruster
2024-01-16 18:28 ` 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=20240109181717.42493-1-kwolf@redhat.com \
    --to=kwolf@redhat.com \
    --cc=aliang@redhat.com \
    --cc=armbru@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@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).