diff mbox series

[v2,4/6] hw/i386/x86: Don't leak "pc.bios" memory region

Message ID 20240430150643.111976-5-shentey@gmail.com
State New
Headers show
Series This series changes the "isa-bios" MemoryRegion to be an alias rather than a | expand

Commit Message

Bernhard Beschow April 30, 2024, 3:06 p.m. UTC 
Fix the leaking in x86_bios_rom_init() by adding a "bios" attribute to
X86MachineState. Note that it is only used in the -bios case.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 include/hw/i386/x86.h |  1 +
 hw/i386/x86.c         | 13 ++++++-------
 2 files changed, 7 insertions(+), 7 deletions(-)

Comments

Philippe Mathieu-Daudé April 30, 2024, 3:31 p.m. UTC | #1 
On 30/4/24 17:06, Bernhard Beschow wrote:
> Fix the leaking in x86_bios_rom_init() by adding a "bios" attribute to
> X86MachineState. Note that it is only used in the -bios case.
> 
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
>   include/hw/i386/x86.h |  1 +
>   hw/i386/x86.c         | 13 ++++++-------
>   2 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/include/hw/i386/x86.h b/include/hw/i386/x86.h
> index 271ad50470..fb41263b9d 100644
> --- a/include/hw/i386/x86.h
> +++ b/include/hw/i386/x86.h
> @@ -52,6 +52,7 @@ struct X86MachineState {
>       DeviceState *ioapic2;
>       GMappedFile *initrd_mapped_file;
>       HotplugHandler *acpi_dev;
> +    MemoryRegion bios;
>       MemoryRegion isa_bios;

Do you mind describing what is each in a small comment?

Otherwise,
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
diff mbox series

Patch

diff --git a/include/hw/i386/x86.h b/include/hw/i386/x86.h
index 271ad50470..fb41263b9d 100644
--- a/include/hw/i386/x86.h
+++ b/include/hw/i386/x86.h
@@ -52,6 +52,7 @@  struct X86MachineState {
     DeviceState *ioapic2;
     GMappedFile *initrd_mapped_file;
     HotplugHandler *acpi_dev;
+    MemoryRegion bios;
     MemoryRegion isa_bios;
 
     /* RAM information (sizes, addresses, configuration): */
diff --git a/hw/i386/x86.c b/hw/i386/x86.c
index 457e8a34a5..29167de97d 100644
--- a/hw/i386/x86.c
+++ b/hw/i386/x86.c
@@ -1133,7 +1133,6 @@  void x86_bios_rom_init(X86MachineState *x86ms, const char *default_firmware,
 {
     const char *bios_name;
     char *filename;
-    MemoryRegion *bios;
     int bios_size, isa_bios_size;
     ssize_t ret;
 
@@ -1149,8 +1148,8 @@  void x86_bios_rom_init(X86MachineState *x86ms, const char *default_firmware,
         (bios_size % 65536) != 0) {
         goto bios_error;
     }
-    bios = g_malloc(sizeof(*bios));
-    memory_region_init_ram(bios, NULL, "pc.bios", bios_size, &error_fatal);
+    memory_region_init_ram(&x86ms->bios, NULL, "pc.bios", bios_size,
+                           &error_fatal);
     if (sev_enabled()) {
         /*
          * The concept of a "reset" simply doesn't exist for
@@ -1159,11 +1158,11 @@  void x86_bios_rom_init(X86MachineState *x86ms, const char *default_firmware,
          * the firmware as rom to properly re-initialize on reset.
          * Just go for a straight file load instead.
          */
-        void *ptr = memory_region_get_ram_ptr(bios);
+        void *ptr = memory_region_get_ram_ptr(&x86ms->bios);
         load_image_size(filename, ptr, bios_size);
         x86_firmware_configure(ptr, bios_size);
     } else {
-        memory_region_set_readonly(bios, !isapc_ram_fw);
+        memory_region_set_readonly(&x86ms->bios, !isapc_ram_fw);
         ret = rom_add_file_fixed(bios_name, (uint32_t)(-bios_size), -1);
         if (ret != 0) {
             goto bios_error;
@@ -1173,7 +1172,7 @@  void x86_bios_rom_init(X86MachineState *x86ms, const char *default_firmware,
 
     /* map the last 128KB of the BIOS in ISA space */
     isa_bios_size = MIN(bios_size, 128 * KiB);
-    memory_region_init_alias(&x86ms->isa_bios, NULL, "isa-bios", bios,
+    memory_region_init_alias(&x86ms->isa_bios, NULL, "isa-bios", &x86ms->bios,
                              bios_size - isa_bios_size, isa_bios_size);
     memory_region_add_subregion_overlap(rom_memory,
                                         0x100000 - isa_bios_size,
@@ -1184,7 +1183,7 @@  void x86_bios_rom_init(X86MachineState *x86ms, const char *default_firmware,
     /* map all the bios at the top of memory */
     memory_region_add_subregion(rom_memory,
                                 (uint32_t)(-bios_size),
-                                bios);
+                                &x86ms->bios);
     return;
 
 bios_error: