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

[Peter Lieven <pl@kamp.de>]

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: 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.
>


Markus, can you advise please.

Peter