igvm: various fixes

[PATCH 2/3] igvm: fix memory leak on failed memory region init

Posted by Luigi Leonardi 2 days, 15 hours ago

When memory_region_init_* fail, `igvm_pages` is not freed causing a leak.

Free `igvm_pages` in the error path.

Fixes: c1d466d267cf ("backends/igvm: Add IGVM loader and configuration")
Signed-off-by: Luigi Leonardi <leonardi@redhat.com>
---
 backends/igvm.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/backends/igvm.c b/backends/igvm.c
index 50f0d6fb9a7bacddce91f095b6e2710ecbc79c6f..c347d0c17e7ee744e450d42e677946abad8a7211 100644
--- a/backends/igvm.c
+++ b/backends/igvm.c
@@ -216,11 +216,13 @@ static void *qigvm_prepare_memory(QIgvm *ctx, uint64_t addr, uint64_t size,
             ctx->machine_state->cgs->require_guest_memfd) {
             if (!memory_region_init_ram_guest_memfd(igvm_pages, NULL,
                                                     region_name, size, errp)) {
+                g_free(igvm_pages);
                 return NULL;
             }
         } else {
             if (!memory_region_init_ram(igvm_pages, NULL, region_name, size,
                                         errp)) {
+                g_free(igvm_pages);
                 return NULL;
             }
         }

-- 
2.53.0

Re: [PATCH 2/3] igvm: fix memory leak on failed memory region init

Posted by Ani Sinha 1 day, 22 hours ago


> On 30 Mar 2026, at 6:13 PM, Luigi Leonardi <leonardi@redhat.com> wrote:
> 
> When memory_region_init_* fail, `igvm_pages` is not freed causing a leak.
> 
> Free `igvm_pages` in the error path.
> 
> Fixes: c1d466d267cf ("backends/igvm: Add IGVM loader and configuration")
> Signed-off-by: Luigi Leonardi <leonardi@redhat.com>
> ---
> backends/igvm.c | 2 ++
> 1 file changed, 2 insertions(+)
> 
> diff --git a/backends/igvm.c b/backends/igvm.c
> index 50f0d6fb9a7bacddce91f095b6e2710ecbc79c6f..c347d0c17e7ee744e450d42e677946abad8a7211 100644
> --- a/backends/igvm.c
> +++ b/backends/igvm.c
> @@ -216,11 +216,13 @@ static void *qigvm_prepare_memory(QIgvm *ctx, uint64_t addr, uint64_t size,
>             ctx->machine_state->cgs->require_guest_memfd) {
>             if (!memory_region_init_ram_guest_memfd(igvm_pages, NULL,
>                                                     region_name, size, errp)) {
> +                g_free(igvm_pages);

Why not use g_autofree? That way you do not need to explicitly free it.


>                 return NULL;
>             }
>         } else {
>             if (!memory_region_init_ram(igvm_pages, NULL, region_name, size,
>                                         errp)) {
> +                g_free(igvm_pages);
>                 return NULL;
>             }
>         }
> 
> -- 
> 2.53.0
>

Re: [PATCH 2/3] igvm: fix memory leak on failed memory region init

Posted by Gerd Hoffmann 1 day, 21 hours ago

On Tue, Mar 31, 2026 at 11:45:55AM +0530, Ani Sinha wrote:
> 
> 
> > On 30 Mar 2026, at 6:13 PM, Luigi Leonardi <leonardi@redhat.com> wrote:
> > 
> > When memory_region_init_* fail, `igvm_pages` is not freed causing a leak.
> > 
> > Free `igvm_pages` in the error path.
> > 
> > Fixes: c1d466d267cf ("backends/igvm: Add IGVM loader and configuration")
> > Signed-off-by: Luigi Leonardi <leonardi@redhat.com>
> > ---
> > backends/igvm.c | 2 ++
> > 1 file changed, 2 insertions(+)
> > 
> > diff --git a/backends/igvm.c b/backends/igvm.c
> > index 50f0d6fb9a7bacddce91f095b6e2710ecbc79c6f..c347d0c17e7ee744e450d42e677946abad8a7211 100644
> > --- a/backends/igvm.c
> > +++ b/backends/igvm.c
> > @@ -216,11 +216,13 @@ static void *qigvm_prepare_memory(QIgvm *ctx, uint64_t addr, uint64_t size,
> >             ctx->machine_state->cgs->require_guest_memfd) {
> >             if (!memory_region_init_ram_guest_memfd(igvm_pages, NULL,
> >                                                     region_name, size, errp)) {
> > +                g_free(igvm_pages);
> 
> Why not use g_autofree? That way you do not need to explicitly free it.

autofree does not work because it must be freed in the error case only.

take care,
  Gerd

Re: [PATCH 2/3] igvm: fix memory leak on failed memory region init

Posted by Ani Sinha 1 day, 21 hours ago


> On 31 Mar 2026, at 12:11 PM, Gerd Hoffmann <kraxel@redhat.com> wrote:
> 
> On Tue, Mar 31, 2026 at 11:45:55AM +0530, Ani Sinha wrote:
>> 
>> 
>>> On 30 Mar 2026, at 6:13 PM, Luigi Leonardi <leonardi@redhat.com> wrote:
>>> 
>>> When memory_region_init_* fail, `igvm_pages` is not freed causing a leak.
>>> 
>>> Free `igvm_pages` in the error path.
>>> 
>>> Fixes: c1d466d267cf ("backends/igvm: Add IGVM loader and configuration")
>>> Signed-off-by: Luigi Leonardi <leonardi@redhat.com>

Reviewed-by: Ani Sinha <anisinha@redhat.com>

>>> ---
>>> backends/igvm.c | 2 ++
>>> 1 file changed, 2 insertions(+)
>>> 
>>> diff --git a/backends/igvm.c b/backends/igvm.c
>>> index 50f0d6fb9a7bacddce91f095b6e2710ecbc79c6f..c347d0c17e7ee744e450d42e677946abad8a7211 100644
>>> --- a/backends/igvm.c
>>> +++ b/backends/igvm.c
>>> @@ -216,11 +216,13 @@ static void *qigvm_prepare_memory(QIgvm *ctx, uint64_t addr, uint64_t size,
>>>            ctx->machine_state->cgs->require_guest_memfd) {
>>>            if (!memory_region_init_ram_guest_memfd(igvm_pages, NULL,
>>>                                                    region_name, size, errp)) {
>>> +                g_free(igvm_pages);
>> 
>> Why not use g_autofree? That way you do not need to explicitly free it.
> 
> autofree does not work because it must be freed in the error case only.
> 
> take care,
>  Gerd
>

Re: [PATCH 2/3] igvm: fix memory leak on failed memory region init

Posted by Ani Sinha 1 day, 2 hours ago


> On 31 Mar 2026, at 12:37 PM, Ani Sinha <anisinha@redhat.com> wrote:
> 
> 
> 
>> On 31 Mar 2026, at 12:11 PM, Gerd Hoffmann <kraxel@redhat.com> wrote:
>> 
>> On Tue, Mar 31, 2026 at 11:45:55AM +0530, Ani Sinha wrote:
>>> 
>>> 
>>>> On 30 Mar 2026, at 6:13 PM, Luigi Leonardi <leonardi@redhat.com> wrote:
>>>> 
>>>> When memory_region_init_* fail, `igvm_pages` is not freed causing a leak.
>>>> 
>>>> Free `igvm_pages` in the error path.
>>>> 
>>>> Fixes: c1d466d267cf ("backends/igvm: Add IGVM loader and configuration")
>>>> Signed-off-by: Luigi Leonardi <leonardi@redhat.com>
> 
> Reviewed-by: Ani Sinha <anisinha@redhat.com>

Btw, I think this is 11.0 material, particularly since https://gitlab.com/qemu-project/qemu/-/commit/a85c5ed33fc203d5ab6f863cec00f6257251be76
got merged.


> 
>>>> ---
>>>> backends/igvm.c | 2 ++
>>>> 1 file changed, 2 insertions(+)
>>>> 
>>>> diff --git a/backends/igvm.c b/backends/igvm.c
>>>> index 50f0d6fb9a7bacddce91f095b6e2710ecbc79c6f..c347d0c17e7ee744e450d42e677946abad8a7211 100644
>>>> --- a/backends/igvm.c
>>>> +++ b/backends/igvm.c
>>>> @@ -216,11 +216,13 @@ static void *qigvm_prepare_memory(QIgvm *ctx, uint64_t addr, uint64_t size,
>>>>           ctx->machine_state->cgs->require_guest_memfd) {
>>>>           if (!memory_region_init_ram_guest_memfd(igvm_pages, NULL,
>>>>                                                   region_name, size, errp)) {
>>>> +                g_free(igvm_pages);
>>> 
>>> Why not use g_autofree? That way you do not need to explicitly free it.
>> 
>> autofree does not work because it must be freed in the error case only.
>> 
>> take care,
>> Gerd