Hi Anirudh, On 2026-07-19T18:03:31, Anirudh Srinivasan <[email protected]> wrote: > bios_emulator: don't free a caller-supplied BIOS image in biosemu_run > > biosemu_run can be called with a BIOS image supplied by the caller > (argument bios_rom is not NULL), or with no image so that it makes its > own copy from the card's ROM (argument bios_rom is NULL). > > It subsequently calls PCI_postController, where the BIOS image it runs > is stored in vga_info->BIOSImage. In the first case that pointer is the > caller's buffer; in the second it is a buffer biosemu_run itself > allocated. > > The clean_up path in biosemu_run always frees vga_info->BIOSImage. That > is right for the second case, but in the first case it frees memory the > caller still owns. This can cause a double free when the caller tries > freeing this memory again. > > Only free vga_info->BIOSImage when biosemu_run made the copy itself (i.e > bios_rom is not NULL)
The parenthetical is backwards - biosemu_run allocates its own copy when bios_rom is NULL. The code correctly tests !bios_rom, so please fix the commit message to say 'i.e. bios_rom is NULL'. > > Signed-off-by: Anirudh Srinivasan <[email protected]> > > drivers/bios_emulator/atibios.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > diff --git a/drivers/bios_emulator/atibios.c b/drivers/bios_emulator/atibios.c > @@ -509,7 +509,7 @@ int biosemu_run(struct udevice *pcidev, uchar *bios_rom, > int bios_len, > */ > if (clean_up) { > BE_exit(); > - if (vga_info->BIOSImage && > + if (!bios_rom && vga_info->BIOSImage && > (ulong)(vga_info->BIOSImage) != 0xc0000) > free(vga_info->BIOSImage); > free(vga_info); The fix is correct - dm_pci_run_vga_bios() in drivers/pci/pci_rom.c frees 'ram' itself when 'alloced' is true, so freeing it here as well is a double free. BTW with bios_rom non-NULL, vga_info->BIOSImage now always aliases the caller's buffer, so the (ulong)(...) != 0xc0000 check is only meaningful in the !bios_rom case. Not for this patch, but worth noting. Regards, Simon
