qapi: add visit_start_union and visit_end_union

In some cases an input visitor might bail out on filling out a
struct for various reasons, such as missing fields when running
in strict mode. In the case of a QAPI Union type, this may lead
to cases where the .kind field which encodes the union type
is uninitialized. Subsequently, other visitors, such as the
dealloc visitor, may use this .kind value as if it were
initialized, leading to assumptions about the union type which
in this case may lead to segfaults. For example, freeing an
integer value.

However, we can generally rely on the fact that the always-present
.data void * field that we generate for these union types will
always be NULL in cases where .kind is uninitialized (at least,
there shouldn't be a reason where we'd do this purposefully).

So pass this information on to Visitor implementation via these
optional start_union/end_union interfaces so this information
can be used to guard against the situation above. We will make
use of this information in a subsequent patch for the dealloc
visitor.

Cc: [email protected]
Reported-by: Fam Zheng <[email protected]>
Suggested-by: Paolo Bonzini <[email protected]>
Reviewed-by: Paolo Bonzini <[email protected]>
Reviewed-by: Eric Blake <[email protected]>
Signed-off-by: Michael Roth <[email protected]>
Signed-off-by: Luiz Capitulino <[email protected]>

diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h
index ecc018319602ccc9021a226084c62e4bd2369a8b..09bb0fd40838518ead849739d77ed106012bce20 100644 (file)
--- a/include/qapi/visitor-impl.h
+++ b/include/qapi/visitor-impl.h
@@ -55,6 +55,8 @@ struct Visitor
     void (*type_int64)(Visitor *v, int64_t *obj, const char *name, Error **errp);
     /* visit_type_size() falls back to (*type_uint64)() if type_size is unset */
     void (*type_size)(Visitor *v, uint64_t *obj, const char *name, Error **errp);
+    bool (*start_union)(Visitor *v, bool data_present, Error **errp);
+    void (*end_union)(Visitor *v, bool data_present, Error **errp);
 };
 
 void input_type_enum(Visitor *v, int *obj, const char *strings[],

diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h
index 4a0178fa4623d1a0f7156fb367836015167db61a..5934f59ad833eafbfb056b36af0a07e4224c72fe 100644 (file)
--- a/include/qapi/visitor.h
+++ b/include/qapi/visitor.h
@@ -58,5 +58,7 @@ void visit_type_size(Visitor *v, uint64_t *obj, const char *name, Error **errp);
 void visit_type_bool(Visitor *v, bool *obj, const char *name, Error **errp);
 void visit_type_str(Visitor *v, char **obj, const char *name, Error **errp);
 void visit_type_number(Visitor *v, double *obj, const char *name, Error **errp);
+bool visit_start_union(Visitor *v, bool data_present, Error **errp);
+void visit_end_union(Visitor *v, bool data_present, Error **errp);
 
 #endif

diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
index 55f8d4068cbb689a9b5740a9eaab8a715aa438b7..b66b93ae2b0113c8080dfddfbe9bebfb8028bda9 100644 (file)
--- a/qapi/qapi-visit-core.c
+++ b/qapi/qapi-visit-core.c
@@ -58,6 +58,21 @@ void visit_end_list(Visitor *v, Error **errp)
     v->end_list(v, errp);
 }
 
+bool visit_start_union(Visitor *v, bool data_present, Error **errp)
+{
+    if (v->start_union) {
+        return v->start_union(v, data_present, errp);
+    }
+    return true;
+}
+
+void visit_end_union(Visitor *v, bool data_present, Error **errp)
+{
+    if (v->end_union) {
+        v->end_union(v, data_present, errp);
+    }
+}
+
 void visit_optional(Visitor *v, bool *present, const char *name,
                     Error **errp)
 {

diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index df9f7fb657b20cecf71e7d76b8f45437ae495137..8f845a2b2910963324310df4b52f880564d80483 100644 (file)
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -358,6 +358,9 @@ void visit_type_%(name)s(Visitor *m, %(name)s **obj, const char *name, Error **e
         if (err) {
             goto out_obj;
         }
+        if (!visit_start_union(m, !!(*obj)->data, &err) || err) {
+            goto out_obj;
+        }
         switch ((*obj)->kind) {
 ''',
                  disc_type = disc_type,
@@ -386,6 +389,9 @@ void visit_type_%(name)s(Visitor *m, %(name)s **obj, const char *name, Error **e
 out_obj:
         error_propagate(errp, err);
         err = NULL;
+        visit_end_union(m, !!(*obj)->data, &err);
+        error_propagate(errp, err);
+        err = NULL;
     }
     visit_end_struct(m, &err);
 out: