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

[Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>]

On Thu, Oct 01, 2020 at 12:22:20AM +0300, Jarkko Sakkinen wrote:
> On Wed, Sep 30, 2020 at 01:45:52PM -0700, Xing, Cedric wrote:
> > On 9/30/2020 12:25 PM, Jarkko Sakkinen wrote:
> > > On Wed, Sep 30, 2020 at 07:09:33PM +0100, Andrew Cooper wrote:
> > > > 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
> > > 
> > > I tend to agree. We cannot drive changes based on unknown unknowns.
> > > 
> > > And I don't see why we could not add boot time patching of retpoline
> > > even after the code is in the mainline kernel, if something ever
> > > pushes to that direction.
> > > 
> > > /Jarkko
> > > 
> > I agree. It'll be compatible with CET. The overhead of LFENCE is negligible
> > comparing to entering/exiting SGX mode.
> 
> Andrew's advice was to do "just call" as for now.
> 
> If we add also lfence, what is the real-world threat scenario that we
> are protecting against that exposes a real visible risk that could harm
> the users of these patches?
> 
> Please remember that:
> 
> 1. We can assume that the usage model and implementation is for the
>    callback is sane. It is something that is contained to the run-time
>    and there is just one instance of the callback.
> 2. We can always harden this more later on.
> 
> I do not want to add any extra bytes to the vDSO without any practical
> purpose and I also need to document this choice.

What if we  just keep everything as it is? Why boot time patching
cannot be part of CET-SS patch set?

Why?

1. Full reptoline is the safest alternative and we have it done already.
2. Before CET-SS there is no *functional* need to do boot time patching.
   The usual guideline is: do not add unused cruft to the kernel.

There is too much guesswork in other alternatives. If CET-SS actually
lands before SGX patches, then I'm happy to add in boot time patching.

AFAIK we actually could not add boot time patching without any
applications for it. That's incompatible with the common practices.

I'd leaving the code as it is and remark to the changelog that
CET-SS will require refining the reptoline to be optional.

/Jarkko