x86/efistub: Omit physical KASLR when memory reservations exist
Subsystems (2)
PATCH 1/1 x86/efistub: Omit physical KASLR when memory reservations exist
From: Ard Biesheuvel <[email protected]>
The legacy decompressor has elaborate logic to ensure that the randomized physical placement of the decompressed kernel image does not conflict with any memory reservations, including ones specified on the command line using mem=, memmap=, efi_fake_mem= or hugepages=, which are taken into account by the kernel proper at a later stage.
When booting in EFI mode, it is the firmware's job to ensure that the chosen range does not conflict with any memory reservations that it knows about, and this is trivially achieved by using the firmware's memory allocation APIs.
That leaves reservations specified on the command line, though, which the firmware knows nothing about, as these regions have no other special significance to the platform. Since commit
a1b87d54f4e4 ("x86/efistub: Avoid legacy decompressor when doing EFI boot")
these reservations are not taken into account when randomizing the physical placement, which may result in conflicts where the memory cannot be reserved by the kernel proper because its own executable image resides there.
To avoid having to duplicate or reuse the existing complicated logic, disable physical KASLR entirely when such overrides are specified. These are mostly diagnostic tools or niche features, and physical KASLR (as opposed to virtual KASLR, which is much more important as it affects the memory addresses observed by code executing in the kernel) is something we can live without.
--- drivers/firmware/efi/libstub/x86-stub.c | 28 +++++++++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-)
drivers/firmware/efi/libstub/x86-stub.c +26 -2
@@ -776,6 +776,26 @@ static void error(char *str)efi_warn ("Decompression failed: %s\n" , str); } +static const char *cmdline_memmap_override; + +static efi_status_t parse_options (const char *cmdline) +{ +static const char opts[][14 ] = { +"mem=" ,"memmap=" ,"efi_fake_mem=" ,"hugepages=" + }; + +for (int i =0 ; i <ARRAY_SIZE (opts); i++) { +const char *p =strstr (cmdline, opts[i]); + +if (p == cmdline || (p > cmdline &&isspace (p[-1 ]))) { + cmdline_memmap_override = opts[i]; +break ; + } + } + +return efi_parse_options (cmdline); +} +static efi_status_t efi_decompress_kernel (unsigned long *kernel_entry) {unsigned long virt_addr =LOAD_PHYSICAL_ADDR ;
@@ -807,6 +827,10 @@ static efi_status_t efi_decompress_kernel(unsigned long *kernel_entry) !memcmp(efistub_fw_vendor(), ami, sizeof(ami))) {efi_debug ("AMI firmware v2.0 or older detected - disabling physical KASLR\n" ); seed[0 ] =0 ; + } else if (cmdline_memmap_override) { +efi_info ("%s detected on the kernel command line - disabling physical KASLR\n" , + cmdline_memmap_override); + seed[0 ] =0 ; } boot_params_ptr->hdr.loadflags |=KASLR_FLAG ;
@@ -883,7 +907,7 @@ void __noreturn efi_stub_entry(efi_handle_t handle, }#ifdef CONFIG_CMDLINE_BOOL - status =efi_parse_options (CONFIG_CMDLINE ); + status =parse_options (CONFIG_CMDLINE );if (status !=EFI_SUCCESS ) {efi_err ("Failed to parse options\n" ); goto fail;
@@ -892,7 +916,7 @@ void __noreturn efi_stub_entry(efi_handle_t handle,if (!IS_ENABLED (CONFIG_CMDLINE_OVERRIDE )) {unsigned long cmdline_paddr = ((u64 )hdr->cmd_line_ptr | ((u64 )boot_params->ext_cmd_line_ptr <<32 )); - status =efi_parse_options ((char *)cmdline_paddr); + status =parse_options ((char *)cmdline_paddr);if (status !=EFI_SUCCESS ) {efi_err ("Failed to parse options\n" ); goto fail;
Signature · 1 line
2.45.0.rc1.225.g2a3ae87e7f-goog
drivers/firmware/efi/libstub/x86-stub.c static void error(char *str) · line 786
Quoted 6 lines
drivers/firmware/efi/libstub/x86-stub.c +6
-0+static efi_status_t parse_options (const char *cmdline) +{ +static const char opts[][14 ] = { +"mem=" ,"memmap=" ,"efi_fake_mem=" ,"hugepages=" + }; +
I think we probably want to include both crashkernel and pstore as arguments that can disable this randomization.
Thanks,
Ben
Ard Biesheuvel
maintainerQuoted 10 lines · Ben
drivers/firmware/efi/libstub/x86-stub.c static void error(char *str) · line 786
Quoted 7 lines
drivers/firmware/efi/libstub/x86-stub.c +6
-0+static efi_status_t parse_options (const char *cmdline) +{ +static const char opts[][14 ] = { +"mem=" ,"memmap=" ,"efi_fake_mem=" ,"hugepages=" + }; +I think we probably want to include both crashkernel and pstore as arguments that can disable this randomization.
The existing code in arch/x86/boot/compressed/kaslr.c currently does not take these into account, as far as I can tell.
Quoted 8 lines · Ben
drivers/firmware/efi/libstub/x86-stub.c static void error(char *str) · line 786
Quoted 6 lines
drivers/firmware/efi/libstub/x86-stub.c +6
-0+static efi_status_t parse_options (const char *cmdline) +{ +static const char opts[][14 ] = { +"mem=" ,"memmap=" ,"efi_fake_mem=" ,"hugepages=" + }; +I think we probably want to include both crashkernel and pstore as arguments that can disable this randomization.
The carve-outs that pstore uses should already appear in the physical memory mapping that EFI has. (i.e. those things get listed in e820 as non-RAM, etc)
I don't know anything about crashkernel, but if we really do have a lot of these, we likely need to find a way to express them to EFI...
Signature · 1 line
Kees Cook
Ard Biesheuvel
maintainerQuoted 11 lines · Kees Cook
Quoted 8 lines · Ben
drivers/firmware/efi/libstub/x86-stub.c static void error(char *str) · line 786
Quoted 6 lines
drivers/firmware/efi/libstub/x86-stub.c +6
-0+static efi_status_t parse_options (const char *cmdline) +{ +static const char opts[][14 ] = { +"mem=" ,"memmap=" ,"efi_fake_mem=" ,"hugepages=" + }; +I think we probably want to include both crashkernel and pstore as arguments that can disable this randomization.
Quoted 7 lines
The carve-outs that pstore uses should already appear in the physical memory mapping that EFI has. (i.e. those things get listed in e820 as non-RAM, etc)
I don't know anything about crashkernel, but if we really do have a lot of these, we likely need to find a way to express them to EFI...
Perhaps. But the fact that the current KASLR code ignores it entirely suggests that this has not been a problem up to this point.
Quoted 31 lines · Ard Biesheuvel
From: Ard Biesheuvel <[email protected]>
The legacy decompressor has elaborate logic to ensure that the randomized physical placement of the decompressed kernel image does not conflict with any memory reservations, including ones specified on the command line using mem=, memmap=, efi_fake_mem= or hugepages=, which are taken into account by the kernel proper at a later stage.
When booting in EFI mode, it is the firmware's job to ensure that the chosen range does not conflict with any memory reservations that it knows about, and this is trivially achieved by using the firmware's memory allocation APIs.
That leaves reservations specified on the command line, though, which the firmware knows nothing about, as these regions have no other special significance to the platform. Since commit
a1b87d54f4e4("x86/efistub: Avoid legacy decompressor when doing EFI boot")these reservations are not taken into account when randomizing the physical placement, which may result in conflicts where the memory cannot be reserved by the kernel proper because its own executable image resides there.
To avoid having to duplicate or reuse the existing complicated logic, disable physical KASLR entirely when such overrides are specified. These are mostly diagnostic tools or niche features, and physical KASLR (as opposed to virtual KASLR, which is much more important as it affects the memory addresses observed by code executing in the kernel) is something we can live without.
Quoted 5 lines
Reported-by Ben Chaney <[email protected]>Fixes a1b87d54f4e4 ("x86/efistub: Avoid legacy decompressor when doing EFI boot")Cc <[email protected]> # v6.1+Signed-off-by Ard Biesheuvel <[email protected]>
Yup, all good by me:
Signature · 1 line
Kees Cook