From 93c76555d842b5d84b95f66abecb6b19545338d9 Mon Sep 17 00:00:00 2001 From: David Woodhouse Date: Wed, 19 Jun 2024 14:03:08 +0100 Subject: hw/i386/fw_cfg: Add etc/e820 to fw_cfg late In e820_add_entry() the e820_table is reallocated with g_renew() to make space for a new entry. However, fw_cfg_arch_create() just uses the existing e820_table pointer. This leads to a use-after-free if anything adds a new entry after fw_cfg is set up. Shift the addition of the etc/e820 file to the machine done notifier, via a new fw_cfg_add_e820() function. Also make e820_table private and use an e820_get_table() accessor function for it, which sets a flag that will trigger an assert() for any *later* attempts to add to the table. Make e820_add_entry() return void, as most callers don't check for error anyway. Signed-off-by: David Woodhouse Message-Id: Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/i386/fw_cfg.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) (limited to 'hw/i386/fw_cfg.c') diff --git a/hw/i386/fw_cfg.c b/hw/i386/fw_cfg.c index 7c43c32..0e44946 100644 --- a/hw/i386/fw_cfg.c +++ b/hw/i386/fw_cfg.c @@ -48,6 +48,15 @@ const char *fw_cfg_arch_key_name(uint16_t key) return NULL; } +/* Add etc/e820 late, once all regions should be present */ +void fw_cfg_add_e820(FWCfgState *fw_cfg) +{ + struct e820_entry *table; + int nr_e820 = e820_get_table(&table); + + fw_cfg_add_file(fw_cfg, "etc/e820", table, nr_e820 * sizeof(*table)); +} + void fw_cfg_build_smbios(PCMachineState *pcms, FWCfgState *fw_cfg, SmbiosEntryPointType ep_type) { @@ -60,6 +69,7 @@ void fw_cfg_build_smbios(PCMachineState *pcms, FWCfgState *fw_cfg, PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms); MachineClass *mc = MACHINE_GET_CLASS(pcms); X86CPU *cpu = X86_CPU(ms->possible_cpus->cpus[0].cpu); + int nr_e820; if (pcmc->smbios_defaults) { /* These values are guest ABI, do not change */ @@ -78,8 +88,9 @@ void fw_cfg_build_smbios(PCMachineState *pcms, FWCfgState *fw_cfg, } /* build the array of physical mem area from e820 table */ - mem_array = g_malloc0(sizeof(*mem_array) * e820_get_num_entries()); - for (i = 0, array_count = 0; i < e820_get_num_entries(); i++) { + nr_e820 = e820_get_table(NULL); + mem_array = g_malloc0(sizeof(*mem_array) * nr_e820); + for (i = 0, array_count = 0; i < nr_e820; i++) { uint64_t addr, len; if (e820_get_entry(i, E820_RAM, &addr, &len)) { @@ -138,9 +149,6 @@ FWCfgState *fw_cfg_arch_create(MachineState *ms, #endif fw_cfg_add_i32(fw_cfg, FW_CFG_IRQ0_OVERRIDE, 1); - fw_cfg_add_file(fw_cfg, "etc/e820", e820_table, - sizeof(struct e820_entry) * e820_get_num_entries()); - fw_cfg_add_bytes(fw_cfg, FW_CFG_HPET, &hpet_cfg, sizeof(hpet_cfg)); /* allocate memory for the NUMA channel: one (64bit) word for the number * of nodes, one word for each VCPU->node and one word for each node to -- cgit v1.1