[PATCH] aspeed: Fix a potential memory leak bug in write_boot_rom()

[PATCH] aspeed: Fix a potential memory leak bug in write_boot_rom()

[Cédric Le Goater]

From: Wentao_Liang <Wentao_Liang_g@163.com>

A memory chunk is allocated with g_new0() and assigned to the variable
'storage'. However, if the branch takes true, there will be only an
error report but not a free operation for 'storage' before function
returns. As a result, a memory leak bug is triggered.

Use g_autofree to fix the issue.

Suggested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Wentao_Liang <Wentao_Liang_g@163.com>
[ clg: reworked the commit log ]
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/arm/aspeed.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index 11558b327bc9..b71bc2559baa 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -246,7 +246,7 @@ static void write_boot_rom(DriveInfo *dinfo, hwaddr addr, size_t rom_size,
                            Error **errp)
 {
     BlockBackend *blk = blk_by_legacy_dinfo(dinfo);
-    uint8_t *storage;
+    g_autofree void *storage = NULL;
     int64_t size;
 
     /* The block backend size should have already been 'validated' by
@@ -262,14 +262,13 @@ static void write_boot_rom(DriveInfo *dinfo, hwaddr addr, size_t rom_size,
         rom_size = size;
     }
 
-    storage = g_new0(uint8_t, rom_size);
+    storage = g_malloc0(rom_size);
     if (blk_pread(blk, 0, storage, rom_size) < 0) {
         error_setg(errp, "failed to read the initial flash content");
         return;
     }
 
     rom_add_blob_fixed("aspeed.boot_rom", storage, rom_size, addr);
-    g_free(storage);
 }
 
 static void aspeed_board_init_flashes(AspeedSMCState *s,
-- 
2.34.1

Re: [PATCH] aspeed: Fix a potential memory leak bug in write_boot_rom()

[Peter Maydell]

On Wed, 2 Mar 2022 at 08:33, Cédric Le Goater <clg@kaod.org> wrote:
>
> From: Wentao_Liang <Wentao_Liang_g@163.com>
>
> A memory chunk is allocated with g_new0() and assigned to the variable
> 'storage'. However, if the branch takes true, there will be only an
> error report but not a free operation for 'storage' before function
> returns. As a result, a memory leak bug is triggered.
>
> Use g_autofree to fix the issue.
>
> Suggested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Signed-off-by: Wentao_Liang <Wentao_Liang_g@163.com>
> [ clg: reworked the commit log ]
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM