x86: Add vDSO exception fixup for SGX

arch/x86/mm/fault.c __bad_area_nosemaphore(struct pt_regs *regs, unsigned long error_code, · line 936

Quoted 11 lines

#define CREATE_TRACE_POINTS
#include <asm/trace/exceptions.h>

arch/x86/mm/fault.c +3 -0

@@ -928,6 +929,9 @@ __bad_area_nosemaphore(struct pt_regs *regs, unsigned long error_code,
 		if (address >= TASK_SIZE_MAX)
 			error_code |= X86_PF_PROT;
 
+		if (fixup_vdso_exception(regs, X86_TRAP_PF, error_code, address))
+			return;
+
 		if (likely(show_unhandled_signals))
 			show_signal_msg(regs, error_code, address, tsk);

I'd preferably like to get this plugged into the page fault code before we get to the "bad_area" handling. This plugs it in near the erratum handling which seems really late to me.

Quoted 9 lines

arch/x86/mm/fault.c +3 -0

@@ -1045,6 +1049,9 @@ do_sigbus(struct pt_regs *regs, unsigned long error_code, unsigned long address,
 	if (is_prefetch(regs, error_code, address))
 		return;
 
+	if (fixup_vdso_exception(regs, X86_TRAP_PF, error_code, address))
+		return;
+
 	set_signal_archinfo(address, error_code);
 

arch/x86/mm/fault.c do_sigbus(struct pt_regs *regs, unsigned long error_code, unsigned long address, · line 1057

Quoted 1 line

arch/x86/mm/fault.c +0 -0

 #ifdef CONFIG_MEMORY_FAILURE

This *seems* really late to me. We've already called into the mm fault handling code to try and handle the fault and they told us it was VM_FAULT_SIGBUS. Shouldn't we have just detected that it was in the vDSO first and not even called the handling code?

arch/x86/mm/fault.c __bad_area_nosemaphore(struct pt_regs *regs, unsigned long error_code, · line 936

Quoted 12 lines

#define CREATE_TRACE_POINTS
#include <asm/trace/exceptions.h>

arch/x86/mm/fault.c +3 -0

@@ -928,6 +929,9 @@ __bad_area_nosemaphore(struct pt_regs *regs, unsigned long error_code,
              if (address >= TASK_SIZE_MAX)
                      error_code |= X86_PF_PROT;
 
+             if (fixup_vdso_exception(regs, X86_TRAP_PF, error_code, address))
+                     return;
+
              if (likely(show_unhandled_signals))
                      show_signal_msg(regs, error_code, address, tsk);

I'd preferably like to get this plugged into the page fault code before we get to the "bad_area" handling. This plugs it in near the erratum handling which seems really late to me.

Quoted 9 lines

arch/x86/mm/fault.c +3 -0

@@ -1045,6 +1049,9 @@ do_sigbus(struct pt_regs *regs, unsigned long error_code, unsigned long address,
      if (is_prefetch(regs, error_code, address))
              return;
 
+     if (fixup_vdso_exception(regs, X86_TRAP_PF, error_code, address))
+             return;
+
      set_signal_archinfo(address, error_code);
 

arch/x86/mm/fault.c do_sigbus(struct pt_regs *regs, unsigned long error_code, unsigned long address, · line 1057

Quoted 1 line

arch/x86/mm/fault.c +0 -0

 #ifdef CONFIG_MEMORY_FAILURE

This *seems* really late to me. We've already called into the mm fault handling code to try and handle the fault and they told us it was VM_FAULT_SIGBUS. Shouldn't we have just detected that it was in the vDSO first and not even called the handling code?

arch/x86/kernel/traps.c +7 -0

@@ -223,6 +224,10 @@ do_trap_no_signal(struct task_struct *tsk, int trapnr, const char *str,
 	tsk->thread.error_code = error_code;
 	tsk->thread.trap_nr = trapnr;
 
+	if (user_mode(regs) &&
+	    fixup_vdso_exception(regs, trapnr, error_code, 0))
+		return 0;
+
 	return -1;
 }
 

@@ -563,6 +568,9 @@ do_general_protection(struct pt_regs *regs, long error_code)
 	tsk->thread.error_code = error_code;
 	tsk->thread.trap_nr = X86_TRAP_GP;
 
+	if (fixup_vdso_exception(regs, X86_TRAP_GP, error_code, 0))
+		return;
+
 	show_signal(tsk, SIGSEGV, "", desc, regs, error_code);
 
 	force_sig(SIGSEGV, tsk);

Quoted 22 lines · Sean Christopherson

arch/x86/kernel/traps.c +7 -0

@@ -223,6 +224,10 @@ do_trap_no_signal(struct task_struct *tsk, int trapnr, const char *str,
      tsk->thread.error_code = error_code;
      tsk->thread.trap_nr = trapnr;
 
+     if (user_mode(regs) &&
+         fixup_vdso_exception(regs, trapnr, error_code, 0))
+             return 0;
+
      return -1;
 }
 

@@ -563,6 +568,9 @@ do_general_protection(struct pt_regs *regs, long error_code)
      tsk->thread.error_code = error_code;
      tsk->thread.trap_nr = X86_TRAP_GP;
 
+     if (fixup_vdso_exception(regs, X86_TRAP_GP, error_code, 0))
+             return;
+
      show_signal(tsk, SIGSEGV, "", desc, regs, error_code);
 
      force_sig(SIGSEGV, tsk);

arch/x86/kernel/traps.c static void math_error(struct pt_regs *regs, int error_code, int trapnr)

Quoted 10 lines

arch/x86/kernel/traps.c +3 -1

@@ -854,6 +862,9 @@ static void math_error(struct pt_regs *regs, int error_code, int trapnr)
      if (!si_code)
              return;
 
+     if (fixup_vdso_exception(regs, trapnr, error_code, 0))
+             return;
+
      force_sig_fault(SIGFPE, si_code,
                      (void __user *)uprobe_get_trap_addr(regs), task);
 }
--

Needs commenting, please.

But, also, this seems really ad-hoc. Probably, that's a result of our signal generation being really ad-hoc itself. But, if this claims "Attempt to fixup exceptions in vDSO before signaling", how do we assure ourselves that we hit all the ad-hoc signal generation cases? How do we know we didn't miss one or ten?

I want to hear more of the story of how you picked these sites and also decided that this is a comprehensive-enough set of sites to patch.

Intel Software Guard Extensions (SGX) SGX introduces a new CPL3-only enclave mode that runs as a sort of black box shared object that is hosted by an untrusted normal CPL3 process.

Enclave transitions have semantics that are a lovely blend of SYCSALL, SYSRET and VM-Exit. In a non-faulting scenario, entering and exiting an enclave can only be done through SGX-specific instructions, EENTER and EEXIT respectively. EENTER+EEXIT is analogous to SYSCALL+SYSRET, e.g. EENTER/SYSCALL load RCX with the next RIP and EEXIT/SYSRET load RIP from R{B,C}X.

But in a faulting/interrupting scenario, enclave transitions act more like VM-Exit and VMRESUME. Maintaining the black box nature of the enclave means that hardware must automatically switch CPU context when an Asynchronous Exiting Event (AEE) occurs, an AEE being any interrupt or exception (exceptions are AEEs because asynchronous in this context is relative to the enclave and not CPU execution, e.g. the enclave doesn't get an opportunity to save/fuzz CPU state).

Like VM-Exits, all AEEs jump to a common location, referred to as the Asynchronous Exiting Point (AEP). The AEP is specified at enclave entry via register passed to EENTER/ERESUME, similar to how the hypervisor specifies the VM-Exit point (via VMCS.HOST_RIP at VMLAUNCH/VMRESUME). Resuming the enclave/VM after the exiting event is handled is done via ERESUME/VMRESUME respectively. In SGX, AEEs that are handled by the kernel, e.g. INTR, NMI and most page faults, IRET will journey back to the AEP which then ERESUMEs th enclave.

Enclaves also behave a bit like VMs in the sense that they can generate exceptions as part of their normal operation that for all intents and purposes need to handled in the enclave/VM. However, unlike VMX, SGX doesn't allow the host to modify its guest's, a.k.a. enclave's, state, as doing so would circumvent the enclave's security. So to handle an exception, the enclave must first be re-entered through the normal EENTER flow (SYSCALL/SYSRET behavior), and then resumed via ERESUME (VMRESUME behavior) after the source of the exception is resolved.

All of the above is just the tip of the iceberg when it comes to running an enclave. But, SGX was designed in such a way that the host process can utilize a library to build, launch and run an enclave. This is roughly analogous to how e.g. libc implementations are used by most applications so that the application can focus on its business logic.

The big gotcha is that because enclaves can generate *and* handle exceptions, any SGX library must be prepared to handle nearly any exception at any time (well, any time a thread is executing in an enclave). In Linux, this means the SGX library must register a signal handler in order to intercept relevant exceptions and forward them to the enclave (or in some cases, take action on behalf of the enclave). Unfortunately, Linux's signal mechanism doesn't mesh well with libraries, e.g. signal handlers are process wide, are difficult to chain, etc... This becomes particularly nasty when using multiple levels of libraries that register signal handlers, e.g. running an enclave via cgo inside of the Go runtime.

In comes vDSO to save the day. Now that vDSO can fixup exceptions, add a function to wrap enclave transitions and intercept any exceptions that occur in the enclave or on EENTER/ERESUME. The actually code is blissfully short (especially compared to this changelog).

In addition to the obvious trapnr, error_code and address, propagate the leaf number, i.e. RAX, back to userspace so that the caller can know whether the fault occurred in the enclave or if it occurred on EENTER. A fault on EENTER generally means the enclave has died and needs to be restarted.

Suggested-by Andy Lutomirski <[email protected]>

Cc Andy Lutomirski <[email protected]>

Cc Jarkko Sakkinen <[email protected]>

Cc Dave Hansen <[email protected]>

Cc Josh Triplett <[email protected]>

Signed-off-by Sean Christopherson <[email protected]>

---
 arch/x86/entry/vdso/Makefile      |   1 +
 arch/x86/entry/vdso/vdso.lds.S    |   1 +
 arch/x86/entry/vdso/vsgx_eenter.c | 108 ++++++++++++++++++++++++++++++
 3 files changed, 110 insertions(+)
 create mode 100644 arch/x86/entry/vdso/vsgx_eenter.c

arch/x86/entry/vdso/Makefile +1 -0

index eb543ee1bcec..ba46673076bd 100644
@@ -18,6 +18,7 @@ VDSO32-$(CONFIG_IA32_EMULATION)       := y
 
 # files to link into the vdso
 vobjs-y := vdso-note.o vclock_gettime.o vgetcpu.o
+vobjs-$(VDSO64-y)              += vsgx_eenter.o
 
 # files to link into kernel
 obj-y                          += vma.o extable.o

arch/x86/entry/vdso/vdso.lds.S +1 -0

index d3a2dce4cfa9..e422c4454f34 100644
@@ -25,6 +25,7 @@ VERSION {
                __vdso_getcpu;
                time;
                __vdso_time;
+               __vdso_sgx_eenter;
        local: *;
        };
 }

arch/x86/entry/vdso/vsgx_eenter.c new file +41 -0

new file mode 100644
index 000000000000..3df4a95a34cc
@@ -0,0 +1,108 @@
+// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause)
+// Copyright(c) 2018 Intel Corporation.
+
+#include <uapi/linux/errno.h>
+#include <uapi/linux/types.h>
+
+#include "extable.h"
+
+/*
+ * This struct will be defined elsewhere in the actual implementation,
+ * e.g. arch/x86/include/uapi/asm/sgx.h.
+ */
+struct sgx_eenter_fault_info {
+       __u32   leaf;
+       __u16   trapnr;
+       __u16   error_code;
+       __u64   address;
+};
+
+/*
+ * ENCLU (ENCLave User) is an umbrella instruction for a variety of CPL3
+ * SGX functions,  The ENCLU function that is executed is specified in EAX,
+ * with each function potentially having more leaf-specific operands beyond
+ * EAX.  In the vDSO we're only concerned with the leafs that are used to
+ * transition to/from the enclave.
+ */
+enum sgx_enclu_leaves {
+       SGX_EENTER      = 2,
+       SGX_ERESUME     = 3,
+       SGX_EEXIT       = 4,
+};
+
+notrace long __vdso_sgx_eenter(void *tcs, void *priv,
+                              struct sgx_eenter_fault_info *fault_info)
+{
+       u32 trapnr, error_code;
+       long leaf;
+       u64 addr;
+
+       /*
+        *      %eax = EENTER

Quoted 14 lines · Sean Christopherson

arch/x86/entry/vdso/vsgx_eenter.c new file +14 -0

+notrace long __vdso_sgx_eenter(void *tcs, void *priv,
+                              struct sgx_eenter_fault_info *fault_info)
+{
+       u32 trapnr, error_code;
+       long leaf;
+       u64 addr;
+
+       /*
+        *      %eax = EENTER
+        *      %rbx = tcs
+        *      %rcx = do_eresume
+        *      %rdi = priv
+        * do_eenter:
+        *      enclu

arch/x86/entry/vdso/vsgx_eenter.c line 51

Quoted 5 lines

arch/x86/entry/vdso/vsgx_eenter.c new file +5 -0

+        *      jmp     out
+        *
+        * do_eresume:
+        *      enclu
+        *      ud2

Is the only reason for do_eresume to be different from do_eenter so that you can do the ud2?

arch/x86/entry/vdso/vsgx_eenter.c line 59

Quoted 8 lines

arch/x86/entry/vdso/vsgx_eenter.c new file +8 -0

+        *
+        * out:
+        *      <return to C code>
+        *
+        * fault_fixup:
+        *      <extable loads RDI, DSI and RDX with fault info>
+        *      jmp     out
+        */

This has the IMO excellent property that it's extremely awkward to use it for a model where the enclave is reentrant. I think it's excellent because reentrancy on the same enclave thread is just asking for severe bugs. Of course, I fully expect the SDK to emulate reentrancy, but then it's 100% their problem :) On the fiip side, it means that you can't really recover from a reported fault, even if you want to, because there's no way to ask for ERESUME. So maybe the API should allow that after all.

I think it might be polite to at least give some out regs, maybe RSI and RDI?

Quoted 16 lines · Andy Lutomirski

Quoted 14 lines · Sean Christopherson

arch/x86/entry/vdso/vsgx_eenter.c new file +14 -0

+notrace long __vdso_sgx_eenter(void *tcs, void *priv,
+                              struct sgx_eenter_fault_info *fault_info)
+{
+       u32 trapnr, error_code;
+       long leaf;
+       u64 addr;
+
+       /*
+        *      %eax = EENTER
+        *      %rbx = tcs
+        *      %rcx = do_eresume
+        *      %rdi = priv
+        * do_eenter:
+        *      enclu

Quoted 8 lines

arch/x86/entry/vdso/vsgx_eenter.c line 51

Quoted 5 lines

arch/x86/entry/vdso/vsgx_eenter.c new file +5 -0

+        *      jmp     out
+        *
+        * do_eresume:
+        *      enclu
+        *      ud2

Is the only reason for do_eresume to be different from do_eenter so that you can do the ud2?

No, it was a holdover from doing fixup via a magic prefix in user code. The fixup could only skip the ENCLU and so a second ENCLU was needed to differentiate between EENTER and ERESUME. The need for two ENCLUs got ingrained in my head. I can't think of anything that will break if we use a single ENCLU.

Quoted 9 lines

arch/x86/entry/vdso/vsgx_eenter.c line 59

Quoted 8 lines

arch/x86/entry/vdso/vsgx_eenter.c new file +8 -0

+        *
+        * out:
+        *      <return to C code>
+        *
+        * fault_fixup:
+        *      <extable loads RDI, DSI and RDX with fault info>
+        *      jmp     out
+        */

Quoted 8 lines

This has the IMO excellent property that it's extremely awkward to use it for a model where the enclave is reentrant. I think it's excellent because reentrancy on the same enclave thread is just asking for severe bugs. Of course, I fully expect the SDK to emulate reentrancy, but then it's 100% their problem :) On the fiip side, it means that you can't really recover from a reported fault, even if you want to, because there's no way to ask for ERESUME. So maybe the API should allow that after all.

Doh. The ability to do ERESUME is an explicit requirement from the SDK folks. More code that I pulled from my userspace implementation and didn't revisit.