From f58391d7ed12eef1c6a3732cbca52dcd4644ee30 Mon Sep 17 00:00:00 2001 From: Josh Boyer Date: Mon, 3 Jun 2013 19:05:22 -0400 Subject: [PATCH] Fix UEFI anti-bricking code (rhbz 964335) --- Modify-UEFI-anti-bricking-code.patch | 371 +++++++++++++++++++++++++++ kernel.spec | 11 +- 2 files changed, 381 insertions(+), 1 deletion(-) create mode 100644 Modify-UEFI-anti-bricking-code.patch diff --git a/Modify-UEFI-anti-bricking-code.patch b/Modify-UEFI-anti-bricking-code.patch new file mode 100644 index 000000000..862574556 --- /dev/null +++ b/Modify-UEFI-anti-bricking-code.patch @@ -0,0 +1,371 @@ +From 2380baac8b96f6e93ef72135d1b60d686d7f82e6 Mon Sep 17 00:00:00 2001 +From: Matthew Garrett +Date: Sat, 1 Jun 2013 16:06:20 -0400 +Subject: [PATCH] Modify UEFI anti-bricking code + +This patch reworks the UEFI anti-bricking code, including an effective +reversion of cc5a080c and 31ff2f20. It turns out that calling +QueryVariableInfo() from boot services results in some firmware +implementations jumping to physical addresses even after entering virtual +mode, so until we have 1:1 mappings for UEFI runtime space this isn't +going to work so well. + +Reverting these gets us back to the situation where we'd refuse to create +variables on some systems because they classify deleted variables as "used" +until the firmware triggers a garbage collection run, which they won't do +until they reach a lower threshold. This results in it being impossible to +install a bootloader, which is unhelpful. + +Feedback from Samsung indicates that the firmware doesn't need more than +5KB of storage space for its own purposes, so that seems like a reasonable +threshold. However, there's still no guarantee that a platform will attempt +garbage collection merely because it drops below this threshold. It seems +that this is often only triggered if an attempt to write generates a +genuine EFI_OUT_OF_RESOURCES error. We can force that by attempting to +create a variable larger than the remaining space. This should fail, but if +it somehow succeeds we can then immediately delete it. + +I've tested this on the UEFI machines I have available, but I don't have +a Samsung and so can't verify that it avoids the bricking problem. + +Signed-off-by: Matthew Garrett +--- + arch/x86/boot/compressed/eboot.c | 47 ---------- + arch/x86/include/asm/efi.h | 7 -- + arch/x86/include/uapi/asm/bootparam.h | 1 - + arch/x86/platform/efi/efi.c | 167 +++++++++------------------------- + 4 files changed, 44 insertions(+), 178 deletions(-) + +diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c +index 35ee62f..c205035 100644 +--- a/arch/x86/boot/compressed/eboot.c ++++ b/arch/x86/boot/compressed/eboot.c +@@ -251,51 +251,6 @@ static void find_bits(unsigned long mask, u8 *pos, u8 *size) + *size = len; + } + +-static efi_status_t setup_efi_vars(struct boot_params *params) +-{ +- struct setup_data *data; +- struct efi_var_bootdata *efidata; +- u64 store_size, remaining_size, var_size; +- efi_status_t status; +- +- if (sys_table->runtime->hdr.revision < EFI_2_00_SYSTEM_TABLE_REVISION) +- return EFI_UNSUPPORTED; +- +- data = (struct setup_data *)(unsigned long)params->hdr.setup_data; +- +- while (data && data->next) +- data = (struct setup_data *)(unsigned long)data->next; +- +- status = efi_call_phys4((void *)sys_table->runtime->query_variable_info, +- EFI_VARIABLE_NON_VOLATILE | +- EFI_VARIABLE_BOOTSERVICE_ACCESS | +- EFI_VARIABLE_RUNTIME_ACCESS, &store_size, +- &remaining_size, &var_size); +- +- if (status != EFI_SUCCESS) +- return status; +- +- status = efi_call_phys3(sys_table->boottime->allocate_pool, +- EFI_LOADER_DATA, sizeof(*efidata), &efidata); +- +- if (status != EFI_SUCCESS) +- return status; +- +- efidata->data.type = SETUP_EFI_VARS; +- efidata->data.len = sizeof(struct efi_var_bootdata) - +- sizeof(struct setup_data); +- efidata->data.next = 0; +- efidata->store_size = store_size; +- efidata->remaining_size = remaining_size; +- efidata->max_var_size = var_size; +- +- if (data) +- data->next = (unsigned long)efidata; +- else +- params->hdr.setup_data = (unsigned long)efidata; +- +-} +- + static efi_status_t setup_efi_pci(struct boot_params *params) + { + efi_pci_io_protocol *pci; +@@ -1202,8 +1157,6 @@ struct boot_params *efi_main(void *handle, efi_system_table_t *_table, + + setup_graphics(boot_params); + +- setup_efi_vars(boot_params); +- + setup_efi_pci(boot_params); + + status = efi_call_phys3(sys_table->boottime->allocate_pool, +diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h +index 2fb5d58..60c89f3 100644 +--- a/arch/x86/include/asm/efi.h ++++ b/arch/x86/include/asm/efi.h +@@ -102,13 +102,6 @@ extern void efi_call_phys_epilog(void); + extern void efi_unmap_memmap(void); + extern void efi_memory_uc(u64 addr, unsigned long size); + +-struct efi_var_bootdata { +- struct setup_data data; +- u64 store_size; +- u64 remaining_size; +- u64 max_var_size; +-}; +- + #ifdef CONFIG_EFI + + static inline bool efi_is_native(void) +diff --git a/arch/x86/include/uapi/asm/bootparam.h b/arch/x86/include/uapi/asm/bootparam.h +index 0874424..c15ddaf 100644 +--- a/arch/x86/include/uapi/asm/bootparam.h ++++ b/arch/x86/include/uapi/asm/bootparam.h +@@ -6,7 +6,6 @@ + #define SETUP_E820_EXT 1 + #define SETUP_DTB 2 + #define SETUP_PCI 3 +-#define SETUP_EFI_VARS 4 + + /* ram_size flags */ + #define RAMDISK_IMAGE_START_MASK 0x07FF +diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c +index e4a86a6..beb5d5f 100644 +--- a/arch/x86/platform/efi/efi.c ++++ b/arch/x86/platform/efi/efi.c +@@ -41,7 +41,6 @@ + #include + #include + #include +-#include + + #include + #include +@@ -52,13 +51,6 @@ + + #define EFI_DEBUG 1 + +-/* +- * There's some additional metadata associated with each +- * variable. Intel's reference implementation is 60 bytes - bump that +- * to account for potential alignment constraints +- */ +-#define VAR_METADATA_SIZE 64 +- + struct efi __read_mostly efi = { + .mps = EFI_INVALID_TABLE_ADDR, + .acpi = EFI_INVALID_TABLE_ADDR, +@@ -77,13 +69,6 @@ struct efi_memory_map memmap; + static struct efi efi_phys __initdata; + static efi_system_table_t efi_systab __initdata; + +-static u64 efi_var_store_size; +-static u64 efi_var_remaining_size; +-static u64 efi_var_max_var_size; +-static u64 boot_used_size; +-static u64 boot_var_size; +-static u64 active_size; +- + unsigned long x86_efi_facility; + + /* +@@ -186,53 +171,8 @@ static efi_status_t virt_efi_get_next_variable(unsigned long *name_size, + efi_char16_t *name, + efi_guid_t *vendor) + { +- efi_status_t status; +- static bool finished = false; +- static u64 var_size; +- +- status = efi_call_virt3(get_next_variable, ++ return efi_call_virt3(get_next_variable, + name_size, name, vendor); +- +- if (status == EFI_NOT_FOUND) { +- finished = true; +- if (var_size < boot_used_size) { +- boot_var_size = boot_used_size - var_size; +- active_size += boot_var_size; +- } else { +- printk(KERN_WARNING FW_BUG "efi: Inconsistent initial sizes\n"); +- } +- } +- +- if (boot_used_size && !finished) { +- unsigned long size; +- u32 attr; +- efi_status_t s; +- void *tmp; +- +- s = virt_efi_get_variable(name, vendor, &attr, &size, NULL); +- +- if (s != EFI_BUFFER_TOO_SMALL || !size) +- return status; +- +- tmp = kmalloc(size, GFP_ATOMIC); +- +- if (!tmp) +- return status; +- +- s = virt_efi_get_variable(name, vendor, &attr, &size, tmp); +- +- if (s == EFI_SUCCESS && (attr & EFI_VARIABLE_NON_VOLATILE)) { +- var_size += size; +- var_size += ucs2_strsize(name, 1024); +- active_size += size; +- active_size += VAR_METADATA_SIZE; +- active_size += ucs2_strsize(name, 1024); +- } +- +- kfree(tmp); +- } +- +- return status; + } + + static efi_status_t virt_efi_set_variable(efi_char16_t *name, +@@ -241,34 +181,9 @@ static efi_status_t virt_efi_set_variable(efi_char16_t *name, + unsigned long data_size, + void *data) + { +- efi_status_t status; +- u32 orig_attr = 0; +- unsigned long orig_size = 0; +- +- status = virt_efi_get_variable(name, vendor, &orig_attr, &orig_size, +- NULL); +- +- if (status != EFI_BUFFER_TOO_SMALL) +- orig_size = 0; +- +- status = efi_call_virt5(set_variable, +- name, vendor, attr, +- data_size, data); +- +- if (status == EFI_SUCCESS) { +- if (orig_size) { +- active_size -= orig_size; +- active_size -= ucs2_strsize(name, 1024); +- active_size -= VAR_METADATA_SIZE; +- } +- if (data_size) { +- active_size += data_size; +- active_size += ucs2_strsize(name, 1024); +- active_size += VAR_METADATA_SIZE; +- } +- } +- +- return status; ++ return efi_call_virt5(set_variable, ++ name, vendor, attr, ++ data_size, data); + } + + static efi_status_t virt_efi_query_variable_info(u32 attr, +@@ -776,9 +691,6 @@ void __init efi_init(void) + char vendor[100] = "unknown"; + int i = 0; + void *tmp; +- struct setup_data *data; +- struct efi_var_bootdata *efi_var_data; +- u64 pa_data; + + #ifdef CONFIG_X86_32 + if (boot_params.efi_info.efi_systab_hi || +@@ -796,22 +708,6 @@ void __init efi_init(void) + if (efi_systab_init(efi_phys.systab)) + return; + +- pa_data = boot_params.hdr.setup_data; +- while (pa_data) { +- data = early_ioremap(pa_data, sizeof(*efi_var_data)); +- if (data->type == SETUP_EFI_VARS) { +- efi_var_data = (struct efi_var_bootdata *)data; +- +- efi_var_store_size = efi_var_data->store_size; +- efi_var_remaining_size = efi_var_data->remaining_size; +- efi_var_max_var_size = efi_var_data->max_var_size; +- } +- pa_data = data->next; +- early_iounmap(data, sizeof(*efi_var_data)); +- } +- +- boot_used_size = efi_var_store_size - efi_var_remaining_size; +- + set_bit(EFI_SYSTEM_TABLES, &x86_efi_facility); + + /* +@@ -1131,28 +1027,53 @@ efi_status_t efi_query_variable_store(u32 attributes, unsigned long size) + if (status != EFI_SUCCESS) + return status; + +- if (!max_size && remaining_size > size) +- printk_once(KERN_ERR FW_BUG "Broken EFI implementation" +- " is returning MaxVariableSize=0\n"); + /* + * Some firmware implementations refuse to boot if there's insufficient + * space in the variable store. We account for that by refusing the + * write if permitting it would reduce the available space to under +- * 50%. However, some firmware won't reclaim variable space until +- * after the used (not merely the actively used) space drops below +- * a threshold. We can approximate that case with the value calculated +- * above. If both the firmware and our calculations indicate that the +- * available space would drop below 50%, refuse the write. ++ * 5KB. This figure was provided by Samsung, so should be safe. + */ ++ if ((remaining_size - size < 5120) && !efi_no_storage_paranoia) { ++ /* ++ * Triggering garbage collection may require that the firmware ++ * generate a real EFI_OUT_OF_RESOURCES error. We can force ++ * that by attempting to use more space than is available. ++ */ ++ unsigned long dummy_size = remaining_size + 1024; ++ void *dummy = kmalloc(dummy_size, GFP_ATOMIC); ++ efi_char16_t efi_name[6] = { 'D', 'U', 'M', 'M', 'Y', 0 }; ++ efi_guid_t guid = EFI_GUID(0x4424ac57, 0xbe4b, 0x47dd, 0x9e, ++ 0x97, 0xed, 0x50, 0xf0, 0x9f, 0x92, ++ 0xa9); ++ ++ status = efi.set_variable(efi_name, &guid, attributes, ++ dummy_size, dummy); ++ ++ if (status == EFI_SUCCESS) { ++ /* ++ * This should have failed, so if it didn't make sure ++ * that we delete it... ++ */ ++ efi.set_variable(efi_name, &guid, attributes, 0, ++ dummy); ++ } + +- if (!storage_size || size > remaining_size || +- (max_size && size > max_size)) +- return EFI_OUT_OF_RESOURCES; ++ /* ++ * The runtime code may now have triggered a garbage collection ++ * run, so check the variable info again ++ */ ++ status = efi.query_variable_info(attributes, &storage_size, ++ &remaining_size, &max_size); + +- if (!efi_no_storage_paranoia && +- ((active_size + size + VAR_METADATA_SIZE > storage_size / 2) && +- (remaining_size - size < storage_size / 2))) +- return EFI_OUT_OF_RESOURCES; ++ if (status != EFI_SUCCESS) ++ return status; ++ ++ /* ++ * There still isn't enough room, so return an error ++ */ ++ if (remaining_size - size < 5120) ++ return EFI_OUT_OF_RESOURCES; ++ } + + return EFI_SUCCESS; + } +-- +1.8.1.4 + diff --git a/kernel.spec b/kernel.spec index 38a38733d..c6b8280a1 100644 --- a/kernel.spec +++ b/kernel.spec @@ -62,7 +62,7 @@ Summary: The Linux kernel # For non-released -rc kernels, this will be appended after the rcX and # gitX tags, so a 3 here would become part of release "0.rcX.gitX.3" # -%global baserelease 1 +%global baserelease 2 %global fedora_build %{baserelease} # base_sublevel is the kernel version we're starting with and patching @@ -745,6 +745,9 @@ Patch25024: intel_iommu-Downgrade-the-warning-if-enabling-irq-remapping-fails.pa #CVE-2013-2850 rhbz 968036 969272 Patch25025: iscsi-target-fix-heap-buffer-overflow-on-error.patch +#rhbz 964335 +Patch25026: Modify-UEFI-anti-bricking-code.patch + # END OF PATCH DEFINITIONS %endif @@ -1433,6 +1436,9 @@ ApplyPatch intel_iommu-Downgrade-the-warning-if-enabling-irq-remapping-fails.pat #CVE-2013-2850 rhbz 968036 969272 ApplyPatch iscsi-target-fix-heap-buffer-overflow-on-error.patch +#rhbz 964335 +ApplyPatch Modify-UEFI-anti-bricking-code.patch + # END OF PATCH APPLICATIONS %endif @@ -2238,6 +2244,9 @@ fi # ||----w | # || || %changelog +* Mon Jun 03 2013 Josh Boyer +- Fix UEFI anti-bricking code (rhbz 964335) + * Mon Jun 3 2013 Peter Robinson - Minor ARM config changes