[PATCH] hw/intc/apic: fix memory leak

[PATCH] hw/intc/apic: fix memory leak

[Paolo Bonzini]

deliver_bitmask is allocated on the heap in apic_deliver(), but there
are many paths in the function that return before the corresponding
g_free() is reached.  Fix this by switching to g_autofree and, while at
it, also switch to g_new.  Do the same in apic_deliver_irq() as well
for consistency.

Fixes: b5ee0468e9d ("apic: add support for x2APIC mode", 2024-02-14)
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/intc/apic.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/hw/intc/apic.c b/hw/intc/apic.c
index 1d887d66b86..4186c57b34c 100644
--- a/hw/intc/apic.c
+++ b/hw/intc/apic.c
@@ -291,14 +291,13 @@ static void apic_deliver_irq(uint32_t dest, uint8_t dest_mode,
                              uint8_t delivery_mode, uint8_t vector_num,
                              uint8_t trigger_mode)
 {
-    uint32_t *deliver_bitmask = g_malloc(max_apic_words * sizeof(uint32_t));
+    g_autofree uint32_t *deliver_bitmask = g_new(uint32_t, max_apic_words);
 
     trace_apic_deliver_irq(dest, dest_mode, delivery_mode, vector_num,
                            trigger_mode);
 
     apic_get_delivery_bitmask(deliver_bitmask, dest, dest_mode);
     apic_bus_deliver(deliver_bitmask, delivery_mode, vector_num, trigger_mode);
-    g_free(deliver_bitmask);
 }
 
 bool is_x2apic_mode(DeviceState *dev)
@@ -662,7 +661,7 @@ static void apic_deliver(DeviceState *dev, uint32_t dest, uint8_t dest_mode,
     APICCommonState *s = APIC(dev);
     APICCommonState *apic_iter;
     uint32_t deliver_bitmask_size = max_apic_words * sizeof(uint32_t);
-    uint32_t *deliver_bitmask = g_malloc(deliver_bitmask_size);
+    g_autofree uint32_t *deliver_bitmask = g_new(uint32_t, max_apic_words);
     uint32_t current_apic_id;
 
     if (is_x2apic_mode(dev)) {
@@ -708,7 +707,6 @@ static void apic_deliver(DeviceState *dev, uint32_t dest, uint8_t dest_mode,
     }
 
     apic_bus_deliver(deliver_bitmask, delivery_mode, vector_num, trigger_mode);
-    g_free(deliver_bitmask);
 }
 
 static bool apic_check_pic(APICCommonState *s)
-- 
2.43.2

Re: [PATCH] hw/intc/apic: fix memory leak

[Philippe Mathieu-Daudé]

On 4/3/24 23:41, Paolo Bonzini wrote:
> deliver_bitmask is allocated on the heap in apic_deliver(), but there
> are many paths in the function that return before the corresponding
> g_free() is reached.  Fix this by switching to g_autofree and, while at
> it, also switch to g_new.  Do the same in apic_deliver_irq() as well
> for consistency.
> 
> Fixes: b5ee0468e9d ("apic: add support for x2APIC mode", 2024-02-14)
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>   hw/intc/apic.c | 6 ++----
>   1 file changed, 2 insertions(+), 4 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH] hw/intc/apic: fix memory leak

[Bui Quang Minh]

On 3/5/24 05:41, Paolo Bonzini wrote:
> deliver_bitmask is allocated on the heap in apic_deliver(), but there
> are many paths in the function that return before the corresponding
> g_free() is reached.  Fix this by switching to g_autofree and, while at
> it, also switch to g_new.  Do the same in apic_deliver_irq() as well
> for consistency.
> 
> Fixes: b5ee0468e9d ("apic: add support for x2APIC mode", 2024-02-14)
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>   hw/intc/apic.c | 6 ++----
>   1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/intc/apic.c b/hw/intc/apic.c
> index 1d887d66b86..4186c57b34c 100644
> --- a/hw/intc/apic.c
> +++ b/hw/intc/apic.c
> @@ -291,14 +291,13 @@ static void apic_deliver_irq(uint32_t dest, uint8_t dest_mode,
>                                uint8_t delivery_mode, uint8_t vector_num,
>                                uint8_t trigger_mode)
>   {
> -    uint32_t *deliver_bitmask = g_malloc(max_apic_words * sizeof(uint32_t));
> +    g_autofree uint32_t *deliver_bitmask = g_new(uint32_t, max_apic_words);
>   
>       trace_apic_deliver_irq(dest, dest_mode, delivery_mode, vector_num,
>                              trigger_mode);
>   
>       apic_get_delivery_bitmask(deliver_bitmask, dest, dest_mode);
>       apic_bus_deliver(deliver_bitmask, delivery_mode, vector_num, trigger_mode);
> -    g_free(deliver_bitmask);
>   }
>   
>   bool is_x2apic_mode(DeviceState *dev)
> @@ -662,7 +661,7 @@ static void apic_deliver(DeviceState *dev, uint32_t dest, uint8_t dest_mode,
>       APICCommonState *s = APIC(dev);
>       APICCommonState *apic_iter;
>       uint32_t deliver_bitmask_size = max_apic_words * sizeof(uint32_t);
> -    uint32_t *deliver_bitmask = g_malloc(deliver_bitmask_size);
> +    g_autofree uint32_t *deliver_bitmask = g_new(uint32_t, max_apic_words);
>       uint32_t current_apic_id;
>   
>       if (is_x2apic_mode(dev)) {
> @@ -708,7 +707,6 @@ static void apic_deliver(DeviceState *dev, uint32_t dest, uint8_t dest_mode,
>       }
>   
>       apic_bus_deliver(deliver_bitmask, delivery_mode, vector_num, trigger_mode);
> -    g_free(deliver_bitmask);
>   }
>   
>   static bool apic_check_pic(APICCommonState *s)

Reviewed-by: Bui Quang Minh <minhquangbui99@gmail.com>

Thanks,
Quang Minh.

Re: [PATCH] hw/intc/apic: fix memory leak

[Alex Bennée]

Paolo Bonzini <pbonzini@redhat.com> writes:

> deliver_bitmask is allocated on the heap in apic_deliver(), but there
> are many paths in the function that return before the corresponding
> g_free() is reached.  Fix this by switching to g_autofree and, while at
> it, also switch to g_new.  Do the same in apic_deliver_irq() as well
> for consistency.
>
> Fixes: b5ee0468e9d ("apic: add support for x2APIC mode", 2024-02-14)
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH] hw/intc/apic: fix memory leak

[Philippe Mathieu-Daudé]

On 4/3/24 23:41, Paolo Bonzini wrote:
> deliver_bitmask is allocated on the heap in apic_deliver(), but there
> are many paths in the function that return before the corresponding
> g_free() is reached.  Fix this by switching to g_autofree and, while at
> it, also switch to g_new.  Do the same in apic_deliver_irq() as well
> for consistency.
> 
> Fixes: b5ee0468e9d ("apic: add support for x2APIC mode", 2024-02-14)
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>   hw/intc/apic.c | 6 ++----
>   1 file changed, 2 insertions(+), 4 deletions(-)

Thanks, patch queued.