From: Bernhard Beschow <shentey@gmail.com>
To: qemu-devel@nongnu.org
Cc: "Michael S. Tsirkin" <mst@redhat.com>,
"Sergio Lopez" <slp@redhat.com>,
"Richard Henderson" <richard.henderson@linaro.org>,
"Eduardo Habkost" <eduardo@habkost.net>,
"Philippe Mathieu-Daudé" <philmd@linaro.org>,
"Marcel Apfelbaum" <marcel.apfelbaum@gmail.com>,
"Paolo Bonzini" <pbonzini@redhat.com>,
"Michael Roth" <michael.roth@amd.com>,
"Bernhard Beschow" <shentey@gmail.com>
Subject: [PATCH v3 3/6] hw/i386/x86: Don't leak "isa-bios" memory regions
Date: Wed, 8 May 2024 19:55:04 +0200 [thread overview]
Message-ID: <20240508175507.22270-4-shentey@gmail.com> (raw)
In-Reply-To: <20240508175507.22270-1-shentey@gmail.com>
Fix the leaking in x86_bios_rom_init() and pc_isa_bios_init() by adding an
"isa_bios" attribute to X86MachineState.
Suggested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
include/hw/i386/x86.h | 7 +++++++
hw/i386/pc_sysfw.c | 7 +++----
hw/i386/x86.c | 9 ++++-----
3 files changed, 14 insertions(+), 9 deletions(-)
diff --git a/include/hw/i386/x86.h b/include/hw/i386/x86.h
index cb07618d19..a07de79167 100644
--- a/include/hw/i386/x86.h
+++ b/include/hw/i386/x86.h
@@ -18,6 +18,7 @@
#define HW_I386_X86_H
#include "exec/hwaddr.h"
+#include "exec/memory.h"
#include "hw/boards.h"
#include "hw/intc/ioapic.h"
@@ -52,6 +53,12 @@ struct X86MachineState {
GMappedFile *initrd_mapped_file;
HotplugHandler *acpi_dev;
+ /*
+ * Map the upper 128 KiB of the BIOS just underneath the 1 MiB address
+ * boundary.
+ */
+ MemoryRegion isa_bios;
+
/* RAM information (sizes, addresses, configuration): */
ram_addr_t below_4g_mem_size, above_4g_mem_size;
diff --git a/hw/i386/pc_sysfw.c b/hw/i386/pc_sysfw.c
index 59c7a81692..82d37cb376 100644
--- a/hw/i386/pc_sysfw.c
+++ b/hw/i386/pc_sysfw.c
@@ -40,11 +40,10 @@
#define FLASH_SECTOR_SIZE 4096
-static void pc_isa_bios_init(MemoryRegion *rom_memory,
+static void pc_isa_bios_init(MemoryRegion *isa_bios, MemoryRegion *rom_memory,
MemoryRegion *flash_mem)
{
int isa_bios_size;
- MemoryRegion *isa_bios;
uint64_t flash_size;
void *flash_ptr, *isa_bios_ptr;
@@ -52,7 +51,6 @@ static void pc_isa_bios_init(MemoryRegion *rom_memory,
/* map the last 128KB of the BIOS in ISA space */
isa_bios_size = MIN(flash_size, 128 * KiB);
- isa_bios = g_malloc(sizeof(*isa_bios));
memory_region_init_ram(isa_bios, NULL, "isa-bios", isa_bios_size,
&error_fatal);
memory_region_add_subregion_overlap(rom_memory,
@@ -136,6 +134,7 @@ void pc_system_flash_cleanup_unused(PCMachineState *pcms)
static void pc_system_flash_map(PCMachineState *pcms,
MemoryRegion *rom_memory)
{
+ X86MachineState *x86ms = X86_MACHINE(pcms);
hwaddr total_size = 0;
int i;
BlockBackend *blk;
@@ -185,7 +184,7 @@ static void pc_system_flash_map(PCMachineState *pcms,
if (i == 0) {
flash_mem = pflash_cfi01_get_memory(system_flash);
- pc_isa_bios_init(rom_memory, flash_mem);
+ pc_isa_bios_init(&x86ms->isa_bios, rom_memory, flash_mem);
/* Encrypt the pflash boot ROM */
if (sev_enabled()) {
diff --git a/hw/i386/x86.c b/hw/i386/x86.c
index 6d3c72f124..457e8a34a5 100644
--- a/hw/i386/x86.c
+++ b/hw/i386/x86.c
@@ -1133,7 +1133,7 @@ void x86_bios_rom_init(X86MachineState *x86ms, const char *default_firmware,
{
const char *bios_name;
char *filename;
- MemoryRegion *bios, *isa_bios;
+ MemoryRegion *bios;
int bios_size, isa_bios_size;
ssize_t ret;
@@ -1173,14 +1173,13 @@ 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);
- isa_bios = g_malloc(sizeof(*isa_bios));
- memory_region_init_alias(isa_bios, NULL, "isa-bios", bios,
+ memory_region_init_alias(&x86ms->isa_bios, NULL, "isa-bios", bios,
bios_size - isa_bios_size, isa_bios_size);
memory_region_add_subregion_overlap(rom_memory,
0x100000 - isa_bios_size,
- isa_bios,
+ &x86ms->isa_bios,
1);
- memory_region_set_readonly(isa_bios, !isapc_ram_fw);
+ memory_region_set_readonly(&x86ms->isa_bios, !isapc_ram_fw);
/* map all the bios at the top of memory */
memory_region_add_subregion(rom_memory,
--
2.45.0
next prev parent reply other threads:[~2024-05-08 17:55 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-08 17:55 [PATCH v3 0/6] X86: Alias isa-bios area and clean up Bernhard Beschow
2024-05-08 17:55 ` [PATCH v3 1/6] hw/i386/x86: Eliminate two if statements in x86_bios_rom_init() Bernhard Beschow
2024-05-08 17:55 ` [PATCH v3 2/6] hw/i386: Have x86_bios_rom_init() take X86MachineState rather than MachineState Bernhard Beschow
2024-05-08 17:55 ` Bernhard Beschow [this message]
2024-05-08 17:55 ` [PATCH v3 4/6] hw/i386/x86: Don't leak "pc.bios" memory region Bernhard Beschow
2024-05-08 17:55 ` [PATCH v3 5/6] hw/i386/x86: Extract x86_isa_bios_init() from x86_bios_rom_init() Bernhard Beschow
2024-05-08 17:55 ` [PATCH v3 6/6] hw/i386/pc_sysfw: Alias rather than copy isa-bios region Bernhard Beschow
2024-05-08 20:39 ` [PATCH v3 0/6] X86: Alias isa-bios area and clean up BALATON Zoltan
2024-05-14 8:31 ` Bernhard Beschow
2024-05-08 22:10 ` Philippe Mathieu-Daudé
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240508175507.22270-4-shentey@gmail.com \
--to=shentey@gmail.com \
--cc=eduardo@habkost.net \
--cc=marcel.apfelbaum@gmail.com \
--cc=michael.roth@amd.com \
--cc=mst@redhat.com \
--cc=pbonzini@redhat.com \
--cc=philmd@linaro.org \
--cc=qemu-devel@nongnu.org \
--cc=richard.henderson@linaro.org \
--cc=slp@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).