Peter Lieven <p...@kamp.de> writes:
> 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-sta...@nongnu.org
>>> Signed-off-by: Peter Lieven <p...@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 <ebl...@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.
My series doesn't address this problem, and I can in fact reproduce the
crash with it applied.
Your fix effectively reverts my commit 25a7017. Let's turn it into a
proper revert:
Revert "qapi: Clean up superfluous null check in qapi_dealloc_type_str()"
This reverts commit 25a7017555f1b4aeb543b5d323ff4afb8f9c5437.
Turns out the argument *can* be null: QEMU now 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-sta...@nongnu.org
Signed-off-by: Peter Lieven <p...@kamp.de>
Reviewed-by: Eric Blake <ebl...@redhat.com>
Reviewed-by: Markus Armbruster <arm...@redhat.com>
The deallocation visitor is more special than I (naively) thought...
