qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Fam Zheng <famz@redhat.com>
To: qemu-devel@nongnu.org
Cc: Kevin Wolf <kwolf@redhat.com>,
	Markus Armbruster <armbru@redhat.com>,
	Michael Roth <mdroth@linux.vnet.ibm.com>,
	Stefan Hajnoczi <stefanha@redhat.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Luiz Capitulino <lcapitulino@redhat.com>
Subject: [Qemu-devel] [PATCH] qapi: Fix crash with enum dealloc when kind is invalid
Date: Wed, 10 Sep 2014 20:30:39 +0800	[thread overview]
Message-ID: <1410352239-8705-1-git-send-email-famz@redhat.com> (raw)

We shouldn't do anything in the switch block in enum's visit_type_
function, when the enum data's ->kind is not valid at all. This happens
when the dealloc visitor is called, after qmp input visitor returned
error.

Now, the input visitor will set ->kind to <TYPE>_MAX if the value is not
found, so that in dealloc, the switch block knows to skip calling into
specific type's visiting functions.

The added test case would trigger SIGSEGV without this fix.

This crash is introduced since commit b1de5f43 (QMP: Add support for
Archipelago).

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 qapi/qapi-visit-core.c     | 12 +++++-------
 scripts/qapi-visit.py      |  6 ++++++
 tests/qemu-iotests/087     | 17 +++++++++++++++++
 tests/qemu-iotests/087.out | 13 +++++++++++++
 4 files changed, 41 insertions(+), 7 deletions(-)

diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
index 55f8d40..6c46e0e 100644
--- a/qapi/qapi-visit-core.c
+++ b/qapi/qapi-visit-core.c
@@ -276,23 +276,21 @@ void input_type_enum(Visitor *v, int *obj, const char *strings[],
 
     visit_type_str(v, &enum_str, name, &local_err);
     if (local_err) {
-        error_propagate(errp, local_err);
-        return;
+        enum_str = NULL;
     }
 
     while (strings[value] != NULL) {
-        if (strcmp(strings[value], enum_str) == 0) {
+        if (enum_str && strcmp(strings[value], enum_str) == 0) {
             break;
         }
         value++;
     }
 
-    if (strings[value] == NULL) {
-        error_set(errp, QERR_INVALID_PARAMETER, enum_str);
-        g_free(enum_str);
-        return;
+    if (!local_err && strings[value] == NULL) {
+        error_set(&local_err, QERR_INVALID_PARAMETER, enum_str);
     }
 
     g_free(enum_str);
     *obj = value;
+    error_propagate(errp, local_err);
 }
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index c129697..dad7561 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -379,6 +379,12 @@ void visit_type_%(name)s(Visitor *m, %(name)s **obj, const char *name, Error **e
                 c_name=c_fun(key))
 
     ret += mcgen('''
+        case %(enum_full_value)s:
+            break;
+''',
+                enum_full_value = generate_enum_full_value(disc_type, 'MAX'))
+
+    ret += mcgen('''
         default:
             abort();
         }
diff --git a/tests/qemu-iotests/087 b/tests/qemu-iotests/087
index 82c56b1..d7454d1 100755
--- a/tests/qemu-iotests/087
+++ b/tests/qemu-iotests/087
@@ -218,6 +218,23 @@ run_qemu <<EOF
 { "execute": "quit" }
 EOF
 
+echo
+echo === Missing driver ===
+echo
+
+_make_test_img -o encryption=on $size
+run_qemu -S <<EOF
+{ "execute": "qmp_capabilities" }
+{ "execute": "blockdev-add",
+  "arguments": {
+      "options": {
+        "id": "disk"
+      }
+    }
+  }
+{ "execute": "quit" }
+EOF
+
 # success, all done
 echo "*** done"
 rm -f $seq.full
diff --git a/tests/qemu-iotests/087.out b/tests/qemu-iotests/087.out
index 7fbee3f..f16bad0 100644
--- a/tests/qemu-iotests/087.out
+++ b/tests/qemu-iotests/087.out
@@ -64,4 +64,17 @@ QMP_VERSION
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "DEVICE_TRAY_MOVED", "data": {"device": "ide1-cd0", "tray-open": true}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "DEVICE_TRAY_MOVED", "data": {"device": "floppy0", "tray-open": true}}
 
+
+=== Missing driver ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 encryption=on 
+Testing: -S
+QMP_VERSION
+{"return": {}}
+{"error": {"class": "GenericError", "desc": "Invalid parameter type for 'driver', expected: string"}}
+{"return": {}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "SHUTDOWN"}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "DEVICE_TRAY_MOVED", "data": {"device": "ide1-cd0", "tray-open": true}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "DEVICE_TRAY_MOVED", "data": {"device": "floppy0", "tray-open": true}}
+
 *** done
-- 
1.9.3

             reply	other threads:[~2014-09-10 12:30 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-10 12:30 Fam Zheng [this message]
2014-09-10 13:01 ` [Qemu-devel] [PATCH] qapi: Fix crash with enum dealloc when kind is invalid Paolo Bonzini
2014-09-10 15:02   ` Fam Zheng
2014-09-10 15:32     ` Paolo Bonzini
2014-09-11  0:53       ` Fam Zheng
2014-09-11  4:17         ` Eric Blake
2014-09-11  4:38           ` Fam Zheng
2014-09-11 14:26             ` Michael Roth
2014-09-11 14:35               ` Paolo Bonzini
2014-09-11 23:02                 ` Michael Roth
2014-09-11  1:01 ` Michael Roth
2014-09-11  1:02 ` [Qemu-devel] [PATCH] tests: add QMP input visitor test for unions with no discriminator Michael Roth
2014-09-11  4:19   ` Eric Blake

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=1410352239-8705-1-git-send-email-famz@redhat.com \
    --to=famz@redhat.com \
    --cc=armbru@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=lcapitulino@redhat.com \
    --cc=mdroth@linux.vnet.ibm.com \
    --cc=pbonzini@redhat.com \
    --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).