hw: fix some leaks in xlnx devices

[PATCH 1/2] hw/misc/xlnx-versal-cframe-reg: Free FIFO, g_tree on deinit

Posted by Peter Maydell 5 months, 2 weeks ago

In the xlnx-versal-cframe-reg device we create a FIFO in
instance_init but don't destroy it on deinit, causing ASAN
to report a leak in the device-introspect-test:

Direct leak of 400 byte(s) in 1 object(s) allocated from:
    #0 0x5aded4d54e23 in malloc (/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/qemu-system-aarch64+0x24ffe23) (BuildId: 9f1e6c53fecd904ba5fc1f521d7da080a0e4103b)
    #1 0x71fbfac9bb09 in g_malloc (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x62b09) (BuildId: 1eb6131419edb83b2178b682829a6913cf682d75)
    #2 0x5aded850059d in fifo8_create /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../util/fifo8.c:27:18
    #3 0x5aded582b9e4 in fifo32_create /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/include/qemu/fifo32.h:35:5
    #4 0x5aded582b326 in cframe_reg_init /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../hw/misc/xlnx-versal-cframe-reg.c:693:5

Similarly, we don't clean up the g_tree we create:
Direct leak of 48 byte(s) in 1 object(s) allocated from:
    #0 0x5aded4d54e23 in malloc (/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/qemu-system-aarch64+0x24ffe23) (BuildId: 9f1e6c5
3fecd904ba5fc1f521d7da080a0e4103b)
    #1 0x71fbfac9bb09 in g_malloc (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x62b09) (BuildId: 1eb6131419edb83b2178b682829a6913cf682d75)
    #2 0x71fbfaccc799 in g_tree_new_full (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x93799) (BuildId: 1eb6131419edb83b2178b682829a6913cf682d7
5)
    #3 0x5aded582b21a in cframe_reg_init /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../hw/misc/xlnx-versal-cframe-reg.c:691:18

Add an instance_finalize method to clean up what we
allocated in instance_init.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/misc/xlnx-versal-cframe-reg.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/hw/misc/xlnx-versal-cframe-reg.c b/hw/misc/xlnx-versal-cframe-reg.c
index 1ce083e2409..95e167b9213 100644
--- a/hw/misc/xlnx-versal-cframe-reg.c
+++ b/hw/misc/xlnx-versal-cframe-reg.c
@@ -693,6 +693,14 @@ static void cframe_reg_init(Object *obj)
     fifo32_create(&s->new_f_data, FRAME_NUM_WORDS);
 }
 
+static void cframe_reg_finalize(Object *obj)
+{
+    XlnxVersalCFrameReg *s = XLNX_VERSAL_CFRAME_REG(obj);
+
+    fifo32_destroy(&s->new_f_data);
+    g_tree_destroy(s->cframes);
+}
+
 static const VMStateDescription vmstate_cframe = {
     .name = "cframe",
     .version_id = 1,
@@ -833,6 +841,7 @@ static const TypeInfo cframe_reg_info = {
     .instance_size = sizeof(XlnxVersalCFrameReg),
     .class_init    = cframe_reg_class_init,
     .instance_init = cframe_reg_init,
+    .instance_finalize = cframe_reg_finalize,
     .interfaces = (const InterfaceInfo[]) {
         { TYPE_XLNX_CFI_IF },
         { }
-- 
2.43.0

Re: [PATCH 1/2] hw/misc/xlnx-versal-cframe-reg: Free FIFO, g_tree on deinit

Posted by Francisco Iglesias 5 months, 2 weeks ago

On Tue, Aug 26, 2025 at 06:49:55PM +0100, Peter Maydell wrote:
> In the xlnx-versal-cframe-reg device we create a FIFO in
> instance_init but don't destroy it on deinit, causing ASAN
> to report a leak in the device-introspect-test:
> 
> Direct leak of 400 byte(s) in 1 object(s) allocated from:
>     #0 0x5aded4d54e23 in malloc (/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/qemu-system-aarch64+0x24ffe23) (BuildId: 9f1e6c53fecd904ba5fc1f521d7da080a0e4103b)
>     #1 0x71fbfac9bb09 in g_malloc (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x62b09) (BuildId: 1eb6131419edb83b2178b682829a6913cf682d75)
>     #2 0x5aded850059d in fifo8_create /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../util/fifo8.c:27:18
>     #3 0x5aded582b9e4 in fifo32_create /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/include/qemu/fifo32.h:35:5
>     #4 0x5aded582b326 in cframe_reg_init /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../hw/misc/xlnx-versal-cframe-reg.c:693:5
> 
> Similarly, we don't clean up the g_tree we create:
> Direct leak of 48 byte(s) in 1 object(s) allocated from:
>     #0 0x5aded4d54e23 in malloc (/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/qemu-system-aarch64+0x24ffe23) (BuildId: 9f1e6c5
> 3fecd904ba5fc1f521d7da080a0e4103b)
>     #1 0x71fbfac9bb09 in g_malloc (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x62b09) (BuildId: 1eb6131419edb83b2178b682829a6913cf682d75)
>     #2 0x71fbfaccc799 in g_tree_new_full (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x93799) (BuildId: 1eb6131419edb83b2178b682829a6913cf682d7
> 5)
>     #3 0x5aded582b21a in cframe_reg_init /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../hw/misc/xlnx-versal-cframe-reg.c:691:18
> 
> Add an instance_finalize method to clean up what we
> allocated in instance_init.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Francisco Iglesias <francisco.iglesias@amd.com>

> ---
>  hw/misc/xlnx-versal-cframe-reg.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/hw/misc/xlnx-versal-cframe-reg.c b/hw/misc/xlnx-versal-cframe-reg.c
> index 1ce083e2409..95e167b9213 100644
> --- a/hw/misc/xlnx-versal-cframe-reg.c
> +++ b/hw/misc/xlnx-versal-cframe-reg.c
> @@ -693,6 +693,14 @@ static void cframe_reg_init(Object *obj)
>      fifo32_create(&s->new_f_data, FRAME_NUM_WORDS);
>  }
>  
> +static void cframe_reg_finalize(Object *obj)
> +{
> +    XlnxVersalCFrameReg *s = XLNX_VERSAL_CFRAME_REG(obj);
> +
> +    fifo32_destroy(&s->new_f_data);
> +    g_tree_destroy(s->cframes);
> +}
> +
>  static const VMStateDescription vmstate_cframe = {
>      .name = "cframe",
>      .version_id = 1,
> @@ -833,6 +841,7 @@ static const TypeInfo cframe_reg_info = {
>      .instance_size = sizeof(XlnxVersalCFrameReg),
>      .class_init    = cframe_reg_class_init,
>      .instance_init = cframe_reg_init,
> +    .instance_finalize = cframe_reg_finalize,
>      .interfaces = (const InterfaceInfo[]) {
>          { TYPE_XLNX_CFI_IF },
>          { }
> -- 
> 2.43.0
>

Re: [PATCH 1/2] hw/misc/xlnx-versal-cframe-reg: Free FIFO, g_tree on deinit

Posted by Manos Pitsidianakis 5 months, 2 weeks ago

On Tue, Aug 26, 2025 at 8:51 PM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> In the xlnx-versal-cframe-reg device we create a FIFO in
> instance_init but don't destroy it on deinit, causing ASAN
> to report a leak in the device-introspect-test:
>
> Direct leak of 400 byte(s) in 1 object(s) allocated from:
>     #0 0x5aded4d54e23 in malloc (/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/qemu-system-aarch64+0x24ffe23) (BuildId: 9f1e6c53fecd904ba5fc1f521d7da080a0e4103b)
>     #1 0x71fbfac9bb09 in g_malloc (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x62b09) (BuildId: 1eb6131419edb83b2178b682829a6913cf682d75)
>     #2 0x5aded850059d in fifo8_create /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../util/fifo8.c:27:18
>     #3 0x5aded582b9e4 in fifo32_create /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/include/qemu/fifo32.h:35:5
>     #4 0x5aded582b326 in cframe_reg_init /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../hw/misc/xlnx-versal-cframe-reg.c:693:5
>
> Similarly, we don't clean up the g_tree we create:
> Direct leak of 48 byte(s) in 1 object(s) allocated from:
>     #0 0x5aded4d54e23 in malloc (/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/qemu-system-aarch64+0x24ffe23) (BuildId: 9f1e6c5
> 3fecd904ba5fc1f521d7da080a0e4103b)
>     #1 0x71fbfac9bb09 in g_malloc (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x62b09) (BuildId: 1eb6131419edb83b2178b682829a6913cf682d75)
>     #2 0x71fbfaccc799 in g_tree_new_full (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x93799) (BuildId: 1eb6131419edb83b2178b682829a6913cf682d7
> 5)
>     #3 0x5aded582b21a in cframe_reg_init /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../hw/misc/xlnx-versal-cframe-reg.c:691:18
>
> Add an instance_finalize method to clean up what we
> allocated in instance_init.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  hw/misc/xlnx-versal-cframe-reg.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/hw/misc/xlnx-versal-cframe-reg.c b/hw/misc/xlnx-versal-cframe-reg.c
> index 1ce083e2409..95e167b9213 100644
> --- a/hw/misc/xlnx-versal-cframe-reg.c
> +++ b/hw/misc/xlnx-versal-cframe-reg.c
> @@ -693,6 +693,14 @@ static void cframe_reg_init(Object *obj)
>      fifo32_create(&s->new_f_data, FRAME_NUM_WORDS);
>  }
>
> +static void cframe_reg_finalize(Object *obj)
> +{
> +    XlnxVersalCFrameReg *s = XLNX_VERSAL_CFRAME_REG(obj);
> +
> +    fifo32_destroy(&s->new_f_data);
> +    g_tree_destroy(s->cframes);
> +}
> +
>  static const VMStateDescription vmstate_cframe = {
>      .name = "cframe",
>      .version_id = 1,
> @@ -833,6 +841,7 @@ static const TypeInfo cframe_reg_info = {
>      .instance_size = sizeof(XlnxVersalCFrameReg),
>      .class_init    = cframe_reg_class_init,
>      .instance_init = cframe_reg_init,
> +    .instance_finalize = cframe_reg_finalize,
>      .interfaces = (const InterfaceInfo[]) {
>          { TYPE_XLNX_CFI_IF },
>          { }
> --
> 2.43.0
>
>

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

Re: [PATCH 1/2] hw/misc/xlnx-versal-cframe-reg: Free FIFO, g_tree on deinit

Posted by Edgar E. Iglesias 5 months, 2 weeks ago

On Tue, Aug 26, 2025 at 06:49:55PM +0100, Peter Maydell wrote:
> In the xlnx-versal-cframe-reg device we create a FIFO in
> instance_init but don't destroy it on deinit, causing ASAN
> to report a leak in the device-introspect-test:
> 
> Direct leak of 400 byte(s) in 1 object(s) allocated from:
>     #0 0x5aded4d54e23 in malloc (/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/qemu-system-aarch64+0x24ffe23) (BuildId: 9f1e6c53fecd904ba5fc1f521d7da080a0e4103b)
>     #1 0x71fbfac9bb09 in g_malloc (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x62b09) (BuildId: 1eb6131419edb83b2178b682829a6913cf682d75)
>     #2 0x5aded850059d in fifo8_create /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../util/fifo8.c:27:18
>     #3 0x5aded582b9e4 in fifo32_create /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/include/qemu/fifo32.h:35:5
>     #4 0x5aded582b326 in cframe_reg_init /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../hw/misc/xlnx-versal-cframe-reg.c:693:5
> 
> Similarly, we don't clean up the g_tree we create:
> Direct leak of 48 byte(s) in 1 object(s) allocated from:
>     #0 0x5aded4d54e23 in malloc (/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/qemu-system-aarch64+0x24ffe23) (BuildId: 9f1e6c5
> 3fecd904ba5fc1f521d7da080a0e4103b)
>     #1 0x71fbfac9bb09 in g_malloc (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x62b09) (BuildId: 1eb6131419edb83b2178b682829a6913cf682d75)
>     #2 0x71fbfaccc799 in g_tree_new_full (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x93799) (BuildId: 1eb6131419edb83b2178b682829a6913cf682d7
> 5)
>     #3 0x5aded582b21a in cframe_reg_init /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../hw/misc/xlnx-versal-cframe-reg.c:691:18
> 
> Add an instance_finalize method to clean up what we
> allocated in instance_init.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>


Reviewed-by: Edgar E. Iglesias <edgar.iglesias@amd.com>



> ---
>  hw/misc/xlnx-versal-cframe-reg.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/hw/misc/xlnx-versal-cframe-reg.c b/hw/misc/xlnx-versal-cframe-reg.c
> index 1ce083e2409..95e167b9213 100644
> --- a/hw/misc/xlnx-versal-cframe-reg.c
> +++ b/hw/misc/xlnx-versal-cframe-reg.c
> @@ -693,6 +693,14 @@ static void cframe_reg_init(Object *obj)
>      fifo32_create(&s->new_f_data, FRAME_NUM_WORDS);
>  }
>  
> +static void cframe_reg_finalize(Object *obj)
> +{
> +    XlnxVersalCFrameReg *s = XLNX_VERSAL_CFRAME_REG(obj);
> +
> +    fifo32_destroy(&s->new_f_data);
> +    g_tree_destroy(s->cframes);
> +}
> +
>  static const VMStateDescription vmstate_cframe = {
>      .name = "cframe",
>      .version_id = 1,
> @@ -833,6 +841,7 @@ static const TypeInfo cframe_reg_info = {
>      .instance_size = sizeof(XlnxVersalCFrameReg),
>      .class_init    = cframe_reg_class_init,
>      .instance_init = cframe_reg_init,
> +    .instance_finalize = cframe_reg_finalize,
>      .interfaces = (const InterfaceInfo[]) {
>          { TYPE_XLNX_CFI_IF },
>          { }
> -- 
> 2.43.0
>