qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Michael Roth <mdroth@linux.vnet.ibm.com>
To: qemu-devel@nongnu.org
Cc: famz@redhat.com, armbru@redhat.com, qemu-stable@nongnu.org,
	lcapitulino@redhat.com, pbonzini@redhat.com
Subject: [Qemu-devel] [PATCH v3 1/4] qapi: add visit_start_union and visit_end_union
Date: Thu, 18 Sep 2014 15:36:40 -0500	[thread overview]
Message-ID: <1411072603-8984-2-git-send-email-mdroth@linux.vnet.ibm.com> (raw)
In-Reply-To: <1411072603-8984-1-git-send-email-mdroth@linux.vnet.ibm.com>

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: qemu-stable@nongnu.org
Reported-by: Fam Zheng <famz@redhat.com>
Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 include/qapi/visitor-impl.h |  2 ++
 include/qapi/visitor.h      |  2 ++
 qapi/qapi-visit-core.c      | 15 +++++++++++++++
 scripts/qapi-visit.py       |  6 ++++++
 4 files changed, 25 insertions(+)

diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h
index ecc0183..09bb0fd 100644
--- 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 4a0178f..5934f59 100644
--- 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 55f8d40..b66b93a 100644
--- 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 c129697..cfce31b 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -357,6 +357,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,
@@ -385,6 +388,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:
-- 
1.9.1

  reply	other threads:[~2014-09-18 20:37 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-18 20:36 [Qemu-devel] [PATCH v3 0/4] qapi: fix crash in dealloc visitor for union types Michael Roth
2014-09-18 20:36 ` Michael Roth [this message]
2014-09-18 20:36 ` [Qemu-devel] [PATCH v3 2/4] qapi: dealloc visitor, implement visit_start_union Michael Roth
2014-09-18 20:36 ` [Qemu-devel] [PATCH v3 3/4] tests: add QMP input visitor test for unions with no discriminator Michael Roth
2014-09-18 20:36 ` [Qemu-devel] [PATCH v3 4/4] qemu-iotests: Test missing "driver" key for blockdev-add Michael Roth
2014-09-19 17:52 ` [Qemu-devel] [PATCH v3 0/4] qapi: fix crash in dealloc visitor for union types Luiz Capitulino
2014-09-19 18:21   ` Michael Roth

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=1411072603-8984-2-git-send-email-mdroth@linux.vnet.ibm.com \
    --to=mdroth@linux.vnet.ibm.com \
    --cc=armbru@redhat.com \
    --cc=famz@redhat.com \
    --cc=lcapitulino@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-stable@nongnu.org \
    /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).