Re: Fix a potential memory leak bug in write_boot_rom() (v6.2.0).

[Philippe Mathieu-Daudé]

On 23/2/22 15:39, wliang@stu.xidian.edu.cn wrote:
> Hi all,
>
> I find a memory leak bug in QEMU 6.2.0, which is in 
> write_boot_rom()(./hw/arm/aspeed.c).
>
> Specifically, at line 276, a memory chunk is allocated with g_new0() and 
> assigned to the variable 'storage'. However, if the branch takes true at 
> line 277, there will be only an error report at line 278 but not a free 
> operation for 'storage' before function returns. As a result, a memory 
> leak bug is triggered.
>
>
> 259    BlockBackend *blk = blk_by_legacy_dinfo(dinfo);
> ...
> 276    storage = g_new0(uint8_t, rom_size);
> 277    if (blk_pread(blk, 0, storage, rom_size) < 0) {
> 278        error_setg(errp, "failed to read the initial flash content");
> 279        return;
> 280    }
>
>
> I believe that the problem can be fixed by adding a g_free() before the 
> function returns.
>
>
> 277    if (blk_pread(blk, 0, storage, rom_size) < 0) {
> 278        error_setg(errp, "failed to read the initial flash content");
> +++    g_free(storage);
> 279        return;
> 280    }
>
>
> I'm looking forward to your confirmation.

Correct.

Or using g_autofree:

-- >8 --
diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index d911dc904f..170e773ef8 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -257,7 +257,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
@@ -273,14 +273,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);
 }
---