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

From: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
To: Sean Christopherson <sean.j.christopherson@intel.com>
Cc: Dave Hansen <dave.hansen@intel.com>,
	linux-sgx@vger.kernel.org, x86@kernel.org,
	Haitao Huang <haitao.huang@linux.intel.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	Borislav Petkov <bp@alien8.de>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	Andy Lutomirski <luto@amacapital.net>,
	Cedric Xing <cedric.xing@intel.com>
Subject: Re: [PATCH] x86/vdso: Remove retpoline from SGX vDSO call
Date: Wed, 30 Sep 2020 19:38:22 +0300	[thread overview]
Message-ID: <20200930163822.GA55565@linux.intel.com> (raw)
In-Reply-To: <20200930154349.GB32672@linux.intel.com>

On Wed, Sep 30, 2020 at 08:43:49AM -0700, Sean Christopherson wrote:
> On Wed, Sep 30, 2020 at 06:28:06PM +0300, Jarkko Sakkinen wrote:
> > On Wed, Sep 30, 2020 at 07:33:38AM -0700, Dave Hansen wrote:
> > > On 9/30/20 7:20 AM, Jarkko Sakkinen wrote:
> > > > I'm not expert on Spectre, or any sort of security researcher, but I've
> > > > read a few papers about and understand the general concept. With the
> > > > constraints how the callback is used in practice, I'd *guess* it is
> > > > fine to drop retpoline but I really need some feedback on this from
> > > > people who understand these attacks better.
> > > 
> > > 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

This patch from v20-v21 era is also relevant:

https://lore.kernel.org/linux-sgx/ba2a51568f3adaf74994d33ea3cbee570e20c6f6.1555965327.git.cedric.xing@intel.com/

It introduced the retpoline wrapping to the patch set but unfortunately
does not have any explanation for that particular detail.

Neither does Andy's comment except correctly stating that retpoline is
the modern standard for indirect calls but that does not get us too far.

My argument, or maybe just actually a question, is essentially that
given the usage pattern for this particular indirect call, do we need to
actually retpoline it?

Boot time patching turned out to be simple to implement but it is also
yet another whistle for the already complex vDSO.

/Jarkko