[v1] kvm: fix memory leak on failure to read stats descriptors

[PATCH] kvm: fix memory leak on failure to read stats descriptors

Posted by Paolo Bonzini 1 year, 7 months ago

Reported by Coverity as CID 1490142.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 accel/kvm/kvm-all.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index 7c8ce18bdd..208b0c74e3 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -3962,6 +3962,7 @@ static StatsDescriptors *find_stats_descriptors(StatsTarget target, int stats_fd
                    size_desc * kvm_stats_header->num_desc, ret);
         g_free(descriptors);
         g_free(kvm_stats_desc);
+        g_free(kvm_stats_header);
         return NULL;
     }
     descriptors->kvm_stats_header = kvm_stats_header;
-- 
2.37.2

Re: [PATCH] kvm: fix memory leak on failure to read stats descriptors

Posted by Marc-André Lureau 1 year, 7 months ago

On Mon, Sep 5, 2022 at 2:32 PM Paolo Bonzini <pbonzini@redhat.com> wrote:

> Reported by Coverity as CID 1490142.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>


> ---
>  accel/kvm/kvm-all.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
> index 7c8ce18bdd..208b0c74e3 100644
> --- a/accel/kvm/kvm-all.c
> +++ b/accel/kvm/kvm-all.c
> @@ -3962,6 +3962,7 @@ static StatsDescriptors
> *find_stats_descriptors(StatsTarget target, int stats_fd
>                     size_desc * kvm_stats_header->num_desc, ret);
>          g_free(descriptors);
>          g_free(kvm_stats_desc);
> +        g_free(kvm_stats_header);
>          return NULL;
>      }
>      descriptors->kvm_stats_header = kvm_stats_header;
> --
> 2.37.2
>
>
>

-- 
Marc-André Lureau

Re: [PATCH] kvm: fix memory leak on failure to read stats descriptors

Posted by Richard Henderson 1 year, 7 months ago

On 9/5/22 11:06, Paolo Bonzini wrote:
> Reported by Coverity as CID 1490142.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>   accel/kvm/kvm-all.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
> index 7c8ce18bdd..208b0c74e3 100644
> --- a/accel/kvm/kvm-all.c
> +++ b/accel/kvm/kvm-all.c
> @@ -3962,6 +3962,7 @@ static StatsDescriptors *find_stats_descriptors(StatsTarget target, int stats_fd
>                      size_desc * kvm_stats_header->num_desc, ret);
>           g_free(descriptors);
>           g_free(kvm_stats_desc);
> +        g_free(kvm_stats_header);
>           return NULL;
>       }

kvm_stats_header should also be freed just above, in the other read failure if block.

Alternately, can StatsDescriptors be reorg'ed as

   typedef struct StatsDescriptors {

       const char *ident; /* cache key, currently the StatsTarget */

       struct kvm_stats_desc *kvm_stats_desc;

-     struct kvm_stats_header *kvm_stats_header;
+     struct kvm_stats_header kvm_stats_header;
       QTAILQ_ENTRY(StatsDescriptors) next;

   } StatsDescriptors;


since it appears that kvm_stats_header has the same lifetime as the descriptor, and is not 
variable sized, like kvm_stats_desc.


r~