| Message ID | 20240304224133.267640-1-pbonzini@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | hw/intc/apic: fix memory leak | expand |
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>
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.
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>
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.
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)
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(-)