[kernel] Fix UEFI anti-bricking code (rhbz 964335)
Josh Boyer
jwboyer at fedoraproject.org
Mon Jun 3 23:09:47 UTC 2013
commit f58391d7ed12eef1c6a3732cbca52dcd4644ee30
Author: Josh Boyer <jwboyer at redhat.com>
Date: Mon Jun 3 19:05:22 2013 -0400
Fix UEFI anti-bricking code (rhbz 964335)
Modify-UEFI-anti-bricking-code.patch | 371 ++++++++++++++++++++++++++++++++++
kernel.spec | 11 +-
2 files changed, 381 insertions(+), 1 deletions(-)
---
diff --git a/Modify-UEFI-anti-bricking-code.patch b/Modify-UEFI-anti-bricking-code.patch
new file mode 100644
index 0000000..8625745
--- /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 <matthew.garrett at nebula.com>
+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 <matthew.garrett at nebula.com>
+---
+ 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 <linux/io.h>
+ #include <linux/reboot.h>
+ #include <linux/bcd.h>
+-#include <linux/ucs2_string.h>
+
+ #include <asm/setup.h>
+ #include <asm/efi.h>
+@@ -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 38a3873..c6b8280 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 <jwboyer at redhat.com>
+- Fix UEFI anti-bricking code (rhbz 964335)
+
* Mon Jun 3 2013 Peter Robinson <pbrobinson at fedoraproject.org>
- Minor ARM config changes
More information about the scm-commits
mailing list