From aacfa0ef247b0130b7a98bb52378f8cd727a66ca Mon Sep 17 00:00:00 2001 From: Jonathan Marek Date: Sun, 13 Oct 2024 01:11:56 -0400 Subject: [PATCH 01/16] efi/libstub: fix efi_parse_options() ignoring the default command line efi_convert_cmdline() always returns a size of at least 1 because it counts the NUL terminator, so the "cmdline_size == 0" condition is never satisfied. Change it to check if the string starts with a NUL character to get the intended behavior: to use CONFIG_CMDLINE when load_options_size == 0. Fixes: 60f38de7a8d4 ("efi/libstub: Unify command line param parsing") Signed-off-by: Jonathan Marek Signed-off-by: Ard Biesheuvel --- drivers/firmware/efi/libstub/efi-stub.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/firmware/efi/libstub/efi-stub.c b/drivers/firmware/efi/libstub/efi-stub.c index 958a680e0660..2a1b43f9e0fa 100644 --- a/drivers/firmware/efi/libstub/efi-stub.c +++ b/drivers/firmware/efi/libstub/efi-stub.c @@ -129,7 +129,7 @@ efi_status_t efi_handle_cmdline(efi_loaded_image_t *image, char **cmdline_ptr) if (IS_ENABLED(CONFIG_CMDLINE_EXTEND) || IS_ENABLED(CONFIG_CMDLINE_FORCE) || - cmdline_size == 0) { + cmdline[0] == 0) { status = efi_parse_options(CONFIG_CMDLINE); if (status != EFI_SUCCESS) { efi_err("Failed to parse options\n"); From ade7ccba2d647fced0c1f5f290705dcab14b674e Mon Sep 17 00:00:00 2001 From: Jonathan Marek Date: Sun, 13 Oct 2024 01:11:57 -0400 Subject: [PATCH 02/16] efi/libstub: remove unnecessary cmd_line_len from efi_convert_cmdline() efi_convert_cmdline() always sets cmdline_size to at least 1 on success, so the "cmdline_size > 0" does nothing and can be removed (the intent was to avoid parsing an empty string, but there is nothing wrong with parsing an empty string, it is only making boot negligibly slower). Then the cmd_line_len argument to efi_convert_cmdline can be removed because there is nothing left using it. Signed-off-by: Jonathan Marek Signed-off-by: Ard Biesheuvel --- drivers/firmware/efi/libstub/efi-stub-helper.c | 3 +-- drivers/firmware/efi/libstub/efi-stub.c | 5 ++--- drivers/firmware/efi/libstub/efistub.h | 2 +- drivers/firmware/efi/libstub/x86-stub.c | 3 +-- 4 files changed, 5 insertions(+), 8 deletions(-) diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c b/drivers/firmware/efi/libstub/efi-stub-helper.c index de659f6a815f..e7346f8edb38 100644 --- a/drivers/firmware/efi/libstub/efi-stub-helper.c +++ b/drivers/firmware/efi/libstub/efi-stub-helper.c @@ -327,7 +327,7 @@ fail: * Size of memory allocated return in *cmd_line_len. * Returns NULL on error. */ -char *efi_convert_cmdline(efi_loaded_image_t *image, int *cmd_line_len) +char *efi_convert_cmdline(efi_loaded_image_t *image) { const efi_char16_t *options = efi_table_attr(image, load_options); u32 options_size = efi_table_attr(image, load_options_size); @@ -405,7 +405,6 @@ char *efi_convert_cmdline(efi_loaded_image_t *image, int *cmd_line_len) snprintf((char *)cmdline_addr, options_bytes, "%.*ls", options_bytes - 1, options); - *cmd_line_len = options_bytes; return (char *)cmdline_addr; } diff --git a/drivers/firmware/efi/libstub/efi-stub.c b/drivers/firmware/efi/libstub/efi-stub.c index 2a1b43f9e0fa..f09e277ba210 100644 --- a/drivers/firmware/efi/libstub/efi-stub.c +++ b/drivers/firmware/efi/libstub/efi-stub.c @@ -112,7 +112,6 @@ static u32 get_supported_rt_services(void) efi_status_t efi_handle_cmdline(efi_loaded_image_t *image, char **cmdline_ptr) { - int cmdline_size = 0; efi_status_t status; char *cmdline; @@ -121,7 +120,7 @@ efi_status_t efi_handle_cmdline(efi_loaded_image_t *image, char **cmdline_ptr) * protocol. We are going to copy the command line into the * device tree, so this can be allocated anywhere. */ - cmdline = efi_convert_cmdline(image, &cmdline_size); + cmdline = efi_convert_cmdline(image); if (!cmdline) { efi_err("getting command line via LOADED_IMAGE_PROTOCOL\n"); return EFI_OUT_OF_RESOURCES; @@ -137,7 +136,7 @@ efi_status_t efi_handle_cmdline(efi_loaded_image_t *image, char **cmdline_ptr) } } - if (!IS_ENABLED(CONFIG_CMDLINE_FORCE) && cmdline_size > 0) { + if (!IS_ENABLED(CONFIG_CMDLINE_FORCE)) { status = efi_parse_options(cmdline); if (status != EFI_SUCCESS) { efi_err("Failed to parse options\n"); diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h index 685098f9626f..76e44c185f29 100644 --- a/drivers/firmware/efi/libstub/efistub.h +++ b/drivers/firmware/efi/libstub/efistub.h @@ -1056,7 +1056,7 @@ void efi_free(unsigned long size, unsigned long addr); void efi_apply_loadoptions_quirk(const void **load_options, u32 *load_options_size); -char *efi_convert_cmdline(efi_loaded_image_t *image, int *cmd_line_len); +char *efi_convert_cmdline(efi_loaded_image_t *image); efi_status_t efi_get_memory_map(struct efi_boot_memmap **map, bool install_cfg_tbl); diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c index f8e465da344d..188c8000d245 100644 --- a/drivers/firmware/efi/libstub/x86-stub.c +++ b/drivers/firmware/efi/libstub/x86-stub.c @@ -537,7 +537,6 @@ efi_status_t __efiapi efi_pe_entry(efi_handle_t handle, efi_guid_t proto = LOADED_IMAGE_PROTOCOL_GUID; struct boot_params *boot_params; struct setup_header *hdr; - int options_size = 0; efi_status_t status; unsigned long alloc; char *cmdline_ptr; @@ -569,7 +568,7 @@ efi_status_t __efiapi efi_pe_entry(efi_handle_t handle, hdr->initrd_addr_max = INT_MAX; /* Convert unicode cmdline to ascii */ - cmdline_ptr = efi_convert_cmdline(image, &options_size); + cmdline_ptr = efi_convert_cmdline(image); if (!cmdline_ptr) { efi_free(PARAM_SIZE, alloc); efi_exit(handle, EFI_OUT_OF_RESOURCES); From c004703ed7ae6be30ee7dcb37801bda4c6a24c3b Mon Sep 17 00:00:00 2001 From: Jeremy Linton Date: Mon, 30 Sep 2024 22:20:28 -0500 Subject: [PATCH 03/16] efi/libstub: measure initrd to PCR9 independent of source Currently the initrd is only measured if it can be loaded using the INITRD_MEDIA_GUID, if we are loading it from a path provided via the command line it is never measured. Lets move the check down a couple lines so the measurement happens independent of the source. Signed-off-by: Jeremy Linton Reviewed-by: Ilias Apalodimas Signed-off-by: Ard Biesheuvel --- drivers/firmware/efi/libstub/efi-stub-helper.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c b/drivers/firmware/efi/libstub/efi-stub-helper.c index e7346f8edb38..c0c81ca4237e 100644 --- a/drivers/firmware/efi/libstub/efi-stub-helper.c +++ b/drivers/firmware/efi/libstub/efi-stub-helper.c @@ -620,10 +620,6 @@ efi_status_t efi_load_initrd(efi_loaded_image_t *image, status = efi_load_initrd_dev_path(&initrd, hard_limit); if (status == EFI_SUCCESS) { efi_info("Loaded initrd from LINUX_EFI_INITRD_MEDIA_GUID device path\n"); - if (initrd.size > 0 && - efi_measure_tagged_event(initrd.base, initrd.size, - EFISTUB_EVT_INITRD) == EFI_SUCCESS) - efi_info("Measured initrd data into PCR 9\n"); } else if (status == EFI_NOT_FOUND) { status = efi_load_initrd_cmdline(image, &initrd, soft_limit, hard_limit); @@ -636,6 +632,11 @@ efi_status_t efi_load_initrd(efi_loaded_image_t *image, if (status != EFI_SUCCESS) goto failed; + if (initrd.size > 0 && + efi_measure_tagged_event(initrd.base, initrd.size, + EFISTUB_EVT_INITRD) == EFI_SUCCESS) + efi_info("Measured initrd data into PCR 9\n"); + status = efi_bs_call(allocate_pool, EFI_LOADER_DATA, sizeof(initrd), (void **)&tbl); if (status != EFI_SUCCESS) From e6d654e9f5a97742cfe794b1c4bb5d3fb2d25e98 Mon Sep 17 00:00:00 2001 From: Gregory Price Date: Fri, 13 Sep 2024 19:19:51 -0400 Subject: [PATCH 04/16] tpm: fix signed/unsigned bug when checking event logs A prior bugfix that fixes a signed/unsigned error causes another signed unsigned error. A situation where log_tbl->size is invalid can cause the size passed to memblock_reserve to become negative. log_size from the main event log is an unsigned int, and the code reduces to the following u64 value = (int)unsigned_value; This results in sign extension, and the value sent to memblock_reserve becomes effectively negative. Fixes: be59d57f9806 ("efi/tpm: Fix sanity check of unsigned tbl_size being less than zero") Signed-off-by: Gregory Price Reviewed-by: Ilias Apalodimas Signed-off-by: Ard Biesheuvel --- drivers/firmware/efi/tpm.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/drivers/firmware/efi/tpm.c b/drivers/firmware/efi/tpm.c index e8d69bd548f3..9c3613e6af15 100644 --- a/drivers/firmware/efi/tpm.c +++ b/drivers/firmware/efi/tpm.c @@ -40,7 +40,8 @@ int __init efi_tpm_eventlog_init(void) { struct linux_efi_tpm_eventlog *log_tbl; struct efi_tcg2_final_events_table *final_tbl; - int tbl_size; + unsigned int tbl_size; + int final_tbl_size; int ret = 0; if (efi.tpm_log == EFI_INVALID_TABLE_ADDR) { @@ -80,26 +81,26 @@ int __init efi_tpm_eventlog_init(void) goto out; } - tbl_size = 0; + final_tbl_size = 0; if (final_tbl->nr_events != 0) { void *events = (void *)efi.tpm_final_log + sizeof(final_tbl->version) + sizeof(final_tbl->nr_events); - tbl_size = tpm2_calc_event_log_size(events, - final_tbl->nr_events, - log_tbl->log); + final_tbl_size = tpm2_calc_event_log_size(events, + final_tbl->nr_events, + log_tbl->log); } - if (tbl_size < 0) { + if (final_tbl_size < 0) { pr_err(FW_BUG "Failed to parse event in TPM Final Events Log\n"); ret = -EINVAL; goto out_calc; } memblock_reserve(efi.tpm_final_log, - tbl_size + sizeof(*final_tbl)); - efi_tpm_final_log_size = tbl_size; + final_tbl_size + sizeof(*final_tbl)); + efi_tpm_final_log_size = final_tbl_size; out_calc: early_memunmap(final_tbl, sizeof(*final_tbl)); From c33c28f9f6e017702374e1ab907dfa5a63db5301 Mon Sep 17 00:00:00 2001 From: Gregory Price Date: Fri, 13 Sep 2024 19:19:52 -0400 Subject: [PATCH 05/16] tpm: do not ignore memblock_reserve return value tpm code currently ignores a relevant failure case silently. Add an error to make this failure non-silent. Signed-off-by: Gregory Price Signed-off-by: Ard Biesheuvel --- drivers/firmware/efi/tpm.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/firmware/efi/tpm.c b/drivers/firmware/efi/tpm.c index 9c3613e6af15..b0cc2cc11d7e 100644 --- a/drivers/firmware/efi/tpm.c +++ b/drivers/firmware/efi/tpm.c @@ -61,7 +61,12 @@ int __init efi_tpm_eventlog_init(void) } tbl_size = sizeof(*log_tbl) + log_tbl->size; - memblock_reserve(efi.tpm_log, tbl_size); + if (memblock_reserve(efi.tpm_log, tbl_size)) { + pr_err("TPM Event Log memblock reserve fails (0x%lx, 0x%x)\n", + efi.tpm_log, tbl_size); + ret = -ENOMEM; + goto out; + } if (efi.tpm_final_log == EFI_INVALID_TABLE_ADDR) { pr_info("TPM Final Events table not present\n"); From a066397e8ed1036e8b959050ab6e830ee90d9f58 Mon Sep 17 00:00:00 2001 From: Gregory Price Date: Fri, 13 Sep 2024 19:19:53 -0400 Subject: [PATCH 06/16] tpm: fix unsigned/signed mismatch errors related to __calc_tpm2_event_size __calc_tpm2_event_size returns 0 or a positive length, but return values are often interpreted as ints. Convert everything over to u32 to avoid signed/unsigned logic errors. Signed-off-by: Gregory Price Signed-off-by: Ard Biesheuvel --- drivers/firmware/efi/libstub/tpm.c | 6 +++--- drivers/firmware/efi/tpm.c | 2 +- include/linux/tpm_eventlog.h | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/firmware/efi/libstub/tpm.c b/drivers/firmware/efi/libstub/tpm.c index 1fd6823248ab..d31ea3f351d8 100644 --- a/drivers/firmware/efi/libstub/tpm.c +++ b/drivers/firmware/efi/libstub/tpm.c @@ -57,7 +57,7 @@ static void efi_retrieve_tcg2_eventlog(int version, efi_physical_addr_t log_loca struct linux_efi_tpm_eventlog *log_tbl = NULL; unsigned long first_entry_addr, last_entry_addr; size_t log_size, last_entry_size; - int final_events_size = 0; + u32 final_events_size = 0; first_entry_addr = (unsigned long) log_location; @@ -110,9 +110,9 @@ static void efi_retrieve_tcg2_eventlog(int version, efi_physical_addr_t log_loca */ if (final_events_table && final_events_table->nr_events) { struct tcg_pcr_event2_head *header; - int offset; + u32 offset; void *data; - int event_size; + u32 event_size; int i = final_events_table->nr_events; data = (void *)final_events_table; diff --git a/drivers/firmware/efi/tpm.c b/drivers/firmware/efi/tpm.c index b0cc2cc11d7e..cdd431027065 100644 --- a/drivers/firmware/efi/tpm.c +++ b/drivers/firmware/efi/tpm.c @@ -19,7 +19,7 @@ EXPORT_SYMBOL(efi_tpm_final_log_size); static int __init tpm2_calc_event_log_size(void *data, int count, void *size_info) { struct tcg_pcr_event2_head *header; - int event_size, size = 0; + u32 event_size, size = 0; while (count > 0) { header = data + size; diff --git a/include/linux/tpm_eventlog.h b/include/linux/tpm_eventlog.h index 7d68a5cc5881..891368e82558 100644 --- a/include/linux/tpm_eventlog.h +++ b/include/linux/tpm_eventlog.h @@ -157,7 +157,7 @@ struct tcg_algorithm_info { * Return: size of the event on success, 0 on failure */ -static __always_inline int __calc_tpm2_event_size(struct tcg_pcr_event2_head *event, +static __always_inline u32 __calc_tpm2_event_size(struct tcg_pcr_event2_head *event, struct tcg_pcr_event *event_header, bool do_mapping) { From 63971b0f51faff0ff844a85d297e27861555c328 Mon Sep 17 00:00:00 2001 From: Gregory Price Date: Fri, 13 Sep 2024 19:19:54 -0400 Subject: [PATCH 07/16] libstub,tpm: do not ignore failure case when reading final event log Current code fails to check for an error case when reading events from final event log to calculate offsets. Check the error case, and break early because all subsequent calls will also fail. Signed-off-by: Gregory Price Signed-off-by: Ard Biesheuvel --- drivers/firmware/efi/libstub/tpm.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/firmware/efi/libstub/tpm.c b/drivers/firmware/efi/libstub/tpm.c index d31ea3f351d8..a5c6c4f163fc 100644 --- a/drivers/firmware/efi/libstub/tpm.c +++ b/drivers/firmware/efi/libstub/tpm.c @@ -124,6 +124,9 @@ static void efi_retrieve_tcg2_eventlog(int version, efi_physical_addr_t log_loca event_size = __calc_tpm2_event_size(header, (void *)(long)log_location, false); + /* If calc fails this is a malformed log */ + if (!event_size) + break; final_events_size += event_size; i--; } From 06d39d79cbd5a91a33707951ebf2512d0e759847 Mon Sep 17 00:00:00 2001 From: Ard Biesheuvel Date: Sun, 13 Oct 2024 15:19:04 +0200 Subject: [PATCH 08/16] efi/libstub: Free correct pointer on failure cmdline_ptr is an out parameter, which is not allocated by the function itself, and likely points into the caller's stack. cmdline refers to the pool allocation that should be freed when cleaning up after a failure, so pass this instead to free_pool(). Fixes: 42c8ea3dca09 ("efi: libstub: Factor out EFI stub entrypoint ...") Cc: Signed-off-by: Ard Biesheuvel --- drivers/firmware/efi/libstub/efi-stub.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/firmware/efi/libstub/efi-stub.c b/drivers/firmware/efi/libstub/efi-stub.c index f09e277ba210..fc71dcab43e0 100644 --- a/drivers/firmware/efi/libstub/efi-stub.c +++ b/drivers/firmware/efi/libstub/efi-stub.c @@ -148,7 +148,7 @@ efi_status_t efi_handle_cmdline(efi_loaded_image_t *image, char **cmdline_ptr) return EFI_SUCCESS; fail_free_cmdline: - efi_bs_call(free_pool, cmdline_ptr); + efi_bs_call(free_pool, cmdline); return status; } From 6fce6e9791685e95b70144a414eb90132e497489 Mon Sep 17 00:00:00 2001 From: Ard Biesheuvel Date: Sun, 13 Oct 2024 13:09:09 +0200 Subject: [PATCH 09/16] efi/zboot: Fix outdated comment about using LoadImage/StartImage EFI zboot no longer uses LoadImage/StartImage, but subsumes the arch code to load and start the bare metal image directly. Fix the Kconfig description accordingly. Signed-off-by: Ard Biesheuvel --- drivers/firmware/efi/Kconfig | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/drivers/firmware/efi/Kconfig b/drivers/firmware/efi/Kconfig index 72f2537d90ca..e312d731f4a3 100644 --- a/drivers/firmware/efi/Kconfig +++ b/drivers/firmware/efi/Kconfig @@ -84,12 +84,10 @@ config EFI_ZBOOT help Create the bootable image as an EFI application that carries the actual kernel image in compressed form, and decompresses it into - memory before executing it via LoadImage/StartImage EFI boot service - calls. For compatibility with non-EFI loaders, the payload can be - decompressed and executed by the loader as well, provided that the - loader implements the decompression algorithm and that non-EFI boot - is supported by the encapsulated image. (The compression algorithm - used is described in the zboot image header) + memory before executing it. For compatibility with non-EFI loaders, + the payload can be decompressed and executed by the loader as well, + provided that the loader implements the decompression algorithm. + (The compression algorithm used is described in the zboot header) config EFI_ARMSTUB_DTB_LOADER bool "Enable the DTB loader" From 8fbe4c49c0ccac9a6a3cff35a45fa55d4ae35d6e Mon Sep 17 00:00:00 2001 From: Ard Biesheuvel Date: Thu, 31 Oct 2024 18:58:23 +0100 Subject: [PATCH 10/16] efi/memattr: Ignore table if the size is clearly bogus There are reports [0] of cases where a corrupt EFI Memory Attributes Table leads to out of memory issues at boot because the descriptor size and entry count in the table header are still used to reserve the entire table in memory, even though the resulting region is gigabytes in size. Given that the EFI Memory Attributes Table is supposed to carry up to 3 entries for each EfiRuntimeServicesCode region in the EFI memory map, and given that there is no reason for the descriptor size used in the table to exceed the one used in the EFI memory map, 3x the size of the entire EFI memory map is a reasonable upper bound for the size of this table. This means that sizes exceeding that are highly likely to be based on corrupted data, and the table should just be ignored instead. [0] https://bugzilla.suse.com/show_bug.cgi?id=1231465 Cc: Gregory Price Cc: Usama Arif Acked-by: Jiri Slaby Acked-by: Breno Leitao Link: https://lore.kernel.org/all/20240912155159.1951792-2-ardb+git@google.com/ Signed-off-by: Ard Biesheuvel --- drivers/firmware/efi/memattr.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/drivers/firmware/efi/memattr.c b/drivers/firmware/efi/memattr.c index 164203429fa7..c38b1a335590 100644 --- a/drivers/firmware/efi/memattr.c +++ b/drivers/firmware/efi/memattr.c @@ -22,6 +22,7 @@ unsigned long __ro_after_init efi_mem_attr_table = EFI_INVALID_TABLE_ADDR; int __init efi_memattr_init(void) { efi_memory_attributes_table_t *tbl; + unsigned long size; if (efi_mem_attr_table == EFI_INVALID_TABLE_ADDR) return 0; @@ -39,7 +40,22 @@ int __init efi_memattr_init(void) goto unmap; } - tbl_size = sizeof(*tbl) + tbl->num_entries * tbl->desc_size; + + /* + * Sanity check: the Memory Attributes Table contains up to 3 entries + * for each entry of type EfiRuntimeServicesCode in the EFI memory map. + * So if the size of the table exceeds 3x the size of the entire EFI + * memory map, there is clearly something wrong, and the table should + * just be ignored altogether. + */ + size = tbl->num_entries * tbl->desc_size; + if (size > 3 * efi.memmap.nr_map * efi.memmap.desc_size) { + pr_warn(FW_BUG "Corrupted EFI Memory Attributes Table detected! (version == %u, desc_size == %u, num_entries == %u)\n", + tbl->version, tbl->desc_size, tbl->num_entries); + goto unmap; + } + + tbl_size = sizeof(*tbl) + size; memblock_reserve(efi_mem_attr_table, tbl_size); set_bit(EFI_MEM_ATTR, &efi.flags); From 7eb4e1dd71009472215bc1f8226ee9a041da8d37 Mon Sep 17 00:00:00 2001 From: Nicolas Saenz Julienne Date: Tue, 12 Nov 2024 18:52:16 +0000 Subject: [PATCH 11/16] x86/efi: Drop support for the EFI_PROPERTIES_TABLE Drop support for the EFI_PROPERTIES_TABLE. It was a failed, short-lived experiment that broke the boot both on Linux and Windows, and was replaced by the EFI_MEMORY_ATTRIBUTES_TABLE shortly after. Suggested-by: Ard Biesheuvel Signed-off-by: Nicolas Saenz Julienne Signed-off-by: Ard Biesheuvel --- arch/x86/platform/efi/efi.c | 19 --------------- arch/x86/platform/efi/efi_64.c | 42 ---------------------------------- include/linux/efi.h | 17 +++----------- 3 files changed, 3 insertions(+), 75 deletions(-) diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c index 88a96816de9a..375ebd78296a 100644 --- a/arch/x86/platform/efi/efi.c +++ b/arch/x86/platform/efi/efi.c @@ -54,14 +54,12 @@ #include static unsigned long efi_systab_phys __initdata; -static unsigned long prop_phys = EFI_INVALID_TABLE_ADDR; static unsigned long uga_phys = EFI_INVALID_TABLE_ADDR; static unsigned long efi_runtime, efi_nr_tables; unsigned long efi_fw_vendor, efi_config_table; static const efi_config_table_type_t arch_tables[] __initconst = { - {EFI_PROPERTIES_TABLE_GUID, &prop_phys, "PROP" }, {UGA_IO_PROTOCOL_GUID, &uga_phys, "UGA" }, #ifdef CONFIG_X86_UV {UV_SYSTEM_TABLE_GUID, &uv_systab_phys, "UVsystab" }, @@ -82,7 +80,6 @@ static const unsigned long * const efi_tables[] = { &efi_runtime, &efi_config_table, &efi.esrt, - &prop_phys, &efi_mem_attr_table, #ifdef CONFIG_EFI_RCI2_TABLE &rci2_table_phys, @@ -502,22 +499,6 @@ void __init efi_init(void) return; } - /* Parse the EFI Properties table if it exists */ - if (prop_phys != EFI_INVALID_TABLE_ADDR) { - efi_properties_table_t *tbl; - - tbl = early_memremap_ro(prop_phys, sizeof(*tbl)); - if (tbl == NULL) { - pr_err("Could not map Properties table!\n"); - } else { - if (tbl->memory_protection_attribute & - EFI_PROPERTIES_RUNTIME_MEMORY_PROTECTION_NON_EXECUTABLE_PE_DATA) - set_bit(EFI_NX_PE_DATA, &efi.flags); - - early_memunmap(tbl, sizeof(*tbl)); - } - } - set_bit(EFI_RUNTIME_SERVICES, &efi.flags); efi_clean_memmap(); diff --git a/arch/x86/platform/efi/efi_64.c b/arch/x86/platform/efi/efi_64.c index 91d31ac422d6..ac57259a432b 100644 --- a/arch/x86/platform/efi/efi_64.c +++ b/arch/x86/platform/efi/efi_64.c @@ -412,51 +412,9 @@ static int __init efi_update_mem_attr(struct mm_struct *mm, efi_memory_desc_t *m void __init efi_runtime_update_mappings(void) { - efi_memory_desc_t *md; - - /* - * Use the EFI Memory Attribute Table for mapping permissions if it - * exists, since it is intended to supersede EFI_PROPERTIES_TABLE. - */ if (efi_enabled(EFI_MEM_ATTR)) { efi_disable_ibt_for_runtime = false; efi_memattr_apply_permissions(NULL, efi_update_mem_attr); - return; - } - - /* - * EFI_MEMORY_ATTRIBUTES_TABLE is intended to replace - * EFI_PROPERTIES_TABLE. So, use EFI_PROPERTIES_TABLE to update - * permissions only if EFI_MEMORY_ATTRIBUTES_TABLE is not - * published by the firmware. Even if we find a buggy implementation of - * EFI_MEMORY_ATTRIBUTES_TABLE, don't fall back to - * EFI_PROPERTIES_TABLE, because of the same reason. - */ - - if (!efi_enabled(EFI_NX_PE_DATA)) - return; - - for_each_efi_memory_desc(md) { - unsigned long pf = 0; - - if (!(md->attribute & EFI_MEMORY_RUNTIME)) - continue; - - if (!(md->attribute & EFI_MEMORY_WB)) - pf |= _PAGE_PCD; - - if ((md->attribute & EFI_MEMORY_XP) || - (md->type == EFI_RUNTIME_SERVICES_DATA)) - pf |= _PAGE_NX; - - if (!(md->attribute & EFI_MEMORY_RO) && - (md->type != EFI_RUNTIME_SERVICES_CODE)) - pf |= _PAGE_RW; - - if (cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT)) - pf |= _PAGE_ENC; - - efi_update_mappings(md, pf); } } diff --git a/include/linux/efi.h b/include/linux/efi.h index e28d88066033..e5815867aba9 100644 --- a/include/linux/efi.h +++ b/include/linux/efi.h @@ -379,7 +379,6 @@ void efi_native_runtime_setup(void); #define EFI_SYSTEM_RESOURCE_TABLE_GUID EFI_GUID(0xb122a263, 0x3661, 0x4f68, 0x99, 0x29, 0x78, 0xf8, 0xb0, 0xd6, 0x21, 0x80) #define EFI_FILE_SYSTEM_GUID EFI_GUID(0x964e5b22, 0x6459, 0x11d2, 0x8e, 0x39, 0x00, 0xa0, 0xc9, 0x69, 0x72, 0x3b) #define DEVICE_TREE_GUID EFI_GUID(0xb1b621d5, 0xf19c, 0x41a5, 0x83, 0x0b, 0xd9, 0x15, 0x2c, 0x69, 0xaa, 0xe0) -#define EFI_PROPERTIES_TABLE_GUID EFI_GUID(0x880aaca3, 0x4adc, 0x4a04, 0x90, 0x79, 0xb7, 0x47, 0x34, 0x08, 0x25, 0xe5) #define EFI_RNG_PROTOCOL_GUID EFI_GUID(0x3152bca5, 0xeade, 0x433d, 0x86, 0x2e, 0xc0, 0x1c, 0xdc, 0x29, 0x1f, 0x44) #define EFI_RNG_ALGORITHM_RAW EFI_GUID(0xe43176d7, 0xb6e8, 0x4827, 0xb7, 0x84, 0x7f, 0xfd, 0xc4, 0xb6, 0x85, 0x61) #define EFI_MEMORY_ATTRIBUTES_TABLE_GUID EFI_GUID(0xdcfa911d, 0x26eb, 0x469f, 0xa2, 0x20, 0x38, 0xb7, 0xdc, 0x46, 0x12, 0x20) @@ -580,15 +579,6 @@ struct efi_mem_range { u64 attribute; }; -typedef struct { - u32 version; - u32 length; - u64 memory_protection_attribute; -} efi_properties_table_t; - -#define EFI_PROPERTIES_TABLE_VERSION 0x00010000 -#define EFI_PROPERTIES_RUNTIME_MEMORY_PROTECTION_NON_EXECUTABLE_PE_DATA 0x1 - typedef struct { u16 version; u16 length; @@ -871,10 +861,9 @@ static inline int efi_range_is_wc(unsigned long start, unsigned long len) #define EFI_PARAVIRT 6 /* Access is via a paravirt interface */ #define EFI_ARCH_1 7 /* First arch-specific bit */ #define EFI_DBG 8 /* Print additional debug info at runtime */ -#define EFI_NX_PE_DATA 9 /* Can runtime data regions be mapped non-executable? */ -#define EFI_MEM_ATTR 10 /* Did firmware publish an EFI_MEMORY_ATTRIBUTES table? */ -#define EFI_MEM_NO_SOFT_RESERVE 11 /* Is the kernel configured to ignore soft reservations? */ -#define EFI_PRESERVE_BS_REGIONS 12 /* Are EFI boot-services memory segments available? */ +#define EFI_MEM_ATTR 9 /* Did firmware publish an EFI_MEMORY_ATTRIBUTES table? */ +#define EFI_MEM_NO_SOFT_RESERVE 10 /* Is the kernel configured to ignore soft reservations? */ +#define EFI_PRESERVE_BS_REGIONS 11 /* Are EFI boot-services memory segments available? */ #ifdef CONFIG_EFI /* From 21b1a7f7ae2f53c914f584a72a85cb4f71227e28 Mon Sep 17 00:00:00 2001 From: Nicolas Saenz Julienne Date: Tue, 12 Nov 2024 18:52:17 +0000 Subject: [PATCH 12/16] x86/efi: Apply EFI Memory Attributes after kexec Kexec bypasses EFI's switch to virtual mode. In exchange, it has its own routine, kexec_enter_virtual_mode(), which replays the mappings made by the original kernel. Unfortunately, that function fails to reinstate EFI's memory attributes, which would've otherwise been set after entering virtual mode. Remediate this by calling efi_runtime_update_mappings() within kexec's routine. Signed-off-by: Nicolas Saenz Julienne Signed-off-by: Ard Biesheuvel --- arch/x86/platform/efi/efi.c | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c index 375ebd78296a..a7ff189421c3 100644 --- a/arch/x86/platform/efi/efi.c +++ b/arch/x86/platform/efi/efi.c @@ -765,6 +765,7 @@ static void __init kexec_enter_virtual_mode(void) efi_sync_low_kernel_mappings(); efi_native_runtime_setup(); + efi_runtime_update_mappings(); #endif } From e6384c398459c40e9304fbb478079884a5967bd4 Mon Sep 17 00:00:00 2001 From: Ard Biesheuvel Date: Sun, 13 Oct 2024 15:50:03 +0200 Subject: [PATCH 13/16] efi/libstub: Parse builtin command line after bootloader provided one When CONFIG_CMDLINE_EXTEND is set, the core kernel command line handling logic appends CONFIG_CMDLINE to the bootloader provided command line. The EFI stub does the opposite, and parses the builtin one first. The usual behavior of command line options is that the last one takes precedence if it appears multiple times, unless there is a meaningful way to combine them. In either case, parsing the builtin command line first while the core kernel does it in the opposite order is likely to produce inconsistent results in such cases. Therefore, switch the order in the stub to match the core kernel. Signed-off-by: Ard Biesheuvel --- drivers/firmware/efi/libstub/efi-stub.c | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/drivers/firmware/efi/libstub/efi-stub.c b/drivers/firmware/efi/libstub/efi-stub.c index fc71dcab43e0..382b54f40603 100644 --- a/drivers/firmware/efi/libstub/efi-stub.c +++ b/drivers/firmware/efi/libstub/efi-stub.c @@ -126,28 +126,25 @@ efi_status_t efi_handle_cmdline(efi_loaded_image_t *image, char **cmdline_ptr) return EFI_OUT_OF_RESOURCES; } + if (!IS_ENABLED(CONFIG_CMDLINE_FORCE)) { + status = efi_parse_options(cmdline); + if (status != EFI_SUCCESS) + goto fail_free_cmdline; + } + if (IS_ENABLED(CONFIG_CMDLINE_EXTEND) || IS_ENABLED(CONFIG_CMDLINE_FORCE) || cmdline[0] == 0) { status = efi_parse_options(CONFIG_CMDLINE); - if (status != EFI_SUCCESS) { - efi_err("Failed to parse options\n"); + if (status != EFI_SUCCESS) goto fail_free_cmdline; - } - } - - if (!IS_ENABLED(CONFIG_CMDLINE_FORCE)) { - status = efi_parse_options(cmdline); - if (status != EFI_SUCCESS) { - efi_err("Failed to parse options\n"); - goto fail_free_cmdline; - } } *cmdline_ptr = cmdline; return EFI_SUCCESS; fail_free_cmdline: + efi_err("Failed to parse options\n"); efi_bs_call(free_pool, cmdline); return status; } From 80d01ce607cbffd8fa6ceb8a91ce07667bc51d5a Mon Sep 17 00:00:00 2001 From: Ard Biesheuvel Date: Sun, 13 Oct 2024 11:20:45 +0200 Subject: [PATCH 14/16] efi/libstub: Fix command line fallback handling when loading files CONFIG_CMDLINE, when set, is supposed to serve either as a fallback when no command line is provided by the bootloader, or to be taken into account unconditionally, depending on the configured options. The initrd and dtb loader ignores CONFIG_CMDLINE in either case, and only takes the EFI firmware provided load options into account. This means that configuring the kernel with initrd= or dtb= on the built-in command line does not produce the expected result. Fix this by doing a separate pass over the built-in command line when dealing with initrd= or dtb= options. Signed-off-by: Ard Biesheuvel --- drivers/firmware/efi/libstub/file.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/drivers/firmware/efi/libstub/file.c b/drivers/firmware/efi/libstub/file.c index d6a025df07dc..33d45d1b274c 100644 --- a/drivers/firmware/efi/libstub/file.c +++ b/drivers/firmware/efi/libstub/file.c @@ -175,6 +175,12 @@ static efi_status_t efi_open_device_path(efi_file_protocol_t **volume, return status; } +#ifndef CONFIG_CMDLINE +#define CONFIG_CMDLINE +#endif + +static const efi_char16_t builtin_cmdline[] = L"" CONFIG_CMDLINE; + /* * Check the cmdline for a LILO-style file= arguments. * @@ -189,6 +195,7 @@ efi_status_t handle_cmdline_files(efi_loaded_image_t *image, unsigned long *load_addr, unsigned long *load_size) { + const bool ignore_load_options = false; const efi_char16_t *cmdline = efi_table_attr(image, load_options); u32 cmdline_len = efi_table_attr(image, load_options_size); unsigned long efi_chunk_size = ULONG_MAX; @@ -197,6 +204,7 @@ efi_status_t handle_cmdline_files(efi_loaded_image_t *image, unsigned long alloc_addr; unsigned long alloc_size; efi_status_t status; + bool twopass; int offset; if (!load_addr || !load_size) @@ -209,6 +217,16 @@ efi_status_t handle_cmdline_files(efi_loaded_image_t *image, efi_chunk_size = EFI_READ_CHUNK_SIZE; alloc_addr = alloc_size = 0; + + if (!ignore_load_options && cmdline_len > 0) { + twopass = IS_ENABLED(CONFIG_CMDLINE_BOOL) || + IS_ENABLED(CONFIG_CMDLINE_EXTEND); + } else { +do_builtin: cmdline = builtin_cmdline; + cmdline_len = ARRAY_SIZE(builtin_cmdline) - 1; + twopass = false; + } + do { struct finfo fi; unsigned long size; @@ -290,6 +308,9 @@ efi_status_t handle_cmdline_files(efi_loaded_image_t *image, efi_call_proto(volume, close); } while (offset > 0); + if (twopass) + goto do_builtin; + *load_addr = alloc_addr; *load_size = alloc_size; From 851062278436c9a887749e7b73598a28dd902ac0 Mon Sep 17 00:00:00 2001 From: Ard Biesheuvel Date: Sun, 13 Oct 2024 13:10:38 +0200 Subject: [PATCH 15/16] efi/libstub: Take command line overrides into account for loaded files When CONFIG_CMDLINE_OVERRIDE or CONFIG_CMDLINE_FORCE are configured, the command line provided by the boot stack should be ignored, and only the built-in command line should be taken into account. Add the required handling of this when dealing with initrd= or dtb= command line options in the EFI stub. Signed-off-by: Ard Biesheuvel --- drivers/firmware/efi/libstub/file.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/firmware/efi/libstub/file.c b/drivers/firmware/efi/libstub/file.c index 33d45d1b274c..bd626d55dcbc 100644 --- a/drivers/firmware/efi/libstub/file.c +++ b/drivers/firmware/efi/libstub/file.c @@ -195,7 +195,8 @@ efi_status_t handle_cmdline_files(efi_loaded_image_t *image, unsigned long *load_addr, unsigned long *load_size) { - const bool ignore_load_options = false; + const bool ignore_load_options = IS_ENABLED(CONFIG_CMDLINE_OVERRIDE) || + IS_ENABLED(CONFIG_CMDLINE_FORCE); const efi_char16_t *cmdline = efi_table_attr(image, load_options); u32 cmdline_len = efi_table_attr(image, load_options_size); unsigned long efi_chunk_size = ULONG_MAX; From c5d91b16f525ea8c98b3fd8efc5105106d17fe9a Mon Sep 17 00:00:00 2001 From: Cyrill Gorcunov Date: Mon, 4 Nov 2024 15:13:55 +0300 Subject: [PATCH 16/16] efi: Fix memory leak in efivar_ssdt_load When we load SSDT from efi variable (specified with efivar_ssdt= boot command line argument) a name for the variable is allocated dynamically because we traverse all EFI variables. Unlike ACPI table data, which is later used by ACPI engine, the name is no longer needed once traverse is complete -- don't forget to free this memory. Same time we silently ignore any errors happened here let's print a message if something went wrong (but do not exit since this is not a critical error and the system should continue to boot). Also while here -- add a note why we keep SSDT table on success. Signed-off-by: Cyrill Gorcunov Signed-off-by: Ard Biesheuvel --- drivers/firmware/efi/efi.c | 41 ++++++++++++++++++++++++++------------ 1 file changed, 28 insertions(+), 13 deletions(-) diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c index 70490bf2697b..60c64b81d2c3 100644 --- a/drivers/firmware/efi/efi.c +++ b/drivers/firmware/efi/efi.c @@ -273,6 +273,7 @@ static __init int efivar_ssdt_load(void) efi_char16_t *name = NULL; efi_status_t status; efi_guid_t guid; + int ret = 0; if (!efivar_ssdt[0]) return 0; @@ -294,8 +295,8 @@ static __init int efivar_ssdt_load(void) efi_char16_t *name_tmp = krealloc(name, name_size, GFP_KERNEL); if (!name_tmp) { - kfree(name); - return -ENOMEM; + ret = -ENOMEM; + goto out; } name = name_tmp; continue; @@ -309,26 +310,38 @@ static __init int efivar_ssdt_load(void) pr_info("loading SSDT from variable %s-%pUl\n", efivar_ssdt, &guid); status = efi.get_variable(name, &guid, NULL, &data_size, NULL); - if (status != EFI_BUFFER_TOO_SMALL || !data_size) - return -EIO; + if (status != EFI_BUFFER_TOO_SMALL || !data_size) { + ret = -EIO; + goto out; + } data = kmalloc(data_size, GFP_KERNEL); - if (!data) - return -ENOMEM; + if (!data) { + ret = -ENOMEM; + goto out; + } status = efi.get_variable(name, &guid, NULL, &data_size, data); if (status == EFI_SUCCESS) { - acpi_status ret = acpi_load_table(data, NULL); - if (ret) - pr_err("failed to load table: %u\n", ret); - else + acpi_status acpi_ret = acpi_load_table(data, NULL); + if (ACPI_FAILURE(acpi_ret)) { + pr_err("efivar_ssdt: failed to load table: %u\n", + acpi_ret); + } else { + /* + * The @data will be in use by ACPI engine, + * do not free it! + */ continue; + } } else { - pr_err("failed to get var data: 0x%lx\n", status); + pr_err("efivar_ssdt: failed to get var data: 0x%lx\n", status); } kfree(data); } - return 0; +out: + kfree(name); + return ret; } #else static inline int efivar_ssdt_load(void) { return 0; } @@ -433,7 +446,9 @@ static int __init efisubsys_init(void) error = generic_ops_register(); if (error) goto err_put; - efivar_ssdt_load(); + error = efivar_ssdt_load(); + if (error) + pr_err("efi: failed to load SSDT, error %d.\n", error); platform_device_register_simple("efivars", 0, NULL, 0); }