Re: [PATCH v38 21/24] x86/vdso: Implement a vDSO for Intel SGX enclave call

[Jarkko Sakkinen]

On Mon, Sep 28, 2020 at 11:12:08AM -0700, Andy Lutomirski wrote:
> On Mon, Sep 28, 2020 at 11:08 AM H.J. Lu <hjl.tools@xxxxxxxxx> wrote:
> >
> > On Mon, Sep 28, 2020 at 9:44 AM Andrew Cooper <andrew.cooper3@xxxxxxxxxx> wrote:
> > >
> > > On 28/09/2020 01:58, Jarkko Sakkinen wrote:
> > > > On Fri, Sep 25, 2020 at 07:23:59PM +0100, Andrew Cooper wrote:
> > > >> On 15/09/2020 12:28, Jarkko Sakkinen wrote:
> > > >>> diff --git a/arch/x86/entry/vdso/vsgx_enter_enclave.S b/arch/x86/entry/vdso/vsgx_enter_enclave.S
> > > >>> new file mode 100644
> > > >>> index 000000000000..adbd59d41517
> > > >>> --- /dev/null
> > > >>> +++ b/arch/x86/entry/vdso/vsgx_enter_enclave.S
> > > >>> @@ -0,0 +1,157 @@
> > > >>> +SYM_FUNC_START(__vdso_sgx_enter_enclave)
> > > >>> <snip>
> > > >>> +.Lretpoline:
> > > >>> +   call    2f
> > > >>> +1: pause
> > > >>> +   lfence
> > > >>> +   jmp     1b
> > > >>> +2: mov     %rax, (%rsp)
> > > >>> +   ret
> > > >> I hate to throw further spanners in the work, but this is not compatible
> > > >> with CET, and the user shadow stack work in progress.
> > > > CET goes beyond my expertise. Can you describe, at least rudimentary,
> > > > how this code is not compatible?
> > >
> > > CET Shadow Stacks detect attacks which modify the return address on the
> > > stack.
> > >
> > > Retpoline *is* a ROP gadget.  It really does modify the return address
> > > on the stack, even if its purpose is defensive (vs Spectre v2) rather
> > > than malicious.
> > >
> > > >> Whichever of these two large series lands first is going to inflict
> > > >> fixing this problem on the other.
> > > >>
> > > >> As the vdso text is global (to a first approximation), it must not be a
> > > >> retpoline if any other process is liable to want to use CET-SS.
> > > > Why is that?
> > >
> > > Because when CET-SS is enabled, the ret will suffer a #CP exception
> > > (return address on the stack not matching the one recorded in the shadow
> > > stack), which I presume/hope is wired into SIGSEGV.
> > >
> >
> > Here is the CET compatible retpoline:
> >
> > endbr64
> > /* Check if shadow stack is in use.  NB: R11 is the only usable
> >    scratch register for function calls.  */
> > xorl %r11d, %r11d
> > rdsspq %r11
> > testq %r11, %r11
> > jnz 3f
> > call 2f
> > 1:
> > pause
> > lfence
> > jmp 1b
> > 2:
> > mov %rax, (%rsp)
> > ret
> > 3:
> > /* Shadow stack is in use.  Make the indirect call.  */
> > call *%rax
> > ret
> 
> What do we expect user programs to do on CET systems?  It would be
> nice if we could instead ALTERNATIVE this out if X86_FEATURE_SHSTK.
> 
> --Andy

I'm open to do either solution. My thinking was to initially do things
vsgx.S local (i.e. consider ALTERNATIVE post upstreaming) and use the
above solution but I'm also fine doing ALTERNATIVE. Dave kindly briefed
on details how that thing works and it should be perfectly usable for
our use case.

/Jarkko