From 24427cda90cbbb9015c73e2dd3329a116a00c8de Mon Sep 17 00:00:00 2001 From: Tim Schumacher Date: Thu, 28 Mar 2024 21:50:30 +0100 Subject: [PATCH 1/5] efi: pstore: Request at most 512 bytes for variable names Work around a quirk in a few old (2011-ish) UEFI implementations, where a call to `GetNextVariableName` with a buffer size larger than 512 bytes will always return EFI_INVALID_PARAMETER. This was already done to efivarfs in commit f45812cc23fb ("efivarfs: Request at most 512 bytes for variable names"), but the second copy of the variable iteration implementation was overlooked. Signed-off-by: Tim Schumacher Signed-off-by: Ard Biesheuvel --- drivers/firmware/efi/efi-pstore.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/drivers/firmware/efi/efi-pstore.c b/drivers/firmware/efi/efi-pstore.c index 833cbb995dd3..5b9dc26e6bcb 100644 --- a/drivers/firmware/efi/efi-pstore.c +++ b/drivers/firmware/efi/efi-pstore.c @@ -162,7 +162,15 @@ static ssize_t efi_pstore_read(struct pstore_record *record) efi_status_t status; for (;;) { - varname_size = 1024; + /* + * A small set of old UEFI implementations reject sizes + * above a certain threshold, the lowest seen in the wild + * is 512. + * + * TODO: Commonize with the iteration implementation in + * fs/efivarfs to keep all the quirks in one place. + */ + varname_size = 512; /* * If this is the first read() call in the pstore enumeration, From 5b625181fbde4a27f84fd97eb16ff7513388d9a8 Mon Sep 17 00:00:00 2001 From: Tim Schumacher Date: Thu, 28 Mar 2024 21:50:31 +0100 Subject: [PATCH 2/5] Documentation: Mark the 'efivars' sysfs interface as removed The 'efivars' sysfs interface was removed in commit 0f5b2c69a4cb ("efi: vars: Remove deprecated 'efivars' sysfs interface"), but the ABI documentation was not updated properly. Strip down the documentation file for /sys/firmware/efi/vars to a very basic description of what the interface was about, add a section about the rough removal timeline, and inform the reader about the intended replacement. Signed-off-by: Tim Schumacher Signed-off-by: Ard Biesheuvel --- .../ABI/removed/sysfs-firmware-efi-vars | 12 +++ .../ABI/stable/sysfs-firmware-efi-vars | 79 ------------------- Documentation/filesystems/efivarfs.rst | 2 +- 3 files changed, 13 insertions(+), 80 deletions(-) create mode 100644 Documentation/ABI/removed/sysfs-firmware-efi-vars delete mode 100644 Documentation/ABI/stable/sysfs-firmware-efi-vars diff --git a/Documentation/ABI/removed/sysfs-firmware-efi-vars b/Documentation/ABI/removed/sysfs-firmware-efi-vars new file mode 100644 index 000000000000..8d97368b149b --- /dev/null +++ b/Documentation/ABI/removed/sysfs-firmware-efi-vars @@ -0,0 +1,12 @@ +What: /sys/firmware/efi/vars +Date: April 2004, removed March 2023 +Description: + This directory exposed interfaces for interacting with + EFI variables. For more information on EFI variables, + see 'Variable Services' in the UEFI specification + (section 7.2 in specification version 2.3 Errata D). + + The 'efivars' sysfs interface was removed in March of 2023, + after being considered deprecated no later than September + of 2020. Its functionality has been replaced by the + 'efivarfs' filesystem. diff --git a/Documentation/ABI/stable/sysfs-firmware-efi-vars b/Documentation/ABI/stable/sysfs-firmware-efi-vars deleted file mode 100644 index 46ccd233e359..000000000000 --- a/Documentation/ABI/stable/sysfs-firmware-efi-vars +++ /dev/null @@ -1,79 +0,0 @@ -What: /sys/firmware/efi/vars -Date: April 2004 -Contact: Matt Domsch -Description: - This directory exposes interfaces for interactive with - EFI variables. For more information on EFI variables, - see 'Variable Services' in the UEFI specification - (section 7.2 in specification version 2.3 Errata D). - - In summary, EFI variables are named, and are classified - into separate namespaces through the use of a vendor - GUID. They also have an arbitrary binary value - associated with them. - - The efivars module enumerates these variables and - creates a separate directory for each one found. Each - directory has a name of the form "-" - and contains the following files: - - =============== ======================================== - attributes: A read-only text file enumerating the - EFI variable flags. Potential values - include: - - EFI_VARIABLE_NON_VOLATILE - EFI_VARIABLE_BOOTSERVICE_ACCESS - EFI_VARIABLE_RUNTIME_ACCESS - EFI_VARIABLE_HARDWARE_ERROR_RECORD - EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS - - See the EFI documentation for an - explanation of each of these variables. - - data: A read-only binary file that can be read - to attain the value of the EFI variable - - guid: The vendor GUID of the variable. This - should always match the GUID in the - variable's name. - - raw_var: A binary file that can be read to obtain - a structure that contains everything - there is to know about the variable. - For structure definition see "struct - efi_variable" in the kernel sources. - - This file can also be written to in - order to update the value of a variable. - For this to work however, all fields of - the "struct efi_variable" passed must - match byte for byte with the structure - read out of the file, save for the value - portion. - - **Note** the efi_variable structure - read/written with this file contains a - 'long' type that may change widths - depending on your underlying - architecture. - - size: As ASCII representation of the size of - the variable's value. - =============== ======================================== - - - In addition, two other magic binary files are provided - in the top-level directory and are used for adding and - removing variables: - - =============== ======================================== - new_var: Takes a "struct efi_variable" and - instructs the EFI firmware to create a - new variable. - - del_var: Takes a "struct efi_variable" and - instructs the EFI firmware to remove any - variable that has a matching vendor GUID - and variable key name. - =============== ======================================== diff --git a/Documentation/filesystems/efivarfs.rst b/Documentation/filesystems/efivarfs.rst index 0551985821b8..f646c3f0980f 100644 --- a/Documentation/filesystems/efivarfs.rst +++ b/Documentation/filesystems/efivarfs.rst @@ -40,4 +40,4 @@ accidentally. *See also:* - Documentation/admin-guide/acpi/ssdt-overlays.rst -- Documentation/ABI/stable/sysfs-firmware-efi-vars +- Documentation/ABI/removed/sysfs-firmware-efi-vars From 89ea21d70d9c9968dfd10c7e30520d0f03d465c2 Mon Sep 17 00:00:00 2001 From: Tim Schumacher Date: Thu, 28 Mar 2024 21:50:32 +0100 Subject: [PATCH 3/5] efivarfs: Remove unused internal struct members The structure was moved to the efivarfs internals in commit 2d82e6227ea1 ("efi: vars: Move efivar caching layer into efivarfs") after previously being used as the data ABI for efivars until its removal in commit 0f5b2c69a4cb ("efi: vars: Remove deprecated 'efivars' sysfs interface"). As efivarfs only uses the structure for the variable name caching layer, the data-related members were never in use. Remove them to avoid implying that efivarfs is bound by the same restrictions that efivars once had. While at it, remove the packed attribute, since we no longer have to guarantee a stable layout. Signed-off-by: Tim Schumacher Signed-off-by: Ard Biesheuvel --- fs/efivarfs/internal.h | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/fs/efivarfs/internal.h b/fs/efivarfs/internal.h index f7206158ee81..d71d2e08422f 100644 --- a/fs/efivarfs/internal.h +++ b/fs/efivarfs/internal.h @@ -24,11 +24,8 @@ struct efivarfs_fs_info { struct efi_variable { efi_char16_t VariableName[EFI_VAR_NAME_LEN/sizeof(efi_char16_t)]; efi_guid_t VendorGuid; - unsigned long DataSize; - __u8 Data[1024]; - efi_status_t Status; __u32 Attributes; -} __attribute__((packed)); +}; struct efivar_entry { struct efi_variable var; From cda30c6542c8bb445bc84f6616cac8d012547f0a Mon Sep 17 00:00:00 2001 From: Tim Schumacher Date: Thu, 28 Mar 2024 21:50:33 +0100 Subject: [PATCH 4/5] efi: Clear up misconceptions about a maximum variable name size The UEFI specification does not make any mention of a maximum variable name size, so the headers and implementation shouldn't claim that one exists either. Comments referring to this limit have been removed or rewritten, as this is an implementation detail local to the Linux kernel. Where appropriate, the magic value of 1024 has been replaced with EFI_VAR_NAME_LEN, as this is used for the efi_variable struct definition. This in itself does not change any behavior, but should serve as points of interest when making future changes in the same area. A related build-time check has been added to ensure that the special 512 byte sized buffer will not overflow with a potentially decreased EFI_VAR_NAME_LEN. Signed-off-by: Tim Schumacher Signed-off-by: Ard Biesheuvel --- drivers/firmware/efi/vars.c | 2 +- fs/efivarfs/vars.c | 5 +++-- include/linux/efi.h | 9 ++++----- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/firmware/efi/vars.c b/drivers/firmware/efi/vars.c index f654e6f6af87..4056ba7f3440 100644 --- a/drivers/firmware/efi/vars.c +++ b/drivers/firmware/efi/vars.c @@ -215,7 +215,7 @@ efi_status_t efivar_set_variable_locked(efi_char16_t *name, efi_guid_t *vendor, if (data_size > 0) { status = check_var_size(nonblocking, attr, - data_size + ucs2_strsize(name, 1024)); + data_size + ucs2_strsize(name, EFI_VAR_NAME_LEN)); if (status != EFI_SUCCESS) return status; } diff --git a/fs/efivarfs/vars.c b/fs/efivarfs/vars.c index 4d722af1014f..3cc89bb624f0 100644 --- a/fs/efivarfs/vars.c +++ b/fs/efivarfs/vars.c @@ -295,9 +295,9 @@ static bool variable_is_present(efi_char16_t *variable_name, efi_guid_t *vendor, unsigned long strsize1, strsize2; bool found = false; - strsize1 = ucs2_strsize(variable_name, 1024); + strsize1 = ucs2_strsize(variable_name, EFI_VAR_NAME_LEN); list_for_each_entry_safe(entry, n, head, list) { - strsize2 = ucs2_strsize(entry->var.VariableName, 1024); + strsize2 = ucs2_strsize(entry->var.VariableName, EFI_VAR_NAME_LEN); if (strsize1 == strsize2 && !memcmp(variable_name, &(entry->var.VariableName), strsize2) && @@ -396,6 +396,7 @@ int efivar_init(int (*func)(efi_char16_t *, efi_guid_t, unsigned long, void *, do { variable_name_size = 512; + BUILD_BUG_ON(EFI_VAR_NAME_LEN < 512); status = efivar_get_next_variable(&variable_name_size, variable_name, diff --git a/include/linux/efi.h b/include/linux/efi.h index d59b0947fba0..418e555459da 100644 --- a/include/linux/efi.h +++ b/include/linux/efi.h @@ -1072,12 +1072,11 @@ static inline u64 efivar_reserved_space(void) { return 0; } #endif /* - * The maximum size of VariableName + Data = 1024 - * Therefore, it's reasonable to save that much - * space in each part of the structure, - * and we use a page for reading/writing. + * There is no actual upper limit specified for the variable name size. + * + * This limit exists only for practical purposes, since name conversions + * are bounds-checked and name data is occasionally stored in-line. */ - #define EFI_VAR_NAME_LEN 1024 int efivars_register(struct efivars *efivars, From 4b2543f7e1e6b91cfc8dd1696e3cdf01c3ac8974 Mon Sep 17 00:00:00 2001 From: Hagar Hemdan Date: Tue, 23 Apr 2024 13:59:26 +0000 Subject: [PATCH 5/5] efi: libstub: only free priv.runtime_map when allocated priv.runtime_map is only allocated when efi_novamap is not set. Otherwise, it is an uninitialized value. In the error path, it is freed unconditionally. Avoid passing an uninitialized value to free_pool. Free priv.runtime_map only when it was allocated. This bug was discovered and resolved using Coverity Static Analysis Security Testing (SAST) by Synopsys, Inc. Fixes: f80d26043af9 ("efi: libstub: avoid efi_get_memory_map() for allocating the virt map") Cc: Signed-off-by: Hagar Hemdan Signed-off-by: Ard Biesheuvel --- drivers/firmware/efi/libstub/fdt.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/firmware/efi/libstub/fdt.c b/drivers/firmware/efi/libstub/fdt.c index 70e9789ff9de..6a337f1f8787 100644 --- a/drivers/firmware/efi/libstub/fdt.c +++ b/drivers/firmware/efi/libstub/fdt.c @@ -335,8 +335,8 @@ efi_status_t allocate_new_fdt_and_exit_boot(void *handle, fail: efi_free(fdt_size, fdt_addr); - - efi_bs_call(free_pool, priv.runtime_map); + if (!efi_novamap) + efi_bs_call(free_pool, priv.runtime_map); return EFI_LOAD_ERROR; }