Re: [RFC PATCH] hw/display/virtio-gpu: Fix memory leak (CID 1453811)

[Philippe Mathieu-Daudé]

ping?

On 6/14/21 12:52 PM, Philippe Mathieu-Daudé wrote:
> ping?
> 
> On 5/31/21 12:19 PM, Philippe Mathieu-Daudé wrote:
>> To avoid leaking memory on the error path, reorder the
>> code as:
>> - check the parameters first
>> - check resource already existing
>> - finally allocate memory
>>
>> Reported-by: Coverity (CID 1453811: RESOURCE_LEAK)
>> Fixes: e0933d91b1c ("virtio-gpu: Add virtio_gpu_resource_create_blob")
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> ---
>> RFC because the s->iov check is dubious.
>> ---
>>  hw/display/virtio-gpu.c | 28 +++++++++++-----------------
>>  1 file changed, 11 insertions(+), 17 deletions(-)
>>
>> diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
>> index 4d549377cbc..8d047007bbb 100644
>> --- a/hw/display/virtio-gpu.c
>> +++ b/hw/display/virtio-gpu.c
>> @@ -340,8 +340,15 @@ static void virtio_gpu_resource_create_blob(VirtIOGPU 
>> *g,
>>          return;
>>      }
>>  
>> -    res = virtio_gpu_find_resource(g, cblob.resource_id);
>> -    if (res) {
>> +    if (cblob.blob_mem != VIRTIO_GPU_BLOB_MEM_GUEST &&
>> +        cblob.blob_flags != VIRTIO_GPU_BLOB_FLAG_USE_SHAREABLE) {
>> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: invalid memory type\n",
>> +                      __func__);
>> +        cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_PARAMETER;
>> +        return;
>> +    }
>> +
>> +    if (virtio_gpu_find_resource(g, cblob.resource_id)) {
>>          qemu_log_mask(LOG_GUEST_ERROR, "%s: resource already exists %d\n",
>>                        __func__, cblob.resource_id);
>>          cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
>> @@ -352,25 +359,12 @@ static void virtio_gpu_resource_create_blob(VirtIOGPU 
>> *g,
>>      res->resource_id = cblob.resource_id;
>>      res->blob_size = cblob.size;
>>  
>> -    if (cblob.blob_mem != VIRTIO_GPU_BLOB_MEM_GUEST &&
>> -        cblob.blob_flags != VIRTIO_GPU_BLOB_FLAG_USE_SHAREABLE) {
>> -        qemu_log_mask(LOG_GUEST_ERROR, "%s: invalid memory type\n",
>> -                      __func__);
>> -        cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_PARAMETER;
>> -        g_free(res);
>> -        return;
>> -    }
>> -
>> -    if (res->iov) {
>> -        cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
>> -        return;
>> -    }
>> -
>>      ret = virtio_gpu_create_mapping_iov(g, cblob.nr_entries, sizeof(cblob),
>>                                          cmd, &res->addrs, &res->iov,
>>                                          &res->iov_cnt);
>> -    if (ret != 0) {
>> +    if (ret != 0 || res->iov) {
>>          cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
>> +        g_free(res);
>>          return;
>>      }
>>  
>>
>