[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH for-5.0] xen-block: Fix uninitialized variable
From: |
Markus Armbruster |
Subject: |
Re: [PATCH for-5.0] xen-block: Fix uninitialized variable |
Date: |
Tue, 07 Apr 2020 07:27:13 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) |
Anthony PERARD <address@hidden> writes:
> On Mon, Apr 06, 2020 at 06:50:41PM +0200, Philippe Mathieu-Daudé wrote:
>> On 4/6/20 6:42 PM, Anthony PERARD wrote:
>> > Since 7f5d9b206d1e ("object-add: don't create return value if
>> > failed"), qmp_object_add() don't write any value in 'ret_data', thus
>> > has random data. Then qobject_unref() fails and abort().
>> >
>> > Fix by initialising 'ret_data' properly.
>>
>> Or move qobject_unref() after the error check?
>>
>> -- >8 --
>> diff --git a/hw/block/xen-block.c b/hw/block/xen-block.c
>> index 07bb32e22b..f3f1cbef65 100644
>> --- a/hw/block/xen-block.c
>> +++ b/hw/block/xen-block.c
>> @@ -869,7 +869,6 @@ static XenBlockIOThread *xen_block_iothread_create(const
>> char *id,
>> qdict_put_str(opts, "id", id);
>> qmp_object_add(opts, &ret_data, &local_err);
>> qobject_unref(opts);
>> - qobject_unref(ret_data);
>>
>> if (local_err) {
>> error_propagate(errp, local_err);
>> @@ -878,6 +877,7 @@ static XenBlockIOThread *xen_block_iothread_create(const
>> char *id,
>> g_free(iothread);
>> return NULL;
>> }
>> + qobject_unref(ret_data);
>
> That won't help, qmp_object_add() doesn't change the value of ret_data
> at all. The other users of qmp_object_add() passes an initialised
> 'ret_data', so we should do the same I think.
Since the QMP core does it, other callers should do it, too.
For QAPI commands that don't return anything, the marshaller should not
use @ret_data, let alone store through it. qmp_object_add() complies.
Thus, assert(!ret_data) would do. qobject_unref(ret_data) is also
correct.
Reviewed-by: Markus Armbruster <address@hidden>