RE: [PATCH V6 2/2] x86/efi: Add efi page fault handler to recover from page faults caused by the firmware

2018-09-12 Thread Prakhya, Sai Praneeth
> On 12 September 2018 at 20:56, Thomas Gleixner  wrote:
> > On Tue, 11 Sep 2018, Sai Praneeth Prakhya wrote:
> >> diff --git a/drivers/firmware/efi/runtime-wrappers.c
> b/drivers/firmware/efi/runtime-wrappers.c
> >> index b18b2d864c2c..7455277a3b65 100644
> >> --- a/drivers/firmware/efi/runtime-wrappers.c
> >> +++ b/drivers/firmware/efi/runtime-wrappers.c
> >> @@ -61,6 +61,11 @@ struct efi_runtime_work efi_rts_work;
> >>  ({   \
> >>   efi_rts_work.status = EFI_ABORTED;  \
> >>   \
> >> + if (!efi_enabled(EFI_RUNTIME_SERVICES)) {   \
> >> + pr_info("Aborting! EFI Runtime Services disabled\n");   \
> >
> > This probaby wants to be pr_info_ince().
> >

Yes.. that makes sense.

> 
> I can fix that up when applying.
>

Thanks a lot! Ard.
 
> > Other than that:
> >
> >   Reviewed-by: Thomas Gleixner 
> 
> Thanks! I will queue this up in efi/next


Re: [PATCH V6 2/2] x86/efi: Add efi page fault handler to recover from page faults caused by the firmware

2018-09-12 Thread Ard Biesheuvel
On 12 September 2018 at 20:56, Thomas Gleixner  wrote:
> On Tue, 11 Sep 2018, Sai Praneeth Prakhya wrote:
>> diff --git a/drivers/firmware/efi/runtime-wrappers.c 
>> b/drivers/firmware/efi/runtime-wrappers.c
>> index b18b2d864c2c..7455277a3b65 100644
>> --- a/drivers/firmware/efi/runtime-wrappers.c
>> +++ b/drivers/firmware/efi/runtime-wrappers.c
>> @@ -61,6 +61,11 @@ struct efi_runtime_work efi_rts_work;
>>  ({   \
>>   efi_rts_work.status = EFI_ABORTED;  \
>>   \
>> + if (!efi_enabled(EFI_RUNTIME_SERVICES)) {   \
>> + pr_info("Aborting! EFI Runtime Services disabled\n");   \
>
> This probaby wants to be pr_info_ince().
>

I can fix that up when applying.

> Other than that:
>
>   Reviewed-by: Thomas Gleixner 

Thanks! I will queue this up in efi/next


Re: [PATCH V6 2/2] x86/efi: Add efi page fault handler to recover from page faults caused by the firmware

2018-09-12 Thread Thomas Gleixner
On Tue, 11 Sep 2018, Sai Praneeth Prakhya wrote:
> diff --git a/drivers/firmware/efi/runtime-wrappers.c 
> b/drivers/firmware/efi/runtime-wrappers.c
> index b18b2d864c2c..7455277a3b65 100644
> --- a/drivers/firmware/efi/runtime-wrappers.c
> +++ b/drivers/firmware/efi/runtime-wrappers.c
> @@ -61,6 +61,11 @@ struct efi_runtime_work efi_rts_work;
>  ({   \
>   efi_rts_work.status = EFI_ABORTED;  \
>   \
> + if (!efi_enabled(EFI_RUNTIME_SERVICES)) {   \
> + pr_info("Aborting! EFI Runtime Services disabled\n");   \

This probaby wants to be pr_info_ince().

Other than that:

  Reviewed-by: Thomas Gleixner 


[PATCH V6 2/2] x86/efi: Add efi page fault handler to recover from page faults caused by the firmware

2018-09-11 Thread Sai Praneeth Prakhya
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 recovers from such faults by
1. If the efi runtime service is efi_reset_system(), reboot the machine
   through BIOS.
2. If the efi runtime service is _not_ efi_reset_system(), then, freeze
   efi_rts_wq and schedule a new process.

The efi 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.

Tested-by: Bhupesh Sharma 
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  |  1 +
 arch/x86/mm/fault.c |  9 
 arch/x86/platform/efi/quirks.c  | 78 +
 drivers/firmware/efi/runtime-wrappers.c |  8 
 include/linux/efi.h |  8 +++-
 5 files changed, 103 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h
index cec5fae23eb3..eea40d52ca78 100644
--- a/arch/x86/include/asm/efi.h
+++ b/arch/x86/include/asm/efi.h
@@ -140,6 +140,7 @@ extern void __init efi_apply_memmap_quirks(void);
 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);
+extern void efi_recover_from_page_fault(unsigned long phys_addr);
 
 struct efi_setup_data {
u64 fw_vendor;
diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c
index 2aafa6ab6103..fd636c82d3c1 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  /* efi_recover_from_page_fault()*/
 
 #include /* boot_cpu_has, ...*/
 #include  /* dotraplinkage, ...   */
@@ -24,6 +25,7 @@
 #include   /* emulate_vsyscall */
 #include   /* struct vm86  */
 #include/* vma_pkey()   */
+#include/* efi_recover_from_page_fault()*/
 
 #define CREATE_TRACE_POINTS
 #include 
@@ -790,6 +792,13 @@ no_context(struct pt_regs *regs, unsigned long error_code,
return;
 
/*
+* Buggy firmware could access regions which might page fault, try to
+* recover from such faults.
+*/
+   if (IS_ENABLED(CONFIG_EFI))
+   efi_recover_from_page_fault(address);
+
+   /*
 * 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 844d31cb8a0c..669babcaf245 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
 
@@ -654,3 +655,80 @@ int efi_capsule_setup_info(struct capsule_info *cap_info, 
void *kbuff,
 }
 
 #endif
+
+/*
+ * If any access by any efi runtime service causes a page fault, then,
+ * 1. If it's efi_reset_system(), reboot through BIOS.
+ * 2. If any other efi runtime service, then
+ *a. Return error status to the efi caller process.
+ *b. Disable EFI Runtime Services forever and
+ *c. Freeze efi_rts_wq and schedule new process.
+ *
+ * @return: Returns, if the page fault is not handled. This function
+ * will never return if the page fault is handled successfully.
+ */
+void efi_recover_from_page_fault(unsigned long phys_addr)
+{
+   if (!IS_ENABLED(CONFIG_X86_64))
+   return;
+
+   /*
+* Make sure that an efi runtime service caused the page fault.
+* "efi_mm" cannot be used to check if the page fault had occurred
+* in the firmware context because efi=old_map doesn't use efi_pgd.
+*/
+   if (efi_rts_work.efi_rts_id == NONE)
+   return;
+
+   /*
+* Address range 0x - 0x0fff is always mapped in the efi_pgd, so
+* page faulting on these addresses isn't expected.
+*/
+   if (phys_addr >=