Re: [PATCH v4 08/26] x86/boot: Map memory explicitly

From: Ard Biesheuvel
Date: Wed Mar 08 2023 - 04:38:28 EST

On Thu, 15 Dec 2022 at 13:38, Evgeniy Baskov <baskov@xxxxxxxxx> wrote:
>
> Implicit mappings hide possible memory errors, e.g. allocations for
> ACPI tables were not included in boot page table size.
>
> Replace all implicit mappings from page fault handler with
> explicit mappings.
>

I agree with the motivation but this patch seems to break the boot
under SeaBIOS/QEMU, and I imagine other legacy BIOS boot scenarios as
well.

Naively, I would assume that there is simply a legacy BIOS region that
we fail to map here, but I am fairly clueless when it comes to non-EFI
x86 boot so take this with a grain of salt.


> Tested-by: Mario Limonciello <mario.limonciello@xxxxxxx>
> Tested-by: Peter Jones <pjones@xxxxxxxxxx>
> Signed-off-by: Evgeniy Baskov <baskov@xxxxxxxxx>
> ---
> arch/x86/boot/compressed/acpi.c | 25 ++++++++++++++++++++++++-
> arch/x86/boot/compressed/efi.c | 19 ++++++++++++++++++-
> arch/x86/boot/compressed/kaslr.c | 4 ++++
> 3 files changed, 46 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/boot/compressed/acpi.c b/arch/x86/boot/compressed/acpi.c
> index 9caf89063e77..c775e01fc7db 100644
> --- a/arch/x86/boot/compressed/acpi.c
> +++ b/arch/x86/boot/compressed/acpi.c
> @@ -93,6 +93,8 @@ static u8 *scan_mem_for_rsdp(u8 *start, u32 length)
>
> end = start + length;
>
> + kernel_add_identity_map((unsigned long)start, (unsigned long)end, 0);
> +
> /* Search from given start address for the requested length */
> for (address = start; address < end; address += ACPI_RSDP_SCAN_STEP) {
> /*
> @@ -128,6 +130,9 @@ static acpi_physical_address bios_get_rsdp_addr(void)
> unsigned long address;
> u8 *rsdp;
>
> + kernel_add_identity_map((unsigned long)ACPI_EBDA_PTR_LOCATION,
> + (unsigned long)ACPI_EBDA_PTR_LOCATION + 2, 0);
> +
> /* Get the location of the Extended BIOS Data Area (EBDA) */
> address = *(u16 *)ACPI_EBDA_PTR_LOCATION;
> address <<= 4;
> @@ -215,6 +220,9 @@ static unsigned long get_acpi_srat_table(void)
> if (!rsdp)
> return 0;
>
> + kernel_add_identity_map((unsigned long)rsdp,
> + (unsigned long)(rsdp + 1), 0);
> +
> /* Get ACPI root table from RSDP.*/
> if (!(cmdline_find_option("acpi", arg, sizeof(arg)) == 4 &&
> !strncmp(arg, "rsdt", 4)) &&
> @@ -231,10 +239,17 @@ static unsigned long get_acpi_srat_table(void)
> return 0;
>
> header = (struct acpi_table_header *)root_table;
> +
> + kernel_add_identity_map((unsigned long)header,
> + (unsigned long)(header + 1), 0);
> +
> len = header->length;
> if (len < sizeof(struct acpi_table_header) + size)
> return 0;
>
> + kernel_add_identity_map((unsigned long)header,
> + (unsigned long)header + len, 0);
> +
> num_entries = (len - sizeof(struct acpi_table_header)) / size;
> entry = (u8 *)(root_table + sizeof(struct acpi_table_header));
>
> @@ -247,8 +262,16 @@ static unsigned long get_acpi_srat_table(void)
> if (acpi_table) {
> header = (struct acpi_table_header *)acpi_table;
>
> - if (ACPI_COMPARE_NAMESEG(header->signature, ACPI_SIG_SRAT))
> + kernel_add_identity_map(acpi_table,
> + acpi_table + sizeof(*header),
> + 0);
> +
> + if (ACPI_COMPARE_NAMESEG(header->signature, ACPI_SIG_SRAT)) {
> + kernel_add_identity_map(acpi_table,
> + acpi_table + header->length,
> + 0);
> return acpi_table;
> + }
> }
> entry += size;
> }
> diff --git a/arch/x86/boot/compressed/efi.c b/arch/x86/boot/compressed/efi.c
> index 6edd034b0b30..ce70103fbbc0 100644
> --- a/arch/x86/boot/compressed/efi.c
> +++ b/arch/x86/boot/compressed/efi.c
> @@ -57,10 +57,14 @@ enum efi_type efi_get_type(struct boot_params *bp)
> */
> unsigned long efi_get_system_table(struct boot_params *bp)
> {
> - unsigned long sys_tbl_pa;
> + static unsigned long sys_tbl_pa __section(".data");
> struct efi_info *ei;
> + unsigned long sys_tbl_size;
> enum efi_type et;
>
> + if (sys_tbl_pa)
> + return sys_tbl_pa;
> +
> /* Get systab from boot params. */
> ei = &bp->efi_info;
> #ifdef CONFIG_X86_64
> @@ -73,6 +77,13 @@ unsigned long efi_get_system_table(struct boot_params *bp)
> return 0;
> }
>
> + if (efi_get_type(bp) == EFI_TYPE_64)
> + sys_tbl_size = sizeof(efi_system_table_64_t);
> + else
> + sys_tbl_size = sizeof(efi_system_table_32_t);
> +
> + kernel_add_identity_map(sys_tbl_pa, sys_tbl_pa + sys_tbl_size, 0);
> +
> return sys_tbl_pa;
> }
>
> @@ -92,6 +103,10 @@ static struct efi_setup_data *get_kexec_setup_data(struct boot_params *bp,
>
> pa_data = bp->hdr.setup_data;
> while (pa_data) {
> + unsigned long pa_data_end = pa_data + sizeof(struct setup_data)
> + + sizeof(struct efi_setup_data);
> + kernel_add_identity_map(pa_data, pa_data_end, 0);
> +
> data = (struct setup_data *)pa_data;
> if (data->type == SETUP_EFI) {
> esd = (struct efi_setup_data *)(pa_data + sizeof(struct setup_data));
> @@ -160,6 +175,8 @@ int efi_get_conf_table(struct boot_params *bp, unsigned long *cfg_tbl_pa,
> return -EINVAL;
> }
>
> + kernel_add_identity_map(*cfg_tbl_pa, *cfg_tbl_pa + *cfg_tbl_len, 0);
> +
> return 0;
> }
>
> diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
> index 454757fbdfe5..c0ee116c4fa2 100644
> --- a/arch/x86/boot/compressed/kaslr.c
> +++ b/arch/x86/boot/compressed/kaslr.c
> @@ -688,6 +688,8 @@ process_efi_entries(unsigned long minimum, unsigned long image_size)
> u32 nr_desc;
> int i;
>
> + kernel_add_identity_map((unsigned long)e, (unsigned long)(e + 1), 0);
> +
> signature = (char *)&e->efi_loader_signature;
> if (strncmp(signature, EFI32_LOADER_SIGNATURE, 4) &&
> strncmp(signature, EFI64_LOADER_SIGNATURE, 4))
> @@ -704,6 +706,8 @@ process_efi_entries(unsigned long minimum, unsigned long image_size)
> pmap = (e->efi_memmap | ((__u64)e->efi_memmap_hi << 32));
> #endif
>
> + kernel_add_identity_map(pmap, pmap + e->efi_memmap_size, 0);
> +
> nr_desc = e->efi_memmap_size / e->efi_memdesc_size;
> for (i = 0; i < nr_desc; i++) {
> md = efi_early_memdesc_ptr(pmap, e->efi_memdesc_size, i);
> --
> 2.37.4
>