qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH] qapi: fix null pointer dereference on invalid p


From: Peter Lieven
Subject: Re: [Qemu-devel] [PATCH] qapi: fix null pointer dereference on invalid parameter
Date: Wed, 07 May 2014 16:16:45 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.5.0

On 07.05.2014 05:05, Eric Blake wrote:
On 05/06/2014 06:24 PM, Peter Lieven wrote:
qemu segfaults if it receives an invalid parameter via a
qmp command instead of throwing an error.

For example:
{ "execute": "blockdev-add",
     "arguments": { "options" : { "driver": "invalid-driver" } } }

CC: address@hidden
Signed-off-by: Peter Lieven <address@hidden>
---
  qapi/qapi-dealloc-visitor.c |    4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)
Does this overlap with any of Markus' work? It emphasizes how bad it is
that our visitor interface callbacks are undocumented on what they can
be passed and are expected to return.

https://lists.gnu.org/archive/html/qemu-devel/2014-05/msg00225.html

diff --git a/qapi/qapi-dealloc-visitor.c b/qapi/qapi-dealloc-visitor.c
index d0ea118..dc53545 100644
--- a/qapi/qapi-dealloc-visitor.c
+++ b/qapi/qapi-dealloc-visitor.c
@@ -131,7 +131,9 @@ static void qapi_dealloc_end_list(Visitor *v, Error **errp)
  static void qapi_dealloc_type_str(Visitor *v, char **obj, const char *name,
                                    Error **errp)
  {
-    g_free(*obj);
+    if (obj) {
+        g_free(*obj);
+    }
  }
As for solving a crash,
Reviewed-by: Eric Blake <address@hidden>

But if Markus' cleanups solve the problem by guaranteeing that the
cleanup visitor is never passed a NULL obj, then this would be a dead
check.  I'm not familiar enough with the rest of the visitor code to
know whether the caller is at fault, or whether other callers or visitor
callbacks have the same bug.



Markus, can you advise please.

Peter



reply via email to

[Prev in Thread] Current Thread [Next in Thread]