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

[PATCH] 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.

Fix this by moving the GPtrArray allocation after the empty range check,
ensuring memory is only allocated when actually needed.

Signed-off-by: Li Zhijian <lizhijian@fujitsu.com>
---
 hw/acpi/aml-build.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index f8f93a9f66c8..cf1999880119 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);
+    GPtrArray *tmp;
     CrsRangeEntry *entry;
     uint64_t range_base, range_limit;
     int i;
@@ -169,6 +169,7 @@ static void crs_range_merge(GPtrArray *range)
         return;
     }
 
+    tmp = g_ptr_array_new_with_free_func(crs_range_free);
     g_ptr_array_sort(range, crs_range_compare);
 
     entry = g_ptr_array_index(range, 0);
-- 
2.47.0

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

Posted by Daniel P. Berrangé 5 months ago

On Fri, Jun 13, 2025 at 12:40:02PM +0800, Li Zhijian via 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.
> 
> Fix this by moving the GPtrArray allocation after the empty range check,
> ensuring memory is only allocated when actually needed.
> 
> Signed-off-by: Li Zhijian <lizhijian@fujitsu.com>
> ---
>  hw/acpi/aml-build.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> index f8f93a9f66c8..cf1999880119 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);
> +    GPtrArray *tmp;

IMHO it would be better to change this to

  g_autoptr(GPtrArray) tmp = g_ptr....


and remove the existing manual g_ptr_array_free call. This guarantees
it will always be released in every code path.

>      CrsRangeEntry *entry;
>      uint64_t range_base, range_limit;
>      int i;
> @@ -169,6 +169,7 @@ static void crs_range_merge(GPtrArray *range)
>          return;
>      }
>  
> +    tmp = g_ptr_array_new_with_free_func(crs_range_free);
>      g_ptr_array_sort(range, crs_range_compare);
>  
>      entry = g_ptr_array_index(range, 0);
> -- 
> 2.47.0
> 
> 

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] hw/acpi: Fix GPtrArray memory leak in crs_range_merge

Posted by Zhijian Li (Fujitsu) via 5 months ago


On 13/06/2025 16:07, Daniel P. Berrangé wrote:
> On Fri, Jun 13, 2025 at 12:40:02PM +0800, Li Zhijian via 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.
>>
>> Fix this by moving the GPtrArray allocation after the empty range check,
>> ensuring memory is only allocated when actually needed.
>>
>> Signed-off-by: Li Zhijian <lizhijian@fujitsu.com>
>> ---
>>   hw/acpi/aml-build.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
>> index f8f93a9f66c8..cf1999880119 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);
>> +    GPtrArray *tmp;
> 
> IMHO it would be better to change this to
> 
>    g_autoptr(GPtrArray) tmp = g_ptr....
> 
> 
> and remove the existing manual g_ptr_array_free call. This guarantees
> it will always be released in every code path.


It sounds good to me, I will update it soon.

Thank


> 
>>       CrsRangeEntry *entry;
>>       uint64_t range_base, range_limit;
>>       int i;
>> @@ -169,6 +169,7 @@ static void crs_range_merge(GPtrArray *range)
>>           return;
>>       }
>>   
>> +    tmp = g_ptr_array_new_with_free_func(crs_range_free);
>>       g_ptr_array_sort(range, crs_range_compare);
>>   
>>       entry = g_ptr_array_index(range, 0);
>> -- 
>> 2.47.0
>>
>>
> 
> With regards,
> Daniel

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

Posted by Ani Sinha 5 months ago


> On 13 Jun 2025, at 10:10 AM, Li Zhijian <lizhijian@fujitsu.com> 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.
> 
> Fix this by moving the GPtrArray allocation after the empty range check,
> ensuring memory is only allocated when actually needed.

Thanks for the fix.

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

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

> ---
> hw/acpi/aml-build.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> index f8f93a9f66c8..cf1999880119 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);
> +    GPtrArray *tmp;
>     CrsRangeEntry *entry;
>     uint64_t range_base, range_limit;
>     int i;
> @@ -169,6 +169,7 @@ static void crs_range_merge(GPtrArray *range)
>         return;
>     }
> 
> +    tmp = g_ptr_array_new_with_free_func(crs_range_free);
>     g_ptr_array_sort(range, crs_range_compare);
> 
>     entry = g_ptr_array_index(range, 0);
> -- 
> 2.47.0
>