Re: [PATCH] x86/vdso: Remove retpoline from SGX vDSO call

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 30/09/2020 18:01, Jethro Beekman wrote:
> On 2020-09-30 18:28, Dave Hansen wrote:
>> On 9/30/20 8:43 AM, Sean Christopherson wrote:
>>>>> Do you recall why you added it in the first place?  What was the
>>>>> motivation for it?  Were you responding to a review comment?
>>>> Absolutely cannot recall it :-) I even cannot recall the exact time when
>>>> we landed the vDSO in the first place. Too much stuff has happend during
>>>> the long three year upstreaming cycle. I will try to backtrack this
>>>> info.
>>> It originated in a comment from Andy when we were discussing the legitimacy
>>> of the callback.  From that point on it got taken as gospel that the indirect
>>> call would be implemented as a retpoline.
>>>
>>> https://lkml.kernel.org/r/CALCETrVBR+2HjTqX=W4r9GOq69Xg36v4gmCKqK0wUjzAqBJnrw@mail.gmail.com
>> OK, so that was Andy L. saying:
>>
>>> But I have a real argument for dropping exit_handler: in this new age
>>> of Spectre, the indirect call is a retpoline, and it's therefore quite
>>> slow.  So I'm not saying NAK, but I do think it's unnecessary.
>> It sounds like we were never able to jettison the indirect call.  So,
>> we've got a kernel-provided indirect call that's only ever executed by
>> userspace.  A quick grep didn't show any other indirect branches in the
>> VDSO, so there might not be good precedent for what to do.
>>
>> The problem with the VDSO is that even if userspace is compiled to the
>> gills with mitigations, this VDSO branch won't be mitigated since it
>> comes from the kernel.
>>
>> So, here's the big question for me:  How does a security-sensitive
>> userspace *binary* make mitigated indirect calls these days?
>>
>> Seems like the kind of thing for which Intel should have a good writeup.  :)
>>
> If by "these days" you mean also protecting against LVI, then it'd be like so, I think:
>
> lfence 
> callq  *%rax
>
> (at least this is what the LLVM LVI hardening pass does)

The problem is that "what to do about speculation" is exceedingly
complicated.

The 2017/8 code samples to beat Spectre v2 are one of:

1) Retpoline (not totally safe on Skylake uarch, but pretty good)
2) (legacy) IBRS and CALL *reg/mem (Intel)
3) LFENCE; CALL *reg/mem (AMD)

Later (i.e. this year), there is finally public documentation from all
vendors concerning straight-line speculation, so the code fragments now
need to be:

1) Retpoline (not totally safe on Skylake uarch, but pretty good)
2) (legacy) IBRS and CALL *reg/mem; LFENCE (Intel)
3) LFENCE; CALL *reg/mem; LFENCE (AMD)

to avoid potential speculative type confusion in the remainder of the
basic block.

Same goes for indirect JMP (Intel and AMD) and RET (AMD only), although
as there is no architectural execution, you can use INT3/INT1 to halt
speculation with lower code volume than LFENCE.


As noted, to avoid LVI poisoning of the branch target, a leading LFENCE
is needed (or one of the even more complicated variations), including
ahead of the RET of a retpoline.  However in practice, its only code
inside the enclave which is possibly worth protecting in this way.


Intel hardware mitigations, eIBRS, comes recommended with switching back
to a plain CALL *reg/mem, until a recent paper where it was demonstrated
that even with eIBRS active, you can still get speculative type
confusion due to same-mode training.  If same-mode training is a
problem, the recommendation is to use eIBRS and Retpoline.

AMD hardware mitigations, simply called IBRS, is programmed in the same
way as eIBRS (i.e. turn it on at the start of day and leave it on), does
guarantee to to protect against same-mode training.


As for the VDSO, it is userspace, and there's no way that it will have
WRSS generally enabled.  Therefore, it cannot use a retpoline gadget
even with the "fix up the shadow stack" variation when CET is in use. 
Any of the other variations ought to be CET-safe.


What, if anything, userspace does to protect against Spectre attacks is
a matter of every piece of compiled code in every library loaded, and
the risk profile of the application itself.  A browser with a javascript
sandbox is liable to try and take more care than something like cowsay.


Honestly, my advice would be to leave it unprotected for now.  Anyone
who managed to figure out the rest of the practical userspace issues
will probably have a much better idea of what can/should be done in this
case.

If that doesn't sit well with people, then the next best would probably
be LFENCE; CALL *reg/mem; LFENCE to cover as many of the corner cases as
possible without being incompatible with CET.  Its not as if this
callback is the slow aspect of entering/exiting SGX mode.

~Andrew