Re: [PATCH] net/vhost-vdpa: fix memory leak in vhost_vdpa_get_max_queue_pairs()

[Stefano Garzarella <sgarzare@redhat.com>]

On Tue, Nov 02, 2021 at 05:05:21PM +0100, Philippe Mathieu-Daudé wrote:
>On 11/2/21 16:51, Stefano Garzarella wrote:
>> Use g_autofree to ensure that `config` is freed when
>> vhost_vdpa_get_max_queue_pairs() returns.
>>
>> Reported-by: Coverity (CID 1465228: RESOURCE_LEAK)
>> Fixes: 402378407d ("vhost-vdpa: multiqueue support")
>> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
>> ---
>>  net/vhost-vdpa.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
>> index 49ab322511..373b706b90 100644
>> --- a/net/vhost-vdpa.c
>> +++ b/net/vhost-vdpa.c
>> @@ -214,7 +214,7 @@ static NetClientState *net_vhost_vdpa_init(NetClientState *peer,
>>  static int vhost_vdpa_get_max_queue_pairs(int fd, int *has_cvq, Error **errp)
>>  {
>>      unsigned long config_size = offsetof(struct vhost_vdpa_config, buf);
>> -    struct vhost_vdpa_config *config;
>> +    g_autofree struct vhost_vdpa_config *config = NULL;
>>      __virtio16 *max_queue_pairs;
>>      uint64_t features;
>>      int ret;
>
>Eventually reducing the scope:

Yep, I thought the same, moving also `config_size`, but then I switched 
to the simplest patch possible.

>
>-- >8 --
>--- a/net/vhost-vdpa.c
>+++ b/net/vhost-vdpa.c
>@@ -214,7 +214,6 @@ static NetClientState
>*net_vhost_vdpa_init(NetClientState *peer,
> static int vhost_vdpa_get_max_queue_pairs(int fd, int *has_cvq, Error
>**errp)
> {
>     unsigned long config_size = offsetof(struct vhost_vdpa_config, buf);
>-    struct vhost_vdpa_config *config;
>     __virtio16 *max_queue_pairs;
>     uint64_t features;
>     int ret;
>@@ -232,6 +231,8 @@ static int vhost_vdpa_get_max_queue_pairs(int fd,
>int *has_cvq, Error **errp)
>     }
>
>     if (features & (1 << VIRTIO_NET_F_MQ)) {
>+        g_autofree struct vhost_vdpa_config *config = NULL;
>+
>         config = g_malloc0(config_size + sizeof(*max_queue_pairs));
>         config->off = offsetof(struct virtio_net_config,
>max_virtqueue_pairs);
>         config->len = sizeof(*max_queue_pairs);
>---
>
>Either ways:
>Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Thanks, let's see what Jason and Michael prefer.

Stefano