linux-kernel - RE: [PATCH 1/2] x86: cpu/bugs: add support for AMD ERAPS feature

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID:
 <LV3PR12MB9265F0B446D1F6070431830494522@LV3PR12MB9265.namprd12.prod.outlook.com>
Date: Tue, 5 Nov 2024 14:54:31 +0000
From: "Kaplan, David" <David.Kaplan@....com>
To: "Shah, Amit" <Amit.Shah@....com>, "kvm@...r.kernel.org"
	<kvm@...r.kernel.org>, "dave.hansen@...el.com" <dave.hansen@...el.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>, "x86@...nel.org"
	<x86@...nel.org>
CC: "corbet@....net" <corbet@....net>, "boris.ostrovsky@...cle.com"
	<boris.ostrovsky@...cle.com>, "kai.huang@...el.com" <kai.huang@...el.com>,
	"pawan.kumar.gupta@...ux.intel.com" <pawan.kumar.gupta@...ux.intel.com>,
	"jpoimboe@...nel.org" <jpoimboe@...nel.org>, "dave.hansen@...ux.intel.com"
	<dave.hansen@...ux.intel.com>, "daniel.sneddon@...ux.intel.com"
	<daniel.sneddon@...ux.intel.com>, "Lendacky, Thomas"
	<Thomas.Lendacky@....com>, "seanjc@...gle.com" <seanjc@...gle.com>,
	"mingo@...hat.com" <mingo@...hat.com>, "pbonzini@...hat.com"
	<pbonzini@...hat.com>, "tglx@...utronix.de" <tglx@...utronix.de>, "Moger,
 Babu" <Babu.Moger@....com>, "Das1, Sandipan" <Sandipan.Das@....com>,
	"hpa@...or.com" <hpa@...or.com>, "peterz@...radead.org"
	<peterz@...radead.org>, "bp@...en8.de" <bp@...en8.de>, Andrew Cooper
	<andrew.cooper3@...rix.com>
Subject: RE: [PATCH 1/2] x86: cpu/bugs: add support for AMD ERAPS feature

[AMD Official Use Only - AMD Internal Distribution Only]

> -----Original Message-----
> From: Shah, Amit <Amit.Shah@....com>
> Sent: Tuesday, November 5, 2024 4:40 AM
> To: kvm@...r.kernel.org; dave.hansen@...el.com; linux-kernel@...r.kernel.org;
> linux-doc@...r.kernel.org; x86@...nel.org
> Cc: corbet@....net; boris.ostrovsky@...cle.com; kai.huang@...el.com;
> pawan.kumar.gupta@...ux.intel.com; jpoimboe@...nel.org;
> dave.hansen@...ux.intel.com; daniel.sneddon@...ux.intel.com; Lendacky, Thomas
> <Thomas.Lendacky@....com>; seanjc@...gle.com; mingo@...hat.com;
> pbonzini@...hat.com; tglx@...utronix.de; Moger, Babu <Babu.Moger@....com>;
> Das1, Sandipan <Sandipan.Das@....com>; hpa@...or.com;
> peterz@...radead.org; bp@...en8.de; Kaplan, David <David.Kaplan@....com>
> Subject: Re: [PATCH 1/2] x86: cpu/bugs: add support for AMD ERAPS feature
>
> On Mon, 2024-11-04 at 09:45 -0800, Dave Hansen wrote:
> > On 11/4/24 09:22, Shah, Amit wrote:
> > > > I think you're wrong.  We can't depend on ERAPS for this.  Linux
> > > > doesn't flush the TLB on context switches when PCIDs are in play.
> > > > Thus, ERAPS won't flush the RSB and will leave bad state in there
> > > > and will leave the system vulnerable.
> > > >
> > > > Or what am I missing?
> > > I just received confirmation from our hardware engineers on this
> > > too:
> > >
> > > 1. the RSB is flushed when CR3 is updated 2. the RSB is flushed when
> > > INVPCID is issued (except type 0 - single address).
> > >
> > > I didn't mention 1. so far, which led to your question, right?
> >
> > Not only did you not mention it, you said something _completely_
> > different.  So, where the documentation for this thing?  I dug through
> > the 57230 .zip file and I see the CPUID bit:
> >
> >     24 ERAPS. Read-only. Reset: 1. Indicates support for enhanced
> >               return address predictor security.
> >
> > but nothing telling us how it works.
>
> I'm expecting the APM update come out soon, but I have put together
>
> https://amitshah.net/2024/11/eraps-reduces-software-tax-for-hardware-bugs/
>
> based on information I have.  I think it's mostly consistent with what I've said so far -
> with the exception of the mov-CR3 flush only confirmed yesterday.
>
> > > Does this now cover all the cases?
> >
> > Nope, it's worse than I thought.  Look at:
> >
> > > SYM_FUNC_START(__switch_to_asm)
> > ...
> > >         FILL_RETURN_BUFFER %r12, RSB_CLEAR_LOOPS,
> > > X86_FEATURE_RSB_CTXSW
> >
> > which does the RSB fill at the same time it switches RSP.
> >
> > So we feel the need to flush the RSB on *ALL* task switches.  That
> > includes switches between threads in a process *AND* switches over to
> > kernel threads from user ones.
>
> (since these cases are the same as those listed below, I'll only reply in one place)
>
> > So, I'll flip this back around.  Today, X86_FEATURE_RSB_CTXSW zaps the
> > RSB whenever RSP is updated to a new task stack.  Please convince me
> > that ERAPS provides superior coverage or is unnecessary in all the
> > possible combinations switching between:
> >
> >     different thread, same mm
>
> This case is the same userspace process with valid addresses in the RSB for that
> process.  An invalid speculation isn't security sensitive, just a misprediction that
> won't be retired.  So we are good here.

I think it's more accurate to say that the addresses in the RSB may be incorrect for the new thread, but they're still valid return sites since it's the same mm.  There are other existing cases (such as deep call trees) where RSB predictions for a thread may be incorrect but are still to valid return sites.

We could still trigger a RSB flush here I suppose, but I think there's an argument it is unnecessary.

--David Kaplan