[PATCH v2] hw/acpi: Fix GPtrArray memory leak in crs_range_merge

Li Zhijian via posted 1 patch 5 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20250613085110.111204-1-lizhijian@fujitsu.com
Maintainers: "Michael S. Tsirkin" <mst@redhat.com>, Igor Mammedov <imammedo@redhat.com>, Ani Sinha <anisinha@redhat.com>
hw/acpi/aml-build.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
[PATCH v2] hw/acpi: Fix GPtrArray memory leak in crs_range_merge
Posted by Li Zhijian via 5 months ago
This leak was detected by the valgrind.

The crs_range_merge() function unconditionally allocated a GPtrArray
'even when range->len was zero, causing an early return without freeing
the allocated array. This resulted in a memory leak when an empty range
was processed.

Instead of moving the allocation after the check (as previously attempted),
use g_autoptr for automatic cleanup. This ensures the array is freed even
on early returns, and also removes the need for the explicit free at the
end of the function.

Signed-off-by: Li Zhijian <lizhijian@fujitsu.com>
---

Cc: Daniel P. Berrangé <berrange@redhat.com>
Cc: Ani Sinha <anisinha@redhat.com>
Ani Sinha,
I didn't pick your Reviewed-by tag since V2 changes the proposal.
Please take another look.

V2:
- use g_autoptr for automatic cleanup # Suggested by Daniel
---
 hw/acpi/aml-build.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index f8f93a9f66c8..cb817a0f31f1 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -160,7 +160,7 @@ void crs_replace_with_free_ranges(GPtrArray *ranges,
  */
 static void crs_range_merge(GPtrArray *range)
 {
-    GPtrArray *tmp = g_ptr_array_new_with_free_func(crs_range_free);
+    g_autoptr(GPtrArray) tmp = g_ptr_array_new_with_free_func(crs_range_free);
     CrsRangeEntry *entry;
     uint64_t range_base, range_limit;
     int i;
@@ -191,7 +191,6 @@ static void crs_range_merge(GPtrArray *range)
         entry = g_ptr_array_index(tmp, i);
         crs_range_insert(range, entry->base, entry->limit);
     }
-    g_ptr_array_free(tmp, true);
 }
 
 static void
-- 
2.47.0


Re: [PATCH v2] hw/acpi: Fix GPtrArray memory leak in crs_range_merge
Posted by Daniel P. Berrangé 5 months ago
On Fri, Jun 13, 2025 at 04:51:10PM +0800, Li Zhijian wrote:
> This leak was detected by the valgrind.
> 
> The crs_range_merge() function unconditionally allocated a GPtrArray
> 'even when range->len was zero, causing an early return without freeing
> the allocated array. This resulted in a memory leak when an empty range
> was processed.
> 
> Instead of moving the allocation after the check (as previously attempted),
> use g_autoptr for automatic cleanup. This ensures the array is freed even
> on early returns, and also removes the need for the explicit free at the
> end of the function.
> 
> Signed-off-by: Li Zhijian <lizhijian@fujitsu.com>
> ---

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|


Re: [PATCH v2] hw/acpi: Fix GPtrArray memory leak in crs_range_merge
Posted by Ani Sinha 5 months ago

> On 13 Jun 2025, at 2:35 PM, Daniel P. Berrangé <berrange@redhat.com> wrote:
> 
> On Fri, Jun 13, 2025 at 04:51:10PM +0800, Li Zhijian wrote:
>> This leak was detected by the valgrind.
>> 
>> The crs_range_merge() function unconditionally allocated a GPtrArray
>> 'even when range->len was zero, causing an early return without freeing
>> the allocated array. This resulted in a memory leak when an empty range
>> was processed.
>> 
>> Instead of moving the allocation after the check (as previously attempted),
>> use g_autoptr for automatic cleanup. This ensures the array is freed even
>> on early returns, and also removes the need for the explicit free at the
>> end of the function.
>> 
>> Signed-off-by: Li Zhijian <lizhijian@fujitsu.com>
>> ---
> 
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

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

> 
> 
> With regards,
> Daniel
> -- 
> |: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org         -o-            https://fstop138.berrange.com :|
> |: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|