On Tue, Jan 09, 2024 at 07:17:17PM +0100, Kevin Wolf wrote:
> 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 <[email protected]>
> Signed-off-by: Kevin Wolf <[email protected]>
> ---
> qapi/string-output-visitor.c | 46 ++++++++++++++++++++++++++++++++++++
> 1 file changed, 46 insertions(+)Thanks for getting to this before I could: Reviewed-by: Stefan Hajnoczi <[email protected]> > 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 >
signature.asc
Description: PGP signature
