From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51681) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WhsAf-0005zJ-MQ for qemu-devel@nongnu.org; Tue, 06 May 2014 23:05:29 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WhsAa-0001ua-9H for qemu-devel@nongnu.org; Tue, 06 May 2014 23:05:24 -0400 Message-ID: <5369A2E2.3010604@redhat.com> Date: Tue, 06 May 2014 21:05:06 -0600 From: Eric Blake MIME-Version: 1.0 References: <1399422257-5912-1-git-send-email-pl@kamp.de> In-Reply-To: <1399422257-5912-1-git-send-email-pl@kamp.de> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="C6IEFUqd9DVLjeNUWF3qFGIU9oE5nFxuk" Subject: Re: [Qemu-devel] [PATCH] qapi: fix null pointer dereference on invalid parameter List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Lieven , qemu-devel@nongnu.org Cc: Markus Armbruster , lcapitulino@redhat.com, qemu-stable@nongnu.org, mdroth@linux.vnet.ibm.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --C6IEFUqd9DVLjeNUWF3qFGIU9oE5nFxuk Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable 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. >=20 > For example: > { "execute": "blockdev-add", > "arguments": { "options" : { "driver": "invalid-driver" } } } >=20 > CC: qemu-stable@nongnu.org > Signed-off-by: Peter Lieven > --- > 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 >=20 > 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 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. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --C6IEFUqd9DVLjeNUWF3qFGIU9oE5nFxuk Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJTaaLiAAoJEKeha0olJ0NqHgYH/A8z04XQVsAvPZMyEhTF7jpU LphzGTAmQzW/0KScf//1zNKIB0hAqsSPe2lUMaxVi0CFsnhhN+6seZQrJIucyzWY U+BugGLw9ANRQFy5NR4O+dzQ2dl+3TORjkoGDNYfJxyX0yoyWiyyTYuxIpnpdCJ1 v8zNmtUwQv+lfcVu+BVril19XOaNDSCfjLT+dr1UvXdO4zgPhZWS6yh1SydbO5en tzSN9BEoBT0HnJ87ayv4dAtE9y6PjPrzMqaYNMzuZF0KlOoGo8g6D523mu9XEQjR KWpi31cc45vyNP5HCkZep4ixcloEQzhtTDF2T+7o4BWmnbg5lTspFS2M7i0NLGg= =DxaC -----END PGP SIGNATURE----- --C6IEFUqd9DVLjeNUWF3qFGIU9oE5nFxuk--