qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] string-output-visitor: Fix (pseudo) struct handling
@ 2024-01-09 18:17 Kevin Wolf
  2024-01-11 11:45 ` Markus Armbruster
  2024-01-16 18:28 ` Stefan Hajnoczi
  0 siblings, 2 replies; 5+ messages in thread
From: Kevin Wolf @ 2024-01-09 18:17 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, aliang, stefanha, armbru, qemu-devel

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



^ permalink raw reply related	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-01-17 12:33 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-01-09 18:17 [PATCH] string-output-visitor: Fix (pseudo) struct handling Kevin Wolf
2024-01-11 11:45 ` Markus Armbruster
2024-01-15 15:10   ` Kevin Wolf
2024-01-17 12:32     ` Markus Armbruster
2024-01-16 18:28 ` Stefan Hajnoczi

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).