RE: [PATCH V3 2/5] efi: Introduce __efi_init attribute
Hi Boris and Ard, > Buggy firmware could illegally access some efi regions even after the kernel > has > assumed control of the platform. When > "CONFIG_EFI_WARN_ON_ILLEGAL_ACCESS" is enabled, the efi page fault > handler will detect and recover from these illegal accesses. > efi_md_typeattr_format() and memory_type_name are used by the efi page > fault handler to print information about memory descriptor that was illegally > accessed. As the page fault handler is present during/after kernel boot it > doesn't > have an __init attribute, but > efi_md_typeattr_format() has it and thus during kernel build, "WARNING: > modpost: Found * section mismatch(es)" build warning is observed. To fix it, > remove __init attribute for efi_md_typeattr_format(). > > In order to not keep efi_md_typeattr_format() and memory_type_name > needlessly when "CONFIG_EFI_WARN_ON_ILLEGAL_ACCESS" is not selected, > add a new __efi_init attribute whose value changes based on whether the config > option is selected or not. In previous versions (i.e. up to V2), where we handled EFI_BOOT_SERVICES_ regions differently, it made sense to have a separate attribute like __efi_init because many function definitions were modified. From V3, do you think it's still OK to have __efi_init or should I just remove __init attribute (and not have __efi_init) for efi_md_typeattr_format() and memory_type_name because we are just modifying two. Regards, Sai
[PATCH V3 2/5] efi: Introduce __efi_init attribute
From: Sai Praneeth
Buggy firmware could illegally access some efi regions even after the
kernel has assumed control of the platform. When
"CONFIG_EFI_WARN_ON_ILLEGAL_ACCESS" is enabled, the efi page fault
handler will detect and recover from these illegal accesses.
efi_md_typeattr_format() and memory_type_name are used by the efi page
fault handler to print information about memory descriptor that was
illegally accessed. As the page fault handler is present during/after
kernel boot it doesn't have an __init attribute, but
efi_md_typeattr_format() has it and thus during kernel build, "WARNING:
modpost: Found * section mismatch(es)" build warning is observed. To fix
it, remove __init attribute for efi_md_typeattr_format().
In order to not keep efi_md_typeattr_format() and memory_type_name
needlessly when "CONFIG_EFI_WARN_ON_ILLEGAL_ACCESS" is not selected, add
a new __efi_init attribute whose value changes based on whether the
config option is selected or not.
Suggested-by: Matt Fleming
Based-on-code-from: Ricardo Neri
Signed-off-by: Sai Praneeth Prakhya
Cc: Al Stone
Cc: Borislav Petkov
Cc: Ingo Molnar
Cc: Andy Lutomirski
Cc: Bhupesh Sharma
Cc: Thomas Gleixner
Cc: Peter Zijlstra
Cc: Ard Biesheuvel
---
drivers/firmware/efi/efi.c | 4 ++--
include/linux/efi.h| 14 +-
2 files changed, 15 insertions(+), 3 deletions(-)
diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index d8a33a781a57..16571429b19c 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -768,7 +768,7 @@ int __init efi_get_fdt_params(struct efi_fdt_params *params)
}
#endif /* CONFIG_EFI_PARAMS_FROM_FDT */
-static __initdata char memory_type_name[][20] = {
+static __efi_initdata char memory_type_name[][20] = {
"Reserved",
"Loader Code",
"Loader Data",
@@ -786,7 +786,7 @@ static __initdata char memory_type_name[][20] = {
"Persistent Memory",
};
-char * __init efi_md_typeattr_format(char *buf, size_t size,
+char * __efi_init efi_md_typeattr_format(char *buf, size_t size,
const efi_memory_desc_t *md)
{
char *pos;
diff --git a/include/linux/efi.h b/include/linux/efi.h
index 855992b15269..6a07e3166fd1 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -1107,10 +1107,22 @@ extern int efi_memattr_apply_permissions(struct
mm_struct *mm,
for_each_efi_memory_desc_in_map(, md)
/*
+ * __efi_init - if CONFIG_EFI_WARN_ON_ILLEGAL_ACCESS is enabled, remove __init
+ * modifier.
+ */
+#ifdef CONFIG_EFI_WARN_ON_ILLEGAL_ACCESS
+#define __efi_init
+#define __efi_initdata
+#else
+#define __efi_init __init
+#define __efi_initdata __initdata
+#endif
+
+/*
* Format an EFI memory descriptor's type and attributes to a user-provided
* character buffer, as per snprintf(), and return the buffer.
*/
-char * __init efi_md_typeattr_format(char *buf, size_t size,
+char * __efi_init efi_md_typeattr_format(char *buf, size_t size,
const efi_memory_desc_t *md);
/**
--
2.7.4
[PATCH V3 3/5] x86/efi: Permanently save the EFI_MEMORY_MAP passed by the firmware
From: Sai Praneeth
The efi page fault handler that recovers from page faults caused by the
firmware needs the original memory map passed by the firmware. It looks
up this memory map to find the type of the memory region at which the
page fault occurred. Presently, EFI subsystem discards the original
memory map passed by the firmware and replaces it with a new memory map
that has only EFI_RUNTIME_SERVICES_ regions. But illegal
accesses by firmware can occur at any region. Hence, _only_ if
CONFIG_EFI_WARN_ON_ILLEGAL_ACCESS is defined, create a backup of the
original memory map passed by the firmware, so that efi page fault
handler could detect/recover from illegal accesses to *any* efi region.
Suggested-by: Matt Fleming
Based-on-code-from: Ricardo Neri
Signed-off-by: Sai Praneeth Prakhya
Cc: Al Stone
Cc: Borislav Petkov
Cc: Ingo Molnar
Cc: Andy Lutomirski
Cc: Bhupesh Sharma
Cc: Thomas Gleixner
Cc: Peter Zijlstra
Cc: Ard Biesheuvel
---
arch/x86/include/asm/efi.h | 6 ++
arch/x86/platform/efi/efi.c| 2 ++
arch/x86/platform/efi/quirks.c | 48 ++
3 files changed, 56 insertions(+)
diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h
index cec5fae23eb3..788ed4cbce22 100644
--- a/arch/x86/include/asm/efi.h
+++ b/arch/x86/include/asm/efi.h
@@ -141,6 +141,12 @@ extern int __init efi_reuse_config(u64 tables, int
nr_tables);
extern void efi_delete_dummy_variable(void);
extern void efi_switch_mm(struct mm_struct *mm);
+#ifdef CONFIG_EFI_WARN_ON_ILLEGAL_ACCESS
+extern void __init efi_save_original_memmap(void);
+#else
+static inline void __init efi_save_original_memmap(void) { }
+#endif /* CONFIG_EFI_WARN_ON_ILLEGAL_ACCESS */
+
struct efi_setup_data {
u64 fw_vendor;
u64 runtime;
diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
index 9061babfbc83..7a3ea4cd5939 100644
--- a/arch/x86/platform/efi/efi.c
+++ b/arch/x86/platform/efi/efi.c
@@ -946,6 +946,8 @@ static void __init __efi_enter_virtual_mode(void)
pa = __pa(new_memmap);
+ efi_save_original_memmap();
+
/*
* Unregister the early EFI memmap from efi_init() and install
* the new EFI memory map that we are about to pass to the
diff --git a/arch/x86/platform/efi/quirks.c b/arch/x86/platform/efi/quirks.c
index 844d31cb8a0c..36b0b042ba56 100644
--- a/arch/x86/platform/efi/quirks.c
+++ b/arch/x86/platform/efi/quirks.c
@@ -654,3 +654,51 @@ int efi_capsule_setup_info(struct capsule_info *cap_info,
void *kbuff,
}
#endif
+
+#ifdef CONFIG_EFI_WARN_ON_ILLEGAL_ACCESS
+
+static bool original_memory_map_present;
+static struct efi_memory_map original_memory_map;
+
+/*
+ * The efi page fault handler that recovers from page faults caused by
+ * buggy firmware needs original memory map passed by firmware. Hence,
+ * build a new EFI memmap that has all entries and save it for later use.
+ */
+void __init efi_save_original_memmap(void)
+{
+ efi_memory_desc_t *md;
+ void *remapped_phys, *new_md;
+ phys_addr_t new_phys, new_size;
+
+ new_size = efi.memmap.desc_size * efi.memmap.nr_map;
+ new_phys = efi_memmap_alloc(efi.memmap.nr_map);
+ if (!new_phys) {
+ pr_err("Failed to allocate new EFI memmap\n");
+ return;
+ }
+
+ remapped_phys = memremap(new_phys, new_size, MEMREMAP_WB);
+ if (!remapped_phys) {
+ pr_err("Failed to remap new EFI memmap\n");
+ __free_pages(pfn_to_page(PHYS_PFN(new_phys)),
get_order(new_size));
+ return;
+ }
+
+ new_md = remapped_phys;
+ for_each_efi_memory_desc(md) {
+ memcpy(new_md, md, efi.memmap.desc_size);
+ new_md += efi.memmap.desc_size;
+ }
+
+ original_memory_map.late = 1;
+ original_memory_map.phys_map = new_phys;
+ original_memory_map.map = remapped_phys;
+ original_memory_map.nr_map = efi.memmap.nr_map;
+ original_memory_map.desc_size = efi.memmap.desc_size;
+ original_memory_map.map_end = remapped_phys + new_size;
+ original_memory_map.desc_version = efi.memmap.desc_version;
+
+ original_memory_map_present = true;
+}
+#endif /* CONFIG_EFI_WARN_ON_ILLEGAL_ACCESS */
--
2.7.4
[PATCH V3 1/5] efi: Make efi_rts_work accessible to efi page fault handler
From: Sai Praneeth
After the kernel has booted, if the firmware accesses *any* efi regions
other than EFI_RUNTIME_SERVICES_, the efi page fault handler
would freeze efi_rts_wq and schedules a new process. To do this, the efi
page fault handler needs efi_rts_work. Hence, make it accessible.
There will be no race conditions in accessing this structure, because,
all the calls to efi runtime services are already serialized.
Suggested-by: Matt Fleming
Based-on-code-from: Ricardo Neri
Signed-off-by: Sai Praneeth Prakhya
Cc: Al Stone
Cc: Borislav Petkov
Cc: Ingo Molnar
Cc: Andy Lutomirski
Cc: Bhupesh Sharma
Cc: Thomas Gleixner
Cc: Peter Zijlstra
Cc: Ard Biesheuvel
---
drivers/firmware/efi/runtime-wrappers.c | 53 ++---
include/linux/efi.h | 36 ++
2 files changed, 45 insertions(+), 44 deletions(-)
diff --git a/drivers/firmware/efi/runtime-wrappers.c
b/drivers/firmware/efi/runtime-wrappers.c
index aa66cbf23512..b18b2d864c2c 100644
--- a/drivers/firmware/efi/runtime-wrappers.c
+++ b/drivers/firmware/efi/runtime-wrappers.c
@@ -45,39 +45,7 @@
#define __efi_call_virt(f, args...) \
__efi_call_virt_pointer(efi.systab->runtime, f, args)
-/* efi_runtime_service() function identifiers */
-enum efi_rts_ids {
- GET_TIME,
- SET_TIME,
- GET_WAKEUP_TIME,
- SET_WAKEUP_TIME,
- GET_VARIABLE,
- GET_NEXT_VARIABLE,
- SET_VARIABLE,
- QUERY_VARIABLE_INFO,
- GET_NEXT_HIGH_MONO_COUNT,
- UPDATE_CAPSULE,
- QUERY_CAPSULE_CAPS,
-};
-
-/*
- * efi_runtime_work: Details of EFI Runtime Service work
- * @arg<1-5>: EFI Runtime Service function arguments
- * @status:Status of executing EFI Runtime Service
- * @efi_rts_id:EFI Runtime Service function identifier
- * @efi_rts_comp: Struct used for handling completions
- */
-struct efi_runtime_work {
- void *arg1;
- void *arg2;
- void *arg3;
- void *arg4;
- void *arg5;
- efi_status_t status;
- struct work_struct work;
- enum efi_rts_ids efi_rts_id;
- struct completion efi_rts_comp;
-};
+struct efi_runtime_work efi_rts_work;
/*
* efi_queue_work: Queue efi_runtime_service() and wait until it's done
@@ -91,7 +59,6 @@ struct efi_runtime_work {
*/
#define efi_queue_work(_rts, _arg1, _arg2, _arg3, _arg4, _arg5)
\
({ \
- struct efi_runtime_work efi_rts_work; \
efi_rts_work.status = EFI_ABORTED; \
\
init_completion(_rts_work.efi_rts_comp);\
@@ -184,18 +151,16 @@ static DEFINE_SEMAPHORE(efi_runtime_lock);
*/
static void efi_call_rts(struct work_struct *work)
{
- struct efi_runtime_work *efi_rts_work;
void *arg1, *arg2, *arg3, *arg4, *arg5;
efi_status_t status = EFI_NOT_FOUND;
- efi_rts_work = container_of(work, struct efi_runtime_work, work);
- arg1 = efi_rts_work->arg1;
- arg2 = efi_rts_work->arg2;
- arg3 = efi_rts_work->arg3;
- arg4 = efi_rts_work->arg4;
- arg5 = efi_rts_work->arg5;
+ arg1 = efi_rts_work.arg1;
+ arg2 = efi_rts_work.arg2;
+ arg3 = efi_rts_work.arg3;
+ arg4 = efi_rts_work.arg4;
+ arg5 = efi_rts_work.arg5;
- switch (efi_rts_work->efi_rts_id) {
+ switch (efi_rts_work.efi_rts_id) {
case GET_TIME:
status = efi_call_virt(get_time, (efi_time_t *)arg1,
(efi_time_cap_t *)arg2);
@@ -253,8 +218,8 @@ static void efi_call_rts(struct work_struct *work)
*/
pr_err("Requested executing invalid EFI Runtime Service.\n");
}
- efi_rts_work->status = status;
- complete(_rts_work->efi_rts_comp);
+ efi_rts_work.status = status;
+ complete(_rts_work.efi_rts_comp);
}
static efi_status_t virt_efi_get_time(efi_time_t *tm, efi_time_cap_t *tc)
diff --git a/include/linux/efi.h b/include/linux/efi.h
index 401e4b254e30..855992b15269 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -1659,7 +1659,43 @@ struct linux_efi_tpm_eventlog {
extern int efi_tpm_eventlog_init(void);
+/* efi_runtime_service() function identifiers */
+enum efi_rts_ids {
+ GET_TIME,
+ SET_TIME,
+ GET_WAKEUP_TIME,
+ SET_WAKEUP_TIME,
+ GET_VARIABLE,
+ GET_NEXT_VARIABLE,
+ SET_VARIABLE,
+ QUERY_VARIABLE_INFO,
+ GET_NEXT_HIGH_MONO_COUNT,
+ UPDATE_CAPSULE,
+ QUERY_CAPSULE_CAPS,
+};
+
+/*
+ * efi_runtime_work: Details of EFI Runtime Service work
+ * @arg<1-5>: EFI Runtime Service function arguments
+ * @status:Status of executing EFI Runtime Service
+ * @efi_rts_id:EFI Runtime Service
[PATCH V3 4/5] x86/efi: Add efi page fault handler to recover from the page faults caused by firmware
From: Sai Praneeth
As per the UEFI specification, after the call to ExitBootServices(),
accesses by the firmware to any memory regions except
EFI_RUNTIME_SERVICES_ regions is considered illegal. A buggy
firmware could trigger these illegal accesses when an efi runtime
service is invoked and if this happens when the kernel is up and
running, the kernel hangs.
Kernel hangs because the memory region requested by the firmware isn't
mapped in efi_pgd, which causes a page fault in ring 0 and the kernel
fails to handle it, leading to die(). To save kernel from hanging, add
an efi specific page fault handler which detects illegal accesses by the
firmware and if the access is to any region other than
EFI_RUNTIME_SERVICES_, then
1. The efi page fault handler freezes efi_rts_wq and schedules a new
process.
2. If the efi runtime service is efi_reset_system(), then the efi page
fault handler will reboot the machine through BIOS and not through
efi_reset_system().
The efi specific page fault handler offers us two advantages:
1. Recovers from potential hangs that could be caused by buggy firmware.
2. Shout loud that the firmware is buggy and hence is not a kernel bug.
Suggested-by: Matt Fleming
Based-on-code-from: Ricardo Neri
Signed-off-by: Sai Praneeth Prakhya
Cc: Al Stone
Cc: Borislav Petkov
Cc: Ingo Molnar
Cc: Andy Lutomirski
Cc: Bhupesh Sharma
Cc: Thomas Gleixner
Cc: Peter Zijlstra
Cc: Ard Biesheuvel
---
arch/x86/include/asm/efi.h | 5 ++
arch/x86/mm/fault.c | 9 ++
arch/x86/platform/efi/quirks.c | 140
drivers/firmware/efi/runtime-wrappers.c | 7 ++
include/linux/efi.h | 1 +
5 files changed, 162 insertions(+)
diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h
index 788ed4cbce22..f3d9c3c2359e 100644
--- a/arch/x86/include/asm/efi.h
+++ b/arch/x86/include/asm/efi.h
@@ -143,8 +143,13 @@ extern void efi_switch_mm(struct mm_struct *mm);
#ifdef CONFIG_EFI_WARN_ON_ILLEGAL_ACCESS
extern void __init efi_save_original_memmap(void);
+extern int efi_illegal_accesses_fixup(unsigned long phys_addr);
#else
static inline void __init efi_save_original_memmap(void) { }
+static inline int efi_illegal_accesses_fixup(unsigned long phys_addr)
+{
+ return 0;
+}
#endif /* CONFIG_EFI_WARN_ON_ILLEGAL_ACCESS */
struct efi_setup_data {
diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c
index 2aafa6ab6103..4f6939d8e13f 100644
--- a/arch/x86/mm/fault.c
+++ b/arch/x86/mm/fault.c
@@ -16,6 +16,7 @@
#include /* prefetchw*/
#include /* exception_enter(), ... */
#include /* faulthandler_disabled() */
+#include /* fixup for buggy UEFI firmware*/
#include /* boot_cpu_has, ...*/
#include /* dotraplinkage, ... */
@@ -24,6 +25,7 @@
#include /* emulate_vsyscall */
#include /* struct vm86 */
#include/* vma_pkey() */
+#include/* fixup for buggy UEFI firmware*/
#define CREATE_TRACE_POINTS
#include
@@ -790,6 +792,13 @@ no_context(struct pt_regs *regs, unsigned long error_code,
return;
/*
+* Buggy firmware could trigger illegal accesses to some EFI regions
+* which might page fault, try to recover from such faults.
+*/
+ if (efi_illegal_accesses_fixup(address))
+ return;
+
+ /*
* Oops. The kernel tried to access some bad page. We'll have to
* terminate things with extreme prejudice:
*/
diff --git a/arch/x86/platform/efi/quirks.c b/arch/x86/platform/efi/quirks.c
index 36b0b042ba56..2aba28a90800 100644
--- a/arch/x86/platform/efi/quirks.c
+++ b/arch/x86/platform/efi/quirks.c
@@ -16,6 +16,7 @@
#include
#include
#include
+#include
#define EFI_MIN_RESERVE 5120
@@ -701,4 +702,143 @@ void __init efi_save_original_memmap(void)
original_memory_map_present = true;
}
+
+/*
+ * From the original EFI memory map passed by the firmware, return a
+ * pointer to the memory descriptor that describes the given physical
+ * address. If not found, return NULL.
+ */
+static efi_memory_desc_t *efi_get_md(unsigned long phys_addr)
+{
+ efi_memory_desc_t *md;
+
+ for_each_efi_memory_desc_in_map(_memory_map, md) {
+ if (md->phys_addr <= phys_addr &&
+ (phys_addr < (md->phys_addr +
+ (md->num_pages << EFI_PAGE_SHIFT {
+ return md;
+ }
+ }
+ return NULL;
+}
+
+/*
+ * Detect illegal access by the firmware and if the illegally accessed
+ * region is any region described by efi memory map and other than
+ * EFI_RUNTIME_SERVICES_, then
+ * 1. If the efi runtime service is efi_reset_system(), then reboot
+ *through BIOS.
+ * 2. If the efi
[PATCH V3 0/5] Add efi page fault handler to detect and recover
From: Sai Praneeth There may exist some buggy UEFI firmware implementations that access efi memory regions other than EFI_RUNTIME_SERVICES_ even after the kernel has assumed control of the platform. This violates UEFI specification. Hence, provide a debug config option which when enabled detects and recovers from page faults caused by buggy firmware. The above said illegal accesses trigger page fault in ring 0 because firmware executes at ring 0 and if unhandled it hangs the kernel. Provide an efi specific page fault handler to: 1. Avoid panics/hangs caused by buggy firmware. 2. Shout loud that the firmware is buggy and hence is not a kernel bug. Upon detetcing that the illegally accessed region is any region other than EFI_RUNTIME_SERVICES_, the efi page fault handler will check if the access is by efi_reset_system(). 1. If so, then the efi page fault handler will reboot the machine through BIOS and not through efi_reset_system(). 2. If not, then the efi page fault handler will freeze efi_rts_wq and schedules a new process. This issue was reported by Al Stone when he saw that reboot via EFI hangs the machine. Upon debugging, I found that it's efi_reset_system() that's touching memory regions which it shouldn't. To reproduce the same behavior, I have hacked OVMF and made efi_reset_system() buggy. Along with efi_reset_system(), I have also modified get_next_high_mono_count() and set_virtual_address_map(). They illegally access both boot time and other efi regions. Testing the patch set: -- 1. Download buggy firmware from here [1]. 2. Run a qemu instance with this buggy BIOS and boot mainline kernel. Add reboot=efi to the kernel command line arguments and after the kernel is up and running, type "reboot". The kernel should hang while rebooting. 3. With the same setup, boot kernel after applying patches and the reboot should work fine. Also please notice warning/error messages printed by kernel. Changes from RFC to V1: --- 1. Drop "long jump" technique of dealing with illegal access and instead use scheduling away from efi_rts_wq. Changes from V1 to V2: -- 1. Shortened config name to CONFIG_EFI_WARN_ON_ILLEGAL_ACCESS from CONFIG_EFI_WARN_ON_ILLEGAL_ACCESSES. 2. Made the config option available only to expert users. 3. efi_free_boot_services() should be called only when CONFIG_EFI_WARN_ON_ILLEGAL_ACCESS is not enabled. Previously, this was part of init/main.c file. As it is an architecture agnostic code, moved the change to arch/x86/platform/efi/quirks.c file. Changes from V2 to V3: -- 1. Drop treating illegal access to EFI_BOOT_SERVICES_ regions separatley from illegal accesses to other regions like EFI_CONVENTIONAL_MEMORY or EFI_LOADER_. In previous versions, illegal access to EFI_BOOT_SERVICES_ regions were handled by mapping requested region to efi_pgd but from V3 they are handled similar to illegal access to other regions i.e by freezing efi_rts_wq and scheduling new process. 2. Change __efi_init_fixup attribute to __efi_init. Note: - Patch set based on "next" branch in efi tree. [1] https://drive.google.com/drive/folders/1VozKTms92ifyVHAT0ZDQe55ZYL1UE5wt Sai Praneeth (5): efi: Make efi_rts_work accessible to efi page fault handler efi: Introduce __efi_init attribute x86/efi: Permanently save the EFI_MEMORY_MAP passed by the firmware x86/efi: Add efi page fault handler to recover from the page faults caused by firmware x86/efi: Introduce EFI_WARN_ON_ILLEGAL_ACCESS arch/x86/Kconfig| 17 +++ arch/x86/include/asm/efi.h | 11 ++ arch/x86/mm/fault.c | 9 ++ arch/x86/platform/efi/efi.c | 2 + arch/x86/platform/efi/quirks.c | 188 drivers/firmware/efi/efi.c | 4 +- drivers/firmware/efi/runtime-wrappers.c | 60 +++--- include/linux/efi.h | 51 - 8 files changed, 295 insertions(+), 47 deletions(-) Suggested-by: Matt Fleming Based-on-code-from: Ricardo Neri Signed-off-by: Sai Praneeth Prakhya Cc: Al Stone Cc: Borislav Petkov Cc: Ingo Molnar Cc: Andy Lutomirski Cc: Bhupesh Sharma Cc: Thomas Gleixner Cc: Peter Zijlstra Cc: Ard Biesheuvel -- 2.7.4
[PATCH V3 5/5] x86/efi: Introduce EFI_WARN_ON_ILLEGAL_ACCESS
From: Sai Praneeth There may exist some buggy UEFI firmware implementations that might access efi regions other than EFI_RUNTIME_SERVICES_ even after the kernel has assumed control of the platform. This violates UEFI specification. If selected, this debug option will print a warning message if the UEFI firmware tries to access any memory region which it shouldn't. Along with the warning, the efi page fault handler will also try to recover from the page fault triggered by the firmware so that the machine doesn't hang. Suggested-by: Matt Fleming Based-on-code-from: Ricardo Neri Signed-off-by: Sai Praneeth Prakhya Cc: Al Stone Cc: Borislav Petkov Cc: Ingo Molnar Cc: Andy Lutomirski Cc: Bhupesh Sharma Cc: Thomas Gleixner Cc: Peter Zijlstra Cc: Ard Biesheuvel --- arch/x86/Kconfig | 17 + 1 file changed, 17 insertions(+) diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index f1dbb4ee19d7..7dc270c17d0b 100644 --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -1957,6 +1957,23 @@ config EFI_MIXED If unsure, say N. +config EFI_WARN_ON_ILLEGAL_ACCESS + bool "Warn about illegal memory accesses by firmware" if EXPERT + depends on EFI + help + Enable this debug feature so that the kernel can detect illegal + memory accesses by firmware and issue a warning. Also, + 1. If the illegally accessed region is any region other than +EFI_RUNTIME_SERVICES_, then the kernel freezes +efi_rts_wq and schedules a new process. Also, it disables EFI +Runtime Services, so that it will never again call buggy firmware. + 2. If the illegal access is by efi_reset_system(), then the +platform is rebooted through BIOS. + Please see the UEFI specification for details on the expectations + of memory usage. + + If unsure, say N. + config SECCOMP def_bool y prompt "Enable seccomp to safely compute untrusted bytecode" -- 2.7.4
Re: [PATCH v1 2/4] x86/efi: Get rid of custom ICPU() macro
Hi Andy,
I love your patch! Yet something to improve:
[auto build test ERROR on tip/auto-latest]
[also build test ERROR on v4.19-rc2 next-20180831]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Andy-Shevchenko/x86-cpu-Introduce-INTEL_CPU_FAM-_NODATA-helper-macros/20180903-170803
config: i386-randconfig-x078-201835 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
:: branch date: 56 minutes ago
:: commit date: 56 minutes ago
All errors (new ones prefixed by >>):
>> arch/x86/platform/efi/quirks.c:614:2: error: implicit declaration of
>> function 'INTEL_CPU_FAM_ANY' [-Werror=implicit-function-declaration]
INTEL_CPU_FAM_ANY(5, INTEL_FAM5_QUARK_X1000, _capsule_setup_info),
^
>> arch/x86/platform/efi/quirks.c:614:23: error: 'INTEL_FAM5_QUARK_X1000'
>> undeclared here (not in a function)
INTEL_CPU_FAM_ANY(5, INTEL_FAM5_QUARK_X1000, _capsule_setup_info),
^~
cc1: some warnings being treated as errors
#
https://github.com/0day-ci/linux/commit/e4c07e90e85507005bc79839ce9c0dbc8f016578
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout e4c07e90e85507005bc79839ce9c0dbc8f016578
vim +/INTEL_CPU_FAM_ANY +614 arch/x86/platform/efi/quirks.c
2959c95d Jan Kiszka 2017-06-02 612
2959c95d Jan Kiszka 2017-06-02 613 static const struct x86_cpu_id
efi_capsule_quirk_ids[] = {
e4c07e90 Andy Shevchenko 2018-08-31 @614INTEL_CPU_FAM_ANY(5,
INTEL_FAM5_QUARK_X1000, _capsule_setup_info),
2959c95d Jan Kiszka 2017-06-02 615{ }
2959c95d Jan Kiszka 2017-06-02 616 };
2959c95d Jan Kiszka 2017-06-02 617
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
.config.gz
Description: application/gzip
