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

[Philippe Mathieu-Daudé]

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;
>      }
>  
>