RE: [PATCH 2/2] x86/bugs: Don't WARN() when overwriting retbleed_return_thunk with srso_return_thunk

[Kaplan, David]

[AMD Official Use Only - AMD Internal Distribution Only]

> -----Original Message-----
> From: Borislav Petkov <bp@xxxxxxxxx>
> Sent: Friday, May 16, 2025 2:48 AM
> To: Suraj Jitindar Singh <surajjs@xxxxxxxxxx>; Kaplan, David
> <David.Kaplan@xxxxxxx>
> Cc: linux-kernel@xxxxxxxxxxxxxxx; x86@xxxxxxxxxx; Thomas Gleixner
> <tglx@xxxxxxxxxxxxx>; Peter Zijlstra <peterz@xxxxxxxxxxxxx>; Josh Poimboeuf
> <jpoimboe@xxxxxxxxxx>; Pawan Gupta <pawan.kumar.gupta@xxxxxxxxxxxxxxx>;
> Ingo Molnar <mingo@xxxxxxxxxx>; Dave Hansen
> <dave.hansen@xxxxxxxxxxxxxxx>; stable@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH 2/2] x86/bugs: Don't WARN() when overwriting
> retbleed_return_thunk with srso_return_thunk
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> On Thu, May 15, 2025 at 04:34:33PM -0700, Suraj Jitindar Singh wrote:
> > -     WARN(x86_return_thunk != __x86_return_thunk,
> > +     WARN((x86_return_thunk != __x86_return_thunk) &&
> > +          (thunk != srso_return_thunk ||
> > +           x86_return_thunk != retbleed_return_thunk),
> >            "x86/bugs: return thunk changed from %ps to %ps\n",
> >            x86_return_thunk, thunk);
>
> This is still adding that nasty conditional which I'd like to avoid.
>
> And I just had this other idea: we're switching to select/update/apply logic with the
> mitigations and I'm sure we can use that new ability to select the proper mitigation
> when other mitigations are influencing the decision, to select the proper return
> thunk.
>
> I'm thinking for retbleed and SRSO we could set it only once, perhaps in
> srso_select_mitigation() as it runs last.
>
> I don't want to introduce an amd_return_thunk... :-)
>
> But David might have a better idea...
>

Hmm.  Since SRSO is kind of a superset of retbleed, it might make sense to create a new mitigation, RETBLEED_MITIGATION_SAFE_RET.

retbleed_update_mitigation() can change its mitigation to this if srso_mitigation is SAFE_RET (or SAFE_RET_UCODE_NEEDED).  RETBLEED_MITIGATION_SAFE_RET can do nothing in retbleed_apply_mitigation() because it means that srso is taking care of things.  Thoughts?

This also made me realize there's another minor missing interaction here, which is that if spec_rstack_overflow=ibpb, then that should set retbleed_mitigation to IBPB as well.

--David Kaplan