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

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1697 bytes --]

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: qemu-stable@nongnu.org
> Signed-off-by: Peter Lieven <pl@kamp.de>
> ---
>  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 <eblake@redhat.com>

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.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]