| Message ID | 20241111163913.36139-2-amit@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Add support for the ERAPS feature | expand |
On 11/11/24 08:39, Amit Shah wrote: > From: Amit Shah <amit.shah@amd.com> > > AMD CPUs do not fall back to the BTB when the RSB underflows for RET > address speculation. AMD CPUs have not needed to stuff the RSB for > underflow conditions. > > The RSB poisoning case is addressed by RSB filling - clean up the FIXME > comment about it. This amounts to "Josh was wrong" in commit 9756bba284. Before moving forward with this, it would be great to get his ack on this to make sure you two are on the same page.
On Mon, Nov 11, 2024 at 05:39:11PM +0100, Amit Shah wrote: > From: Amit Shah <amit.shah@amd.com> > > AMD CPUs do not fall back to the BTB when the RSB underflows for RET > address speculation. AMD CPUs have not needed to stuff the RSB for > underflow conditions. > > The RSB poisoning case is addressed by RSB filling - clean up the FIXME > comment about it. I'm thinking the comments need more clarification in light of BTC and SRSO. This: > - * AMD has it even worse: *all* returns are speculated from the BTB, > - * regardless of the state of the RSB. is still true (mostly: "all" should be "some"), though it doesn't belong in the "RSB underflow" section. Also the RSB stuffing not only mitigates RET, it mitigates any other instruction which happen to be predicted as a RET. Which is presumably why it's still needed even when SRSO is enabled. Something like below? diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c index 47a01d4028f6..e95d3aa14259 100644 --- a/arch/x86/kernel/cpu/bugs.c +++ b/arch/x86/kernel/cpu/bugs.c @@ -1828,9 +1828,6 @@ static void __init spectre_v2_select_mitigation(void) * speculated return targets may come from the branch predictor, * which could have a user-poisoned BTB or BHB entry. * - * AMD has it even worse: *all* returns are speculated from the BTB, - * regardless of the state of the RSB. - * * When IBRS or eIBRS is enabled, the "user -> kernel" attack * scenario is mitigated by the IBRS branch prediction isolation * properties, so the RSB buffer filling wouldn't be necessary to @@ -1850,10 +1847,22 @@ static void __init spectre_v2_select_mitigation(void) * The "user -> user" scenario, also known as SpectreBHB, requires * RSB clearing. * + * AMD Branch Type Confusion (aka "AMD retbleed") adds some + * additional wrinkles: + * + * - A RET can be mispredicted as a direct or indirect branch, + * causing the CPU to speculatively branch to a BTB target, in + * which case the RSB filling obviously doesn't help. That case + * is mitigated by removing all the RETs (SRSO mitigation). + * + * - The RSB is not only used for architectural RET instructions, + * it may also be used for other instructions which happen to + * get mispredicted as RETs. Therefore RSB filling is still + * needed even when the RETs have all been removed by the SRSO + * mitigation. + * * So to mitigate all cases, unconditionally fill RSB on context * switches. - * - * FIXME: Is this pointless for retbleed-affected AMD? */ setup_force_cpu_cap(X86_FEATURE_RSB_CTXSW); pr_info("Spectre v2 / SpectreRSB mitigation: Filling RSB on context switch\n");
[AMD Official Use Only - AMD Internal Distribution Only] > -----Original Message----- > From: Josh Poimboeuf <jpoimboe@kernel.org> > Sent: Monday, November 11, 2024 1:33 PM > To: Amit Shah <amit@kernel.org> > Cc: linux-kernel@vger.kernel.org; kvm@vger.kernel.org; x86@kernel.org; linux- > doc@vger.kernel.org; Shah, Amit <Amit.Shah@amd.com>; Lendacky, Thomas > <Thomas.Lendacky@amd.com>; bp@alien8.de; tglx@linutronix.de; > peterz@infradead.org; pawan.kumar.gupta@linux.intel.com; corbet@lwn.net; > mingo@redhat.com; dave.hansen@linux.intel.com; hpa@zytor.com; > seanjc@google.com; pbonzini@redhat.com; daniel.sneddon@linux.intel.com; > kai.huang@intel.com; Das1, Sandipan <Sandipan.Das@amd.com>; > boris.ostrovsky@oracle.com; Moger, Babu <Babu.Moger@amd.com>; Kaplan, > David <David.Kaplan@amd.com>; dwmw@amazon.co.uk; > andrew.cooper3@citrix.com > Subject: Re: [RFC PATCH v2 1/3] x86: cpu/bugs: update SpectreRSB comments > for AMD > > Caution: This message originated from an External Source. Use proper caution > when opening attachments, clicking links, or responding. > > > On Mon, Nov 11, 2024 at 05:39:11PM +0100, Amit Shah wrote: > > From: Amit Shah <amit.shah@amd.com> > > > > AMD CPUs do not fall back to the BTB when the RSB underflows for RET > > address speculation. AMD CPUs have not needed to stuff the RSB for > > underflow conditions. > > > > The RSB poisoning case is addressed by RSB filling - clean up the > > FIXME comment about it. > > I'm thinking the comments need more clarification in light of BTC and SRSO. > > This: > > > - * AMD has it even worse: *all* returns are speculated from the BTB, > > - * regardless of the state of the RSB. > > is still true (mostly: "all" should be "some"), though it doesn't belong in the "RSB > underflow" section. > > Also the RSB stuffing not only mitigates RET, it mitigates any other instruction > which happen to be predicted as a RET. Which is presumably why it's still needed > even when SRSO is enabled. > While that's partly true, I'm not sure I'm understanding which scenario you're concerned with. As noted in the AMD BTC whitepaper, there are various types of potential mis-speculation depending on what the actual branch is. The 'late redirect' ones are the most concerning since those have enough of a speculation window to be able to do a load-op-load gadget. The only 'late redirect' case involving an instruction being incorrectly predicted as a RET is when the actual instruction is an indirect JMP/CALL. But those instructions are either removed entirely (due to retpoline) or being protected with IBRS. The cases of other instructions (like Jcc) being predicted as a RET are subject to the 'early redirect' behavior which is much more limited (and note that these can also be predicted as indirect branches for which there is no special protection). So I'm not sure why BTC specifically would necessitate needing to stuff the RSB here. Also, BTC only affects some AMD parts (not Family 19h and later for instance). --David Kaplan
On Mon, Nov 11, 2024 at 07:58:03PM +0000, Kaplan, David wrote: > > I'm thinking the comments need more clarification in light of BTC and SRSO. > > > > This: > > > > > - * AMD has it even worse: *all* returns are speculated from the BTB, > > > - * regardless of the state of the RSB. > > > > is still true (mostly: "all" should be "some"), though it doesn't belong in the "RSB > > underflow" section. > > > > Also the RSB stuffing not only mitigates RET, it mitigates any other instruction > > which happen to be predicted as a RET. Which is presumably why it's still needed > > even when SRSO is enabled. > > > > While that's partly true, I'm not sure I'm understanding which > scenario you're concerned with. As noted in the AMD BTC whitepaper, > there are various types of potential mis-speculation depending on what > the actual branch is. The 'late redirect' ones are the most concerning > since those have enough of a speculation window to be able to do a > load-op-load gadget. The only 'late redirect' case involving an > instruction being incorrectly predicted as a RET is when the actual > instruction is an indirect JMP/CALL. But those instructions are > either removed entirely (due to retpoline) or being protected with > IBRS. The cases of other instructions (like Jcc) being predicted as a > RET are subject to the 'early redirect' behavior which is much more > limited (and note that these can also be predicted as indirect > branches for which there is no special protection). So I'm not sure > why BTC specifically would necessitate needing to stuff the RSB here. > > Also, BTC only affects some AMD parts (not Family 19h and later for > instance). This is why it's important to spell out all the different cases in the comments. I was attempting to document the justifications for the existing behavior. You make some good points, though backing up a bit, I realize my comment was flawed for another reason: the return thunks only protect the kernel, but RSB filling on context switch is meant to protect user space. So, never mind...
On Mon, Nov 11, 2024 at 12:39:09PM -0800, Josh Poimboeuf wrote: > This is why it's important to spell out all the different cases in the > comments. I was attempting to document the justifications for the > existing behavior. > > You make some good points, though backing up a bit, I realize my comment > was flawed for another reason: the return thunks only protect the > kernel, but RSB filling on context switch is meant to protect user > space. > > So, never mind... That said, I still think the comments need an update. I'll try to come up with something later.
On 11/11/2024 7:33 pm, Josh Poimboeuf wrote: > On Mon, Nov 11, 2024 at 05:39:11PM +0100, Amit Shah wrote: >> From: Amit Shah <amit.shah@amd.com> >> >> AMD CPUs do not fall back to the BTB when the RSB underflows for RET >> address speculation. AMD CPUs have not needed to stuff the RSB for >> underflow conditions. >> >> The RSB poisoning case is addressed by RSB filling - clean up the FIXME >> comment about it. > I'm thinking the comments need more clarification in light of BTC and > SRSO. > > This: > >> - * AMD has it even worse: *all* returns are speculated from the BTB, >> - * regardless of the state of the RSB. > is still true (mostly: "all" should be "some"), though it doesn't belong > in the "RSB underflow" section. > > Also the RSB stuffing not only mitigates RET, it mitigates any other > instruction which happen to be predicted as a RET. Which is presumably > why it's still needed even when SRSO is enabled. > > Something like below? > > diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c > index 47a01d4028f6..e95d3aa14259 100644 > --- a/arch/x86/kernel/cpu/bugs.c > +++ b/arch/x86/kernel/cpu/bugs.c > @@ -1828,9 +1828,6 @@ static void __init spectre_v2_select_mitigation(void) > * speculated return targets may come from the branch predictor, > * which could have a user-poisoned BTB or BHB entry. > * > - * AMD has it even worse: *all* returns are speculated from the BTB, > - * regardless of the state of the RSB. > - * > * When IBRS or eIBRS is enabled, the "user -> kernel" attack > * scenario is mitigated by the IBRS branch prediction isolation > * properties, so the RSB buffer filling wouldn't be necessary to > @@ -1850,10 +1847,22 @@ static void __init spectre_v2_select_mitigation(void) > * The "user -> user" scenario, also known as SpectreBHB, requires > * RSB clearing. > * > + * AMD Branch Type Confusion (aka "AMD retbleed") adds some > + * additional wrinkles: > + * > + * - A RET can be mispredicted as a direct or indirect branch, > + * causing the CPU to speculatively branch to a BTB target, in > + * which case the RSB filling obviously doesn't help. That case > + * is mitigated by removing all the RETs (SRSO mitigation). > + * > + * - The RSB is not only used for architectural RET instructions, > + * it may also be used for other instructions which happen to > + * get mispredicted as RETs. Therefore RSB filling is still > + * needed even when the RETs have all been removed by the SRSO > + * mitigation. This is my take. On AMD CPUs, there are two unrelated issues to take into account: 1) SRSO Affects anything which doesn't enumerate SRSO_NO, which is all parts to date including Zen5. SRSO ends up overflowing the RAS with arbitrary BTB targets, such that a subsequent genuine RET follows a prediction which never came from a real CALL instruction. Mitigations for SRSO are either safe-ret, or IBPB-on-entry. Parts without IBPB_RET using IBPB-on-entry need to manually flush the RAS. Importantly, SMEP does not protection you against SRSO across the user->kernel boundary, because the bad RAS entries are arbitrary. New in Zen5 is the SRSO_U/S_NO bit which says this case can't occur any more. So on Zen5, you can in principle get away without a RAS flush on entry. 2) BTC Affects anything which doesn't enumerate BTC_NO, which is Zen2 and older (Fam17h for AMD, Fam18h for Hygon). Attacker can forge any branch type prediction, and the most dangerous one is RET-mispredicted-as-INDIRECT. This causes a genuine RET instruction to follow a prediction that was believed to be an indirect branch. All CPUs which suffer BTC also suffer SRSO, so while jmp2ret is a mitigation for BTC, it's utility became 0 when SRSO was discovered. (Which as shame, because it's equal parts beautiful and terrifying.) Mitigations for BTC are therefore safe-ret or IBPB-on-entry. Flushing the RAS has no effect on BTC, because the whole problem with BTC is that the prediction comes from the "wrong" predictor, but you need to do it for other reasons. ~Andrew
On Mon, Nov 11, 2024 at 12:40:41PM -0800, Josh Poimboeuf wrote: > On Mon, Nov 11, 2024 at 12:39:09PM -0800, Josh Poimboeuf wrote: > > This is why it's important to spell out all the different cases in the > > comments. I was attempting to document the justifications for the > > existing behavior. > > > > You make some good points, though backing up a bit, I realize my comment > > was flawed for another reason: the return thunks only protect the > > kernel, but RSB filling on context switch is meant to protect user > > space. > > > > So, never mind... > > That said, I still think the comments need an update. I'll try to come > up with something later. Here are some clarifications to the comments. Amit, feel free to include this in your next revision. ----8<---- From: Josh Poimboeuf <jpoimboe@kernel.org> Subject: [PATCH] x86/bugs: Update insanely long comment about RSB attacks The long comment above the setting of X86_FEATURE_RSB_CTXSW is a bit confusing. It starts out being about context switching specifically, but then goes on to describe "user -> kernel" mitigations, which aren't necessarily limited to context switches. Clarify that it's about *all* RSB attacks and their mitigations. For consistency, add the "guest -> host" mitigations as well. Then the comment above spectre_v2_determine_rsb_fill_type_at_vmexit() can be removed and the overall line count is reduced. Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org> --- arch/x86/kernel/cpu/bugs.c | 59 ++++++++++++-------------------------- 1 file changed, 19 insertions(+), 40 deletions(-) diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c index 47a01d4028f6..fbdfa151b7a9 100644 --- a/arch/x86/kernel/cpu/bugs.c +++ b/arch/x86/kernel/cpu/bugs.c @@ -1581,26 +1581,6 @@ static void __init spec_ctrl_disable_kernel_rrsba(void) static void __init spectre_v2_determine_rsb_fill_type_at_vmexit(enum spectre_v2_mitigation mode) { - /* - * Similar to context switches, there are two types of RSB attacks - * after VM exit: - * - * 1) RSB underflow - * - * 2) Poisoned RSB entry - * - * When retpoline is enabled, both are mitigated by filling/clearing - * the RSB. - * - * When IBRS is enabled, while #1 would be mitigated by the IBRS branch - * prediction isolation protections, RSB still needs to be cleared - * because of #2. Note that SMEP provides no protection here, unlike - * user-space-poisoned RSB entries. - * - * eIBRS should protect against RSB poisoning, but if the EIBRS_PBRSB - * bug is present then a LITE version of RSB protection is required, - * just a single call needs to retire before a RET is executed. - */ switch (mode) { case SPECTRE_V2_NONE: return; @@ -1818,43 +1798,42 @@ static void __init spectre_v2_select_mitigation(void) pr_info("%s\n", spectre_v2_strings[mode]); /* - * If Spectre v2 protection has been enabled, fill the RSB during a - * context switch. In general there are two types of RSB attacks - * across context switches, for which the CALLs/RETs may be unbalanced. + * In general there are two types of RSB attacks: * - * 1) RSB underflow + * 1) RSB underflow ("Intel Retbleed") * * Some Intel parts have "bottomless RSB". When the RSB is empty, * speculated return targets may come from the branch predictor, * which could have a user-poisoned BTB or BHB entry. * - * AMD has it even worse: *all* returns are speculated from the BTB, - * regardless of the state of the RSB. + * When IBRS or eIBRS is enabled, the "user -> kernel" attack is + * mitigated by the IBRS branch prediction isolation properties, so + * the RSB buffer filling wouldn't be necessary to protect against + * this type of attack. * - * When IBRS or eIBRS is enabled, the "user -> kernel" attack - * scenario is mitigated by the IBRS branch prediction isolation - * properties, so the RSB buffer filling wouldn't be necessary to - * protect against this type of attack. + * The "user -> user" attack is mitigated by RSB filling on context + * switch. * - * The "user -> user" attack scenario is mitigated by RSB filling. + * The "guest -> host" attack is mitigated by IBRS or eIBRS. * * 2) Poisoned RSB entry * * If the 'next' in-kernel return stack is shorter than 'prev', * 'next' could be tricked into speculating with a user-poisoned RSB - * entry. + * entry. Speculative Type Confusion ("AMD retbleed") can also + * create poisoned RSB entries. * - * The "user -> kernel" attack scenario is mitigated by SMEP and - * eIBRS. + * The "user -> kernel" attack is mitigated by SMEP and eIBRS. * - * The "user -> user" scenario, also known as SpectreBHB, requires - * RSB clearing. + * The "user -> user" attack, also known as SpectreBHB, requires RSB + * clearing. * - * So to mitigate all cases, unconditionally fill RSB on context - * switches. - * - * FIXME: Is this pointless for retbleed-affected AMD? + * The "guest -> host" attack is mitigated by eIBRS (not IBRS!) or + * RSB clearing on vmexit. Note that eIBRS implementations with + * X86_BUG_EIBRS_PBRSB still need "lite" RSB clearing which retires + * a single CALL before the first RET. */ + setup_force_cpu_cap(X86_FEATURE_RSB_CTXSW); pr_info("Spectre v2 / SpectreRSB mitigation: Filling RSB on context switch\n");
On Tue, Nov 12, 2024 at 12:29:28AM +0000, Andrew Cooper wrote: > This is my take. On AMD CPUs, there are two unrelated issues to take > into account: > > 1) SRSO > > Affects anything which doesn't enumerate SRSO_NO, which is all parts to > date including Zen5. > > SRSO ends up overflowing the RAS with arbitrary BTB targets, such that a > subsequent genuine RET follows a prediction which never came from a real > CALL instruction. > > Mitigations for SRSO are either safe-ret, or IBPB-on-entry. Parts > without IBPB_RET using IBPB-on-entry need to manually flush the RAS. > > Importantly, SMEP does not protection you against SRSO across the > user->kernel boundary, because the bad RAS entries are arbitrary. New > in Zen5 is the SRSO_U/S_NO bit which says this case can't occur any > more. So on Zen5, you can in principle get away without a RAS flush on > entry. Updated to mention SRSO: /* * In general there are two types of RSB attacks: * * 1) RSB underflow ("Intel Retbleed") * * Some Intel parts have "bottomless RSB". When the RSB is empty, * speculated return targets may come from the branch predictor, * which could have a user-poisoned BTB or BHB entry. * * When IBRS or eIBRS is enabled, the "user -> kernel" attack is * mitigated by the IBRS branch prediction isolation properties, so * the RSB buffer filling wouldn't be necessary to protect against * this type of attack. * * The "user -> user" attack is mitigated by RSB filling on context * switch. * * The "guest -> host" attack is mitigated by IBRS or eIBRS. * * 2) Poisoned RSB entry * * If the 'next' in-kernel return stack is shorter than 'prev', * 'next' could be tricked into speculating with a user-poisoned RSB * entry. Poisoned RSB entries can also be created by Branch Type * Confusion ("AMD retbleed") or SRSO. * * The "user -> kernel" attack is mitigated by SMEP and eIBRS. AMD * without SRSO_NO also needs the SRSO mitigation. * * The "user -> user" attack, also known as SpectreBHB, requires RSB * clearing. * * The "guest -> host" attack is mitigated by either eIBRS (not * IBRS!) or RSB clearing on vmexit. Note that eIBRS * implementations with X86_BUG_EIBRS_PBRSB still need "lite" RSB * clearing which retires a single CALL before the first RET. */ ---8<--- From: Josh Poimboeuf <jpoimboe@kernel.org> Subject: [PATCH] x86/bugs: Update insanely long comment about RSB attacks The long comment above the setting of X86_FEATURE_RSB_CTXSW is a bit confusing. It starts out being about context switching specifically, but then goes on to describe "user -> kernel" mitigations, which aren't necessarily limited to context switches. Clarify that it's about *all* RSB attacks and their mitigations. For consistency, add the "guest -> host" mitigations as well. Then the comment above spectre_v2_determine_rsb_fill_type_at_vmexit() can be removed and the overall line count is reduced. Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org> --- arch/x86/kernel/cpu/bugs.c | 60 +++++++++++++------------------------- 1 file changed, 20 insertions(+), 40 deletions(-) diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c index 47a01d4028f6..3dd1e504d706 100644 --- a/arch/x86/kernel/cpu/bugs.c +++ b/arch/x86/kernel/cpu/bugs.c @@ -1581,26 +1581,6 @@ static void __init spec_ctrl_disable_kernel_rrsba(void) static void __init spectre_v2_determine_rsb_fill_type_at_vmexit(enum spectre_v2_mitigation mode) { - /* - * Similar to context switches, there are two types of RSB attacks - * after VM exit: - * - * 1) RSB underflow - * - * 2) Poisoned RSB entry - * - * When retpoline is enabled, both are mitigated by filling/clearing - * the RSB. - * - * When IBRS is enabled, while #1 would be mitigated by the IBRS branch - * prediction isolation protections, RSB still needs to be cleared - * because of #2. Note that SMEP provides no protection here, unlike - * user-space-poisoned RSB entries. - * - * eIBRS should protect against RSB poisoning, but if the EIBRS_PBRSB - * bug is present then a LITE version of RSB protection is required, - * just a single call needs to retire before a RET is executed. - */ switch (mode) { case SPECTRE_V2_NONE: return; @@ -1818,43 +1798,43 @@ static void __init spectre_v2_select_mitigation(void) pr_info("%s\n", spectre_v2_strings[mode]); /* - * If Spectre v2 protection has been enabled, fill the RSB during a - * context switch. In general there are two types of RSB attacks - * across context switches, for which the CALLs/RETs may be unbalanced. + * In general there are two types of RSB attacks: * - * 1) RSB underflow + * 1) RSB underflow ("Intel Retbleed") * * Some Intel parts have "bottomless RSB". When the RSB is empty, * speculated return targets may come from the branch predictor, * which could have a user-poisoned BTB or BHB entry. * - * AMD has it even worse: *all* returns are speculated from the BTB, - * regardless of the state of the RSB. + * When IBRS or eIBRS is enabled, the "user -> kernel" attack is + * mitigated by the IBRS branch prediction isolation properties, so + * the RSB buffer filling wouldn't be necessary to protect against + * this type of attack. * - * When IBRS or eIBRS is enabled, the "user -> kernel" attack - * scenario is mitigated by the IBRS branch prediction isolation - * properties, so the RSB buffer filling wouldn't be necessary to - * protect against this type of attack. + * The "user -> user" attack is mitigated by RSB filling on context + * switch. * - * The "user -> user" attack scenario is mitigated by RSB filling. + * The "guest -> host" attack is mitigated by IBRS or eIBRS. * * 2) Poisoned RSB entry * * If the 'next' in-kernel return stack is shorter than 'prev', * 'next' could be tricked into speculating with a user-poisoned RSB - * entry. + * entry. Poisoned RSB entries can also be created by Branch Type + * Confusion ("AMD retbleed") or SRSO. * - * The "user -> kernel" attack scenario is mitigated by SMEP and - * eIBRS. + * The "user -> kernel" attack is mitigated by SMEP and eIBRS. AMD + * without SRSO_NO also needs the SRSO mitigation. * - * The "user -> user" scenario, also known as SpectreBHB, requires - * RSB clearing. + * The "user -> user" attack, also known as SpectreBHB, requires RSB + * clearing. * - * So to mitigate all cases, unconditionally fill RSB on context - * switches. - * - * FIXME: Is this pointless for retbleed-affected AMD? + * The "guest -> host" attack is mitigated by either eIBRS (not + * IBRS!) or RSB clearing on vmexit. Note that eIBRS + * implementations with X86_BUG_EIBRS_PBRSB still need "lite" RSB + * clearing which retires a single CALL before the first RET. */ + setup_force_cpu_cap(X86_FEATURE_RSB_CTXSW); pr_info("Spectre v2 / SpectreRSB mitigation: Filling RSB on context switch\n");
On Mon, Nov 11, 2024 at 05:46:44PM -0800, Josh Poimboeuf wrote:
> Subject: [PATCH] x86/bugs: Update insanely long comment about RSB attacks
Why don't you stick this insanely long comment in
Documentation/admin-guide/hw-vuln/ while at it?
Its place is hardly in the code. You can point to it from the code tho...
On Mon, 2024-11-11 at 17:46 -0800, Josh Poimboeuf wrote: > On Tue, Nov 12, 2024 at 12:29:28AM +0000, Andrew Cooper wrote: > > This is my take. On AMD CPUs, there are two unrelated issues to > > take > > into account: > > > > 1) SRSO > > > > Affects anything which doesn't enumerate SRSO_NO, which is all > > parts to > > date including Zen5. > > > > SRSO ends up overflowing the RAS with arbitrary BTB targets, such > > that a > > subsequent genuine RET follows a prediction which never came from a > > real > > CALL instruction. > > > > Mitigations for SRSO are either safe-ret, or IBPB-on-entry. Parts > > without IBPB_RET using IBPB-on-entry need to manually flush the > > RAS. > > > > Importantly, SMEP does not protection you against SRSO across the > > user->kernel boundary, because the bad RAS entries are arbitrary. > > New > > in Zen5 is the SRSO_U/S_NO bit which says this case can't occur any > > more. So on Zen5, you can in principle get away without a RAS > > flush on > > entry. > > Updated to mention SRSO: > > /* > * In general there are two types of RSB attacks: > * > * 1) RSB underflow ("Intel Retbleed") > * > * Some Intel parts have "bottomless RSB". When the RSB > is empty, > * speculated return targets may come from the branch > predictor, > * which could have a user-poisoned BTB or BHB entry. > * > * When IBRS or eIBRS is enabled, the "user -> kernel" > attack is > * mitigated by the IBRS branch prediction isolation > properties, so > * the RSB buffer filling wouldn't be necessary to > protect against > * this type of attack. > * > * The "user -> user" attack is mitigated by RSB filling > on context > * switch. > * > * The "guest -> host" attack is mitigated by IBRS or > eIBRS. > * > * 2) Poisoned RSB entry > * > * If the 'next' in-kernel return stack is shorter than > 'prev', > * 'next' could be tricked into speculating with a user- > poisoned RSB > * entry. Poisoned RSB entries can also be created by > Branch Type > * Confusion ("AMD retbleed") or SRSO. > * > * The "user -> kernel" attack is mitigated by SMEP and > eIBRS. AMD > * without SRSO_NO also needs the SRSO mitigation. > * > * The "user -> user" attack, also known as SpectreBHB, > requires RSB > * clearing. > * > * The "guest -> host" attack is mitigated by either > eIBRS (not > * IBRS!) or RSB clearing on vmexit. Note that eIBRS > * implementations with X86_BUG_EIBRS_PBRSB still need > "lite" RSB > * clearing which retires a single CALL before the first > RET. > */ Thanks, Josh and Andrew. This reads well to me. In the context of ERAPS, I'll end up adding a couple more sentences there as well. Amit
On Mon, Nov 11, 2024 at 05:46:44PM -0800, Josh Poimboeuf wrote: > + * 1) RSB underflow ("Intel Retbleed") > * > * Some Intel parts have "bottomless RSB". When the RSB is empty, > * speculated return targets may come from the branch predictor, > * which could have a user-poisoned BTB or BHB entry. > * > - * AMD has it even worse: *all* returns are speculated from the BTB, > - * regardless of the state of the RSB. > + * When IBRS or eIBRS is enabled, the "user -> kernel" attack is > + * mitigated by the IBRS branch prediction isolation properties, so > + * the RSB buffer filling wouldn't be necessary to protect against > + * this type of attack. > * > - * When IBRS or eIBRS is enabled, the "user -> kernel" attack > - * scenario is mitigated by the IBRS branch prediction isolation > - * properties, so the RSB buffer filling wouldn't be necessary to > - * protect against this type of attack. > + * The "user -> user" attack is mitigated by RSB filling on context > + * switch. user->user SpectreRSB is also mitigated by IBPB, so RSB filling is unnecessary when IBPB is issued. Also, when an appication does not opted-in for IBPB at context switch, spectre-v2 for that app is not mitigated, filling RSB is only a half measure in that case. Is RSB filling really serving any purpose for userspace?
On Tue, Nov 12, 2024 at 01:43:48PM -0800, Pawan Gupta wrote: > On Mon, Nov 11, 2024 at 05:46:44PM -0800, Josh Poimboeuf wrote: > > + * 1) RSB underflow ("Intel Retbleed") > > * > > * Some Intel parts have "bottomless RSB". When the RSB is empty, > > * speculated return targets may come from the branch predictor, > > * which could have a user-poisoned BTB or BHB entry. > > * > > - * AMD has it even worse: *all* returns are speculated from the BTB, > > - * regardless of the state of the RSB. > > + * When IBRS or eIBRS is enabled, the "user -> kernel" attack is > > + * mitigated by the IBRS branch prediction isolation properties, so > > + * the RSB buffer filling wouldn't be necessary to protect against > > + * this type of attack. > > * > > - * When IBRS or eIBRS is enabled, the "user -> kernel" attack > > - * scenario is mitigated by the IBRS branch prediction isolation > > - * properties, so the RSB buffer filling wouldn't be necessary to > > - * protect against this type of attack. > > + * The "user -> user" attack is mitigated by RSB filling on context > > + * switch. > > user->user SpectreRSB is also mitigated by IBPB, so RSB filling is > unnecessary when IBPB is issued. Also, when an appication does not opted-in > for IBPB at context switch, spectre-v2 for that app is not mitigated, > filling RSB is only a half measure in that case. > > Is RSB filling really serving any purpose for userspace? Indeed... If we don't need to flush RSB for user->user, we'd only need to worry about protecting the kernel. Something like so? - eIBRS+!PBRSB: no flush - eIBRS+PBRSB: lite flush - everything else: full flush i.e., same logic as spectre_v2_determine_rsb_fill_type_at_vmexit(), but also for context switches.
On Tue, Nov 12, 2024 at 12:58:11PM +0100, Borislav Petkov wrote: > On Mon, Nov 11, 2024 at 05:46:44PM -0800, Josh Poimboeuf wrote: > > Subject: [PATCH] x86/bugs: Update insanely long comment about RSB attacks > > Why don't you stick this insanely long comment in > Documentation/admin-guide/hw-vuln/ while at it? > > Its place is hardly in the code. You can point to it from the code tho... There are a lot of subtle details to this $#!tstorm, and IMO we probably wouldn't be having these discussions in the first place if the comment lived in the docs, as most people seem to ignore them...
On Wed, Nov 13, 2024 at 01:24:40PM -0800, Josh Poimboeuf wrote: > There are a lot of subtle details to this $#!tstorm, and IMO we probably > wouldn't be having these discussions in the first place if the comment > lived in the docs, as most people seem to ignore them... That's why I'm saying point to the docs from the code. You can't have a big fat comment in the code about this but everything else in the hw-vuln docs.
On Wed, Nov 13, 2024 at 10:37:24PM +0100, Borislav Petkov wrote: > On Wed, Nov 13, 2024 at 01:24:40PM -0800, Josh Poimboeuf wrote: > > There are a lot of subtle details to this $#!tstorm, and IMO we probably > > wouldn't be having these discussions in the first place if the comment > > lived in the docs, as most people seem to ignore them... > > That's why I'm saying point to the docs from the code. You can't have a big > fat comment in the code about this but everything else in the hw-vuln docs. But those docs are user facing, describing the "what" for each vulnerability individually. They're basically historical documents which don't evolve over time unless we tweak an interface or add a new mitigation. This comment relates to the "why" for the code itself (and its poor confused developers), taking all the RSB-related vulnerabilities into account.
On Wed, Nov 13, 2024 at 12:21:05PM -0800, Josh Poimboeuf wrote: > On Tue, Nov 12, 2024 at 01:43:48PM -0800, Pawan Gupta wrote: > > On Mon, Nov 11, 2024 at 05:46:44PM -0800, Josh Poimboeuf wrote: > > > + * 1) RSB underflow ("Intel Retbleed") > > > * > > > * Some Intel parts have "bottomless RSB". When the RSB is empty, > > > * speculated return targets may come from the branch predictor, > > > * which could have a user-poisoned BTB or BHB entry. > > > * > > > - * AMD has it even worse: *all* returns are speculated from the BTB, > > > - * regardless of the state of the RSB. > > > + * When IBRS or eIBRS is enabled, the "user -> kernel" attack is > > > + * mitigated by the IBRS branch prediction isolation properties, so > > > + * the RSB buffer filling wouldn't be necessary to protect against > > > + * this type of attack. > > > * > > > - * When IBRS or eIBRS is enabled, the "user -> kernel" attack > > > - * scenario is mitigated by the IBRS branch prediction isolation > > > - * properties, so the RSB buffer filling wouldn't be necessary to > > > - * protect against this type of attack. > > > + * The "user -> user" attack is mitigated by RSB filling on context > > > + * switch. > > > > user->user SpectreRSB is also mitigated by IBPB, so RSB filling is > > unnecessary when IBPB is issued. Also, when an appication does not opted-in > > for IBPB at context switch, spectre-v2 for that app is not mitigated, > > filling RSB is only a half measure in that case. > > > > Is RSB filling really serving any purpose for userspace? > > Indeed... > > If we don't need to flush RSB for user->user, we'd only need to worry > about protecting the kernel. Something like so? > > - eIBRS+!PBRSB: no flush > - eIBRS+PBRSB: lite flush Yes for VMexit, but not at kernel entry. PBRSB requires an unbalanced RET, and it is only a problem until the first retired CALL. At VMexit we do have unbalanced RET but not at kernel entry. > - everything else: full flush > i.e., same logic as spectre_v2_determine_rsb_fill_type_at_vmexit(), but > also for context switches. Yes, assuming you mean user->kernel switch, and not process context switch.
On Wed, Nov 13, 2024 at 05:55:05PM -0800, Pawan Gupta wrote: > > > user->user SpectreRSB is also mitigated by IBPB, so RSB filling is > > > unnecessary when IBPB is issued. Also, when an appication does not opted-in > > > for IBPB at context switch, spectre-v2 for that app is not mitigated, > > > filling RSB is only a half measure in that case. > > > > > > Is RSB filling really serving any purpose for userspace? > > > > Indeed... > > > > If we don't need to flush RSB for user->user, we'd only need to worry > > about protecting the kernel. Something like so? > > > > - eIBRS+!PBRSB: no flush > > - eIBRS+PBRSB: lite flush > > Yes for VMexit, but not at kernel entry. PBRSB requires an unbalanced RET, > and it is only a problem until the first retired CALL. At VMexit we do have > unbalanced RET but not at kernel entry. > > > - everything else: full flush > > > i.e., same logic as spectre_v2_determine_rsb_fill_type_at_vmexit(), but > > also for context switches. > > Yes, assuming you mean user->kernel switch, and not process context switch. Actually I did mean context switch. AFAIK we don't need to flush RSB at kernel entry. If user->user RSB is already mitigated by IBPB, then at context switch we only have to worry about user->kernel. e.g., if 'next' has more (in kernel) RETs then 'prev' had (in kernel) CALLs, the user could trigger RSB underflow or corruption inside the kernel after the context switch. Doesn't eIBRS already protect against that? For PBRSB, I guess we don't need to worry about that since there would be at least one kernel CALL before context switch.
On Wed, Nov 13, 2024 at 04:43:58PM -0800, Josh Poimboeuf wrote: > This comment relates to the "why" for the code itself (and its poor > confused developers), taking all the RSB-related vulnerabilities into > account. So use Documentation/arch/x86/. This is exactly the reason why we need more "why" documentation - because everytime we have to swap the whole bugs.c horror back in, we're poor confused developers. And we have the "why" spread out across commit messages and other folklore which means everytime we have to change stuff, the git archeology starts. :-\ "err, do you remember why we're doing this?!" And so on converstaions on IRC. So having an implementation document explaining clearly why we did things is long overdue. But it's fine - I can move it later when the dust settles here. Thx.
On Wed, Nov 13, 2024 at 06:31:41PM -0800, Josh Poimboeuf wrote: > On Wed, Nov 13, 2024 at 05:55:05PM -0800, Pawan Gupta wrote: > > > > user->user SpectreRSB is also mitigated by IBPB, so RSB filling is > > > > unnecessary when IBPB is issued. Also, when an appication does not opted-in > > > > for IBPB at context switch, spectre-v2 for that app is not mitigated, > > > > filling RSB is only a half measure in that case. > > > > > > > > Is RSB filling really serving any purpose for userspace? > > > > > > Indeed... > > > > > > If we don't need to flush RSB for user->user, we'd only need to worry > > > about protecting the kernel. Something like so? > > > > > > - eIBRS+!PBRSB: no flush > > > - eIBRS+PBRSB: lite flush > > > > Yes for VMexit, but not at kernel entry. PBRSB requires an unbalanced RET, > > and it is only a problem until the first retired CALL. At VMexit we do have > > unbalanced RET but not at kernel entry. > > > > > - everything else: full flush > > > > > i.e., same logic as spectre_v2_determine_rsb_fill_type_at_vmexit(), but > > > also for context switches. > > > > Yes, assuming you mean user->kernel switch, and not process context switch. > > Actually I did mean context switch. AFAIK we don't need to flush RSB at > kernel entry. > > If user->user RSB is already mitigated by IBPB, then at context switch > we only have to worry about user->kernel. e.g., if 'next' has more (in > kernel) RETs then 'prev' had (in kernel) CALLs, the user could trigger > RSB underflow or corruption inside the kernel after the context switch. Yes, this condition can cause RSB underflow, but that is not enough. More importantly an attacker also needs to control the target of RET. > Doesn't eIBRS already protect against that? Yes, eIBRS does protect against that, because the alternate predictor (TA) is isolated by eIBRS from user influence. > For PBRSB, I guess we don't need to worry about that since there would > be at least one kernel CALL before context switch. Right. So the case where we need RSB filling at context switch is retpoline+CDT mitigation.
* Borislav Petkov <bp@alien8.de> wrote: > On Wed, Nov 13, 2024 at 04:43:58PM -0800, Josh Poimboeuf wrote: > > This comment relates to the "why" for the code itself (and its poor > > confused developers), taking all the RSB-related vulnerabilities into > > account. > > So use Documentation/arch/x86/. > > This is exactly the reason why we need more "why" documentation - because > everytime we have to swap the whole bugs.c horror back in, we're poor confused > developers. And we have the "why" spread out across commit messages and other > folklore which means everytime we have to change stuff, the git archeology > starts. :-\ "err, do you remember why we're doing this?!" And so on > converstaions on IRC. > > So having an implementation document explaining clearly why we did things is > long overdue. > > But it's fine - I can move it later when the dust settles here. I think in-line documentation is better in this case: the primary defense against mistakes and misunderstandings is in the source code itself. And "it's too long" is an argument *against* moving it out into some obscure place 99% of developers aren't even aware of... Thanks, Ingo
On Thu, Nov 14, 2024 at 10:47:23AM +0100, Ingo Molnar wrote: > I think in-line documentation is better in this case: the primary defense > against mistakes and misunderstandings is in the source code itself. > > And "it's too long" is an argument *against* moving it out into some obscure > place 99% of developers aren't even aware of... You mean developers can't even read? /* * See Documentation/arch/x86/ for details on this mitigation * implementation. */ And if we want to expand the "why" and do proper documentation on the implementation decisions of each mitigation, we still keep it there in the code? Or we do one part in Documentation/ and another part in the code?
diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c index 47a01d4028f6..0aa629b5537d 100644 --- a/arch/x86/kernel/cpu/bugs.c +++ b/arch/x86/kernel/cpu/bugs.c @@ -1828,9 +1828,6 @@ static void __init spectre_v2_select_mitigation(void) * speculated return targets may come from the branch predictor, * which could have a user-poisoned BTB or BHB entry. * - * AMD has it even worse: *all* returns are speculated from the BTB, - * regardless of the state of the RSB. - * * When IBRS or eIBRS is enabled, the "user -> kernel" attack * scenario is mitigated by the IBRS branch prediction isolation * properties, so the RSB buffer filling wouldn't be necessary to @@ -1852,8 +1849,6 @@ static void __init spectre_v2_select_mitigation(void) * * So to mitigate all cases, unconditionally fill RSB on context * switches. - * - * FIXME: Is this pointless for retbleed-affected AMD? */ setup_force_cpu_cap(X86_FEATURE_RSB_CTXSW); pr_info("Spectre v2 / SpectreRSB mitigation: Filling RSB on context switch\n");