Re: [PULL 18/31] block/core: add generic infrastructure for x-blockdev-amend qmp command

[Max Reitz <mreitz@redhat.com>]

[-- Attachment #1.1: Type: text/plain, Size: 1728 bytes --]

On 09.07.20 17:09, Peter Maydell wrote:
> On Mon, 6 Jul 2020 at 11:05, Max Reitz <mreitz@redhat.com> wrote:
>>
>> From: Maxim Levitsky <mlevitsk@redhat.com>
>>
>> blockdev-amend will be used similiar to blockdev-create
>> to allow on the fly changes of the structure of the format based block devices.
>>
>> Current plan is to first support encryption keyslot management for luks
>> based formats (raw and embedded in qcow2)
>>
>> Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
>> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
>> Message-Id: <20200608094030.670121-12-mlevitsk@redhat.com>
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
> 
> Hi; Coverity reports a possible issue with this function
> (CID 1430268):

Thanks for the heads-up, I’ve sent a patch (“block/amend: Check whether
the node exists”).

>> +void qmp_x_blockdev_amend(const char *job_id,
>> +                          const char *node_name,
>> +                          BlockdevAmendOptions *options,
>> +                          bool has_force,
>> +                          bool force,
>> +                          Error **errp)
>> +{
>> +    BlockdevAmendJob *s;
>> +    const char *fmt = BlockdevDriver_str(options->driver);
>> +    BlockDriver *drv = bdrv_find_format(fmt);
>> +    BlockDriverState *bs = bdrv_find_node(node_name);
> 
> bdrv_find_node() can return NULL (we check for this
> in almost all callsites)...
> 
>> +    if (bs->drv != drv) {
> 
> ...but here we dereference it unconditionally.
> 
>> +        error_setg(errp,
>> +                   "x-blockdev-amend doesn't support changing the block driver");
>> +        return;
>> +    }
> 
> thanks
> -- PMM
> 



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