[PATCH] hw/arm/boot: Correctly free the MemoryDeviceInfoList

Peter Maydell posted 1 patch 3 weeks, 6 days ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20250901102214.3748011-1-peter.maydell@linaro.org
Maintainers: Peter Maydell <peter.maydell@linaro.org>
hw/arm/boot.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] hw/arm/boot: Correctly free the MemoryDeviceInfoList
Posted by Peter Maydell 3 weeks, 6 days ago
When running the bios-tables-test under ASAN we see leaks like this:

Direct leak of 16 byte(s) in 1 object(s) allocated from:
    #0 0x5bc58579b00d in calloc (/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/qemu-system-aarch64+0x250400d) (BuildId: 2e27b63dc9ac45f522ced40a17c2a60cc32f1d38)
    #1 0x7b4ad90337b1 in g_malloc0 (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x637b1) (BuildId: 1eb6131419edb83b2178b682829a6913cf682d75)
    #2 0x5bc5861826db in qmp_memory_device_list /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../hw/mem/memory-device.c:307:34
    #3 0x5bc587a9edb6 in arm_load_dtb /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../hw/arm/boot.c:656:15

Indirect leak of 28 byte(s) in 2 object(s) allocated from:
    #0 0x5bc58579ae23 in malloc (/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/qemu-system-aarch64+0x2503e23) (BuildId: 2e27b63dc9ac45f522ced40a17c2a60cc32f1d38)
    #1 0x7b4ad6c8f947 in __vasprintf_internal libio/vasprintf.c:116:16
    #2 0x7b4ad9080a52 in g_vasprintf (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0xb0a52) (BuildId: 1eb6131419edb83b2178b682829a6913cf682d75)
    #3 0x7b4ad90515e4 in g_strdup_vprintf (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x815e4) (BuildId: 1eb6131419edb83b2178b682829a6913cf682d75)
    #4 0x7b4ad9051940 in g_strdup_printf (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x81940) (BuildId: 1eb6131419edb83b2178b682829a6913cf682d75)
    #5 0x5bc5885eb739 in object_get_canonical_path /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../qom/object.c:2123:19
    #6 0x5bc58618dca8 in pc_dimm_md_fill_device_info /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../hw/mem/pc-dimm.c:268:18
    #7 0x5bc586182792 in qmp_memory_device_list /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../hw/mem/memory-device.c:310:9

This happens because we declared the MemoryDeviceInfoList *md_list
with g_autofree, which will free the direct memory with g_free() but
doesn't free all the other data structures referenced by it.  Instead
what we want is to declare the pointer with g_autoptr(), which will
automatically call the qapi_free_MemoryDeviceInfoList() cleanup
function when the variable goes out of scope.

Fixes: 36bc78aca83cfd ("hw/arm: add static NVDIMMs in device tree")
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/arm/boot.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index 1e57c4ab9ee..d0840308f5a 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -549,7 +549,7 @@ int arm_load_dtb(hwaddr addr, const struct arm_boot_info *binfo,
     unsigned int i;
     hwaddr mem_base, mem_len;
     char **node_path;
-    g_autofree MemoryDeviceInfoList *md_list = NULL;
+    g_autoptr(MemoryDeviceInfoList) md_list = NULL;
     Error *err = NULL;
 
     if (binfo->dtb_filename) {
-- 
2.43.0
Re: [PATCH] hw/arm/boot: Correctly free the MemoryDeviceInfoList
Posted by Philippe Mathieu-Daudé 3 weeks, 5 days ago
On 1/9/25 12:22, Peter Maydell wrote:
> When running the bios-tables-test under ASAN we see leaks like this:
> 
> Direct leak of 16 byte(s) in 1 object(s) allocated from:
>      #0 0x5bc58579b00d in calloc (/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/qemu-system-aarch64+0x250400d) (BuildId: 2e27b63dc9ac45f522ced40a17c2a60cc32f1d38)
>      #1 0x7b4ad90337b1 in g_malloc0 (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x637b1) (BuildId: 1eb6131419edb83b2178b682829a6913cf682d75)
>      #2 0x5bc5861826db in qmp_memory_device_list /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../hw/mem/memory-device.c:307:34
>      #3 0x5bc587a9edb6 in arm_load_dtb /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../hw/arm/boot.c:656:15
> 
> Indirect leak of 28 byte(s) in 2 object(s) allocated from:
>      #0 0x5bc58579ae23 in malloc (/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/qemu-system-aarch64+0x2503e23) (BuildId: 2e27b63dc9ac45f522ced40a17c2a60cc32f1d38)
>      #1 0x7b4ad6c8f947 in __vasprintf_internal libio/vasprintf.c:116:16
>      #2 0x7b4ad9080a52 in g_vasprintf (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0xb0a52) (BuildId: 1eb6131419edb83b2178b682829a6913cf682d75)
>      #3 0x7b4ad90515e4 in g_strdup_vprintf (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x815e4) (BuildId: 1eb6131419edb83b2178b682829a6913cf682d75)
>      #4 0x7b4ad9051940 in g_strdup_printf (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x81940) (BuildId: 1eb6131419edb83b2178b682829a6913cf682d75)
>      #5 0x5bc5885eb739 in object_get_canonical_path /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../qom/object.c:2123:19
>      #6 0x5bc58618dca8 in pc_dimm_md_fill_device_info /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../hw/mem/pc-dimm.c:268:18
>      #7 0x5bc586182792 in qmp_memory_device_list /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../hw/mem/memory-device.c:310:9
> 
> This happens because we declared the MemoryDeviceInfoList *md_list
> with g_autofree, which will free the direct memory with g_free() but
> doesn't free all the other data structures referenced by it.  Instead
> what we want is to declare the pointer with g_autoptr(), which will
> automatically call the qapi_free_MemoryDeviceInfoList() cleanup
> function when the variable goes out of scope.
> 
> Fixes: 36bc78aca83cfd ("hw/arm: add static NVDIMMs in device tree")
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   hw/arm/boot.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

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

And queued, thanks!

Re: [PATCH] hw/arm/boot: Correctly free the MemoryDeviceInfoList
Posted by Manos Pitsidianakis 3 weeks, 6 days ago
On Mon, Sep 1, 2025 at 1:22 PM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> When running the bios-tables-test under ASAN we see leaks like this:
>
> Direct leak of 16 byte(s) in 1 object(s) allocated from:
>     #0 0x5bc58579b00d in calloc (/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/qemu-system-aarch64+0x250400d) (BuildId: 2e27b63dc9ac45f522ced40a17c2a60cc32f1d38)
>     #1 0x7b4ad90337b1 in g_malloc0 (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x637b1) (BuildId: 1eb6131419edb83b2178b682829a6913cf682d75)
>     #2 0x5bc5861826db in qmp_memory_device_list /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../hw/mem/memory-device.c:307:34
>     #3 0x5bc587a9edb6 in arm_load_dtb /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../hw/arm/boot.c:656:15
>
> Indirect leak of 28 byte(s) in 2 object(s) allocated from:
>     #0 0x5bc58579ae23 in malloc (/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/qemu-system-aarch64+0x2503e23) (BuildId: 2e27b63dc9ac45f522ced40a17c2a60cc32f1d38)
>     #1 0x7b4ad6c8f947 in __vasprintf_internal libio/vasprintf.c:116:16
>     #2 0x7b4ad9080a52 in g_vasprintf (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0xb0a52) (BuildId: 1eb6131419edb83b2178b682829a6913cf682d75)
>     #3 0x7b4ad90515e4 in g_strdup_vprintf (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x815e4) (BuildId: 1eb6131419edb83b2178b682829a6913cf682d75)
>     #4 0x7b4ad9051940 in g_strdup_printf (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x81940) (BuildId: 1eb6131419edb83b2178b682829a6913cf682d75)
>     #5 0x5bc5885eb739 in object_get_canonical_path /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../qom/object.c:2123:19
>     #6 0x5bc58618dca8 in pc_dimm_md_fill_device_info /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../hw/mem/pc-dimm.c:268:18
>     #7 0x5bc586182792 in qmp_memory_device_list /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../hw/mem/memory-device.c:310:9
>
> This happens because we declared the MemoryDeviceInfoList *md_list
> with g_autofree, which will free the direct memory with g_free() but
> doesn't free all the other data structures referenced by it.  Instead
> what we want is to declare the pointer with g_autoptr(), which will
> automatically call the qapi_free_MemoryDeviceInfoList() cleanup
> function when the variable goes out of scope.
>
> Fixes: 36bc78aca83cfd ("hw/arm: add static NVDIMMs in device tree")
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---

Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>

Thanks,

>  hw/arm/boot.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/arm/boot.c b/hw/arm/boot.c
> index 1e57c4ab9ee..d0840308f5a 100644
> --- a/hw/arm/boot.c
> +++ b/hw/arm/boot.c
> @@ -549,7 +549,7 @@ int arm_load_dtb(hwaddr addr, const struct arm_boot_info *binfo,
>      unsigned int i;
>      hwaddr mem_base, mem_len;
>      char **node_path;
> -    g_autofree MemoryDeviceInfoList *md_list = NULL;
> +    g_autoptr(MemoryDeviceInfoList) md_list = NULL;
>      Error *err = NULL;
>
>      if (binfo->dtb_filename) {
> --
> 2.43.0
>