Date: Fri, 5 Jan 2018 17:37:49 +0100
From: Andrea Arcangeli <aarcange@redhat.com>
To: "gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>
Cc: "Woodhouse, David" <dwmw@amazon.co.uk>,
        "tim.c.chen@linux.intel.com" <tim.c.chen@linux.intel.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "arjan.van.de.ven@intel.com" <arjan.van.de.ven@intel.com>,
        "torvalds@linux-foundation.org" <torvalds@linux-foundation.org>,
        "tglx@linutronix.de" <tglx@linutronix.de>,
        "ak@linux.intel.com" <ak@linux.intel.com>,
        "luto@kernel.org" <luto@kernel.org>,
        "dave.hansen@intel.com" <dave.hansen@intel.com>
Subject: Re: [PATCH 5/7] x86: Use IBRS for firmware update path
Message-ID: <20180105163749.GV26807@redhat.com>
References: <cover.1515086770.git.tim.c.chen@linux.intel.com>
 <8d3710432534b27d224283557c4629cd1aa5b0ea.1515086770.git.tim.c.chen@linux.intel.com>
 <20180104200515.GC30228@kroah.com>
 <1515096535.29312.38.camel@amazon.co.uk>
 <20180105160848.GD17349@kroah.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20180105160848.GD17349@kroah.com>
User-Agent: Mutt/1.9.2 (2017-12-15)
Sender: linux-kernel-owner@vger.kernel.org

On Fri, Jan 05, 2018 at 05:08:48PM +0100, Greg Kroah-Hartman wrote:
> On Thu, Jan 04, 2018 at 08:08:55PM +0000, Woodhouse, David wrote:
> > On Thu, 2018-01-04 at 21:05 +0100, Greg KH wrote:
> > > On Thu, Jan 04, 2018 at 09:56:46AM -0800, Tim Chen wrote:
> > > > 
> > > > From: David Woodhouse <dwmw@amazon.co.uk>
> > > > 
> > > > We are impervious to the indirect branch prediction attack with
> > > > retpoline
> > > > but firmware won't be, so we still need to set IBRS to protect
> > > > firmware code execution when calling into firmware at runtime.
> > > Wait, what?
> > > 
> > > Maybe it's just the wine from dinner talking, but if the firmware has
> > > issues, we have bigger things to worry about here, right???It already
> > > handed over the "chain of trust" to us, so we have already implicitly
> > > trusted that the firmware was correct here.??So why do we need to do
> > > anything about firmware calls in this manner?
> > 
> > In the ideal world, firmware exists to boot the kernel and then it gets
> > out of the way, never to be thought of again.
> > 
> > In the Intel world, firmware idiocy permeates everything and we
> > sometimes end up making calls to it at runtime.
> > 
> > If an attacker can poison the BTB for an indirect branch in EFI
> > firmware, then reliably do something which calls EFI runtime calls,
> > that might lead to an exploit. So if we're using retpoline for the
> > kernel, then we should be setting IBRS before any firmware calls.
> 
> Ugh, ok, seems a bit far-fetched to me, but I will not object anymore.
> 
> Except that the patch doesn't actually build, which means no one has
> actually tested it at all :(

One more wish, I personally would prefer the whole:

+#if defined(RETPOLINE)                                                                                               

to be dropped too and if this could be respinned without any REPTOLINE
knowledge at all. It can't be activated anyway so it should go in a
different patchset that can actually activate it if something but
that's much lower prio.

It was great info to know reptoline still needs IBRS anyway though.

Thanks,
Andrea