From: Sean Christopherson <sean.j.christopherson@intel.com> To: Borislav Petkov <bp@alien8.de> Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>, x86@kernel.org, linux-sgx@vger.kernel.org, linux-kernel@vger.kernel.org, Jethro Beekman <jethro@fortanix.com>, akpm@linux-foundation.org, andriy.shevchenko@linux.intel.com, asapek@google.com, cedric.xing@intel.com, chenalexchen@google.com, conradparker@google.com, cyhanish@google.com, dave.hansen@intel.com, haitao.huang@intel.com, josh@joshtriplett.org, kai.huang@intel.com, kai.svahn@intel.com, kmoy@google.com, ludloff@google.com, luto@kernel.org, nhorman@redhat.com, npmccallum@redhat.com, puiterwijk@redhat.com, rientjes@google.com, tglx@linutronix.de, yaozhangx@google.com Subject: Re: [PATCH v33 03/21] x86/mm: x86/sgx: Signal SIGSEGV with PF_SGX Date: Thu, 25 Jun 2020 08:34:31 -0700 Message-ID: <20200625153431.GA3437@linux.intel.com> (raw) In-Reply-To: <20200625085931.GB20319@zn.tnic> On Thu, Jun 25, 2020 at 10:59:31AM +0200, Borislav Petkov wrote: > On Thu, Jun 18, 2020 at 01:08:25AM +0300, Jarkko Sakkinen wrote: > > diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c > > index 66be9bd60307..25d48aae36c1 100644 > > --- a/arch/x86/mm/fault.c > > +++ b/arch/x86/mm/fault.c > > @@ -1055,6 +1055,19 @@ access_error(unsigned long error_code, struct vm_area_struct *vma) > > if (error_code & X86_PF_PK) > > return 1; > > > > + /* > > + * Access is blocked by the Enclave Page Cache Map (EPCM), i.e. the > > + * access is allowed by the PTE but not the EPCM. This usually happens > > + * when the EPCM is yanked out from under us, e.g. by hardware after a > > + * suspend/resume cycle. In any case, software, i.e. the kernel, can't > > + * fix the source of the fault as the EPCM can't be directly modified by > > + * software. Handle the fault as an access error in order to signal > > + * userspace so that userspace can rebuild their enclave(s), even though > > + * userspace may not have actually violated access permissions. > > + */ > > Lemme check whether I understand this correctly: userspace must check > whether the SIGSEGV is generated on an access to an enclave page? Sort of. Technically it's that's an accurate statement, but practically speaking userspace can only access enclave pages when it is executing in the enclave, and exceptions in enclaves have unique behavior. Exceptions in enclaves essentially bounce through a userspace-software-defined location prior to being delivered to the kernel. The trampoline is done by the CPU so that the CPU can scrub the GPRs, XSAVE state, etc... and hide the true RIP of the exception. The pre-exception enclave state is saved into protected memory and restored when userspace resumes the enclave. Enterring or resuming an enclave can only be done through dedicted ENCLU instructions, so really it ends up being that the SIGSEGV handler needs to check the IP that "caused" the fault, which is actually the IP of the trampoline. But, that's only the first half of the story... > Also, do I see it correctly that when this happens, dmesg will have > > printk("%s%s[%d]: segfault at %lx ip %px sp %px error %lx", > > due to: > > if (likely(show_unhandled_signals)) > show_signal_msg(regs, error_code, address, tsk); > > which does: > > if (!unhandled_signal(tsk, SIGSEGV)) > return; > > or is the task expected to register a SIGSEGV handler so that the > segfault doesn't land in dmesg? Yes, without extra help, any task running an enclave is expected to register a SIGSEGV handler so that the task can restart the enclave if the EPC is "lost". However, building and running enclaves is complex, and the vast majority of SGX enabled applications are expected to leverage a library of one kind or another to hand the bulk of the gory details. But, signal handling in libraries is a mess, e.g. requires filtering/forwarding, resignaling, etc... To that end, in v14 of this patch[1], Andy Lutomirski came up with the idea of adding a vDSO function to provide the low level enclave EENTER/ERESUME and trampoline, and then teaching the kernel to do exception fixup on the relevant instructions in the vDSO. The vDSO's exception fixup then returns to normal userspace, with a (technically optional) struct holding the details of the exception. That allows for synchronous delivery of exceptions in enclaves, obviates the need for userspace to regsiter a SIGSEGV handler, and also means the SIGSEGV will never show up in dmesg so long as userspace is using the vDSO. The kernel still supports direct EENTER/ERESUME, but AFAIK everyone is moving (or has moved) to the vDSO interface. The vDSO stuff is in patches 15-18 of this series. There's a gigantic thread on all the alternatives that were considered[2]. [1] https://lkml.kernel.org/r/CALCETrXByb2UVuZ6AXUeOd8y90NAikbZuvdN3wf_TjHZ+CxNhA@mail.gmail.com [2] https://lkml.kernel.org/r/CALCETrWdpoDkbZjkucKL91GWpDPG9p=VqYrULade2pFDR7S=GQ@mail.gmail.com > > If so, are we documenting this? > > If not, then we should not issue any "segfault" messages to dmesg > because that would be wrong. > > Or maybe I'm not seeing it right but I don't have the hardware to test > this out... > > Thx. > > -- > Regards/Gruss, > Boris. > > https://people.kernel.org/tglx/notes-about-netiquette
next prev parent reply index Thread overview: 93+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-06-17 22:08 [PATCH v33 00/21] Intel SGX foundations Jarkko Sakkinen 2020-06-17 22:08 ` [PATCH v33 01/21] x86/cpufeatures: x86/msr: Add Intel SGX hardware bits Jarkko Sakkinen 2020-06-22 17:37 ` Borislav Petkov 2020-06-25 1:25 ` Jarkko Sakkinen 2020-06-17 22:08 ` [PATCH v33 02/21] x86/cpufeatures: x86/msr: Add Intel SGX Launch Control " Jarkko Sakkinen 2020-06-24 13:04 ` Borislav Petkov 2020-06-24 14:34 ` Sean Christopherson 2020-06-25 1:28 ` Jarkko Sakkinen 2020-06-17 22:08 ` [PATCH v33 03/21] x86/mm: x86/sgx: Signal SIGSEGV with PF_SGX Jarkko Sakkinen 2020-06-25 8:59 ` Borislav Petkov 2020-06-25 15:34 ` Sean Christopherson [this message] 2020-06-25 16:49 ` Borislav Petkov 2020-06-25 20:52 ` Jarkko Sakkinen 2020-06-25 21:11 ` Borislav Petkov 2020-06-26 13:34 ` Jarkko Sakkinen 2020-06-17 22:08 ` [PATCH v33 04/21] x86/sgx: Add SGX microarchitectural data structures Jarkko Sakkinen 2020-06-17 22:08 ` [PATCH v33 05/21] x86/sgx: Add wrappers for ENCLS leaf functions Jarkko Sakkinen 2020-06-17 22:08 ` [PATCH v33 06/21] x86/cpu/intel: Detect SGX support Jarkko Sakkinen 2020-06-17 22:08 ` [PATCH v33 07/21] x86/cpu/intel: Add nosgx kernel parameter Jarkko Sakkinen 2020-06-17 22:08 ` [PATCH v33 08/21] x86/sgx: Initialize metadata for Enclave Page Cache (EPC) sections Jarkko Sakkinen 2020-06-25 10:14 ` Borislav Petkov 2020-06-25 20:11 ` Jarkko Sakkinen 2020-06-17 22:08 ` [PATCH v33 09/21] x86/sgx: Add __sgx_alloc_epc_page() and sgx_free_epc_page() Jarkko Sakkinen 2020-06-25 17:06 ` Borislav Petkov 2020-06-25 20:55 ` Jarkko Sakkinen 2020-06-17 22:08 ` [PATCH v33 10/21] mm: Introduce vm_ops->may_mprotect() Jarkko Sakkinen 2020-06-25 17:14 ` Borislav Petkov 2020-06-25 17:30 ` Matthew Wilcox 2020-06-25 18:06 ` Sean Christopherson 2020-06-25 22:40 ` Jarkko Sakkinen 2020-06-25 22:26 ` Jarkko Sakkinen 2020-06-17 22:08 ` [PATCH v33 11/21] x86/sgx: Linux Enclave Driver Jarkko Sakkinen 2020-06-25 17:23 ` Borislav Petkov 2020-06-25 18:34 ` Sean Christopherson 2020-06-25 18:45 ` Borislav Petkov 2020-06-26 14:19 ` Jarkko Sakkinen 2020-06-25 20:21 ` Jarkko Sakkinen 2020-06-25 20:25 ` Borislav Petkov 2020-06-26 13:40 ` Jarkko Sakkinen 2020-06-25 18:53 ` Borislav Petkov 2020-06-26 14:17 ` Jarkko Sakkinen 2020-06-26 9:14 ` Borislav Petkov 2020-06-26 14:16 ` Sean Christopherson 2020-06-26 14:20 ` Borislav Petkov 2020-07-03 23:04 ` Jarkko Sakkinen 2020-07-03 3:09 ` Jarkko Sakkinen 2020-06-26 15:34 ` Borislav Petkov 2020-07-04 0:13 ` Jarkko Sakkinen 2020-10-26 21:26 ` Dave Hansen 2020-10-27 1:52 ` Jarkko Sakkinen 2020-10-27 10:05 ` Borislav Petkov 2020-10-27 15:20 ` Dave Hansen 2020-10-27 15:37 ` Borislav Petkov 2020-06-27 17:43 ` Borislav Petkov 2020-06-29 15:27 ` Sean Christopherson 2020-06-29 15:37 ` Borislav Petkov 2020-07-04 1:43 ` Jarkko Sakkinen 2020-07-07 1:38 ` Sean Christopherson 2020-07-07 3:29 ` Jarkko Sakkinen 2020-07-04 1:42 ` Jarkko Sakkinen 2020-07-02 3:59 ` Sean Christopherson 2020-07-04 3:31 ` Jarkko Sakkinen 2020-09-02 3:06 ` Haitao Huang 2020-09-02 16:10 ` Sean Christopherson 2020-09-02 18:40 ` Haitao Huang 2020-09-04 12:01 ` Jarkko Sakkinen 2020-06-17 22:08 ` [PATCH v33 12/21] x86/sgx: Allow a limited use of ATTRIBUTE.PROVISIONKEY for attestation Jarkko Sakkinen 2020-06-29 16:02 ` Borislav Petkov 2020-06-29 22:04 ` Sean Christopherson 2020-06-30 8:49 ` Borislav Petkov 2020-06-30 14:20 ` Sean Christopherson 2020-06-30 17:13 ` Andy Lutomirski 2020-07-02 20:47 ` Dr. Greg 2020-07-03 2:43 ` Jarkko Sakkinen 2020-07-03 2:38 ` Jarkko Sakkinen 2020-07-03 2:32 ` Jarkko Sakkinen 2020-07-03 2:55 ` Jarkko Sakkinen 2020-06-17 22:08 ` [PATCH v33 13/21] x86/sgx: Add a page reclaimer Jarkko Sakkinen 2020-06-17 22:08 ` [PATCH v33 14/21] x86/sgx: ptrace() support for the SGX driver Jarkko Sakkinen 2020-06-17 22:08 ` [PATCH v33 15/21] x86/vdso: Add support for exception fixup in vDSO functions Jarkko Sakkinen 2020-06-29 17:10 ` Borislav Petkov 2020-06-30 6:00 ` Sean Christopherson 2020-06-30 8:41 ` Borislav Petkov 2020-06-30 14:55 ` Sean Christopherson 2020-06-30 16:48 ` Andy Lutomirski 2020-06-30 17:23 ` Sean Christopherson 2020-07-02 12:52 ` Thomas Gleixner 2020-06-17 22:08 ` [PATCH v33 16/21] x86/fault: Add helper function to sanitize error code Jarkko Sakkinen 2020-06-17 22:08 ` [PATCH v33 17/21] x86/traps: Attempt to fixup exceptions in vDSO before signaling Jarkko Sakkinen 2020-06-17 22:08 ` [PATCH v33 18/21] x86/vdso: Implement a vDSO for Intel SGX enclave call Jarkko Sakkinen 2020-06-17 22:08 ` [PATCH v33 19/21] selftests/x86: Add a selftest for SGX Jarkko Sakkinen 2020-06-17 22:08 ` [PATCH v33 20/21] docs: x86/sgx: Document SGX micro architecture and kernel internals Jarkko Sakkinen 2020-06-17 22:08 ` [PATCH v33 21/21] x86/sgx: Update MAINTAINERS Jarkko Sakkinen
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20200625153431.GA3437@linux.intel.com \ --to=sean.j.christopherson@intel.com \ --cc=akpm@linux-foundation.org \ --cc=andriy.shevchenko@linux.intel.com \ --cc=asapek@google.com \ --cc=bp@alien8.de \ --cc=cedric.xing@intel.com \ --cc=chenalexchen@google.com \ --cc=conradparker@google.com \ --cc=cyhanish@google.com \ --cc=dave.hansen@intel.com \ --cc=haitao.huang@intel.com \ --cc=jarkko.sakkinen@linux.intel.com \ --cc=jethro@fortanix.com \ --cc=josh@joshtriplett.org \ --cc=kai.huang@intel.com \ --cc=kai.svahn@intel.com \ --cc=kmoy@google.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-sgx@vger.kernel.org \ --cc=ludloff@google.com \ --cc=luto@kernel.org \ --cc=nhorman@redhat.com \ --cc=npmccallum@redhat.com \ --cc=puiterwijk@redhat.com \ --cc=rientjes@google.com \ --cc=tglx@linutronix.de \ --cc=x86@kernel.org \ --cc=yaozhangx@google.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
Linux-Sgx Archive on lore.kernel.org Archives are clonable: git clone --mirror https://lore.kernel.org/linux-sgx/0 linux-sgx/git/0.git # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V2 linux-sgx linux-sgx/ https://lore.kernel.org/linux-sgx \ linux-sgx@vger.kernel.org public-inbox-index linux-sgx Example config snippet for mirrors Newsgroup available over NNTP: nntp://nntp.lore.kernel.org/org.kernel.vger.linux-sgx AGPL code for this site: git clone https://public-inbox.org/public-inbox.git