On 5/7/21 8:53 AM, Jon Kohler wrote:
> The only place X86_FEATURE_RSB_CTXSW is currently in use is in
> arch/x86/entry/entry_{32|64}.S, where its use is wrapped with
> ifdef CONFIG_RETPOLINE. If someone uses a system with
> X86_FEATURE_IBRS_ENHANCED and compiles without CONFIG_RETPOLINE
> but still has spectre v2 set to auto, the kernel log will
> print that eIBRS is enabled and that RSB stuffing is enabled;
> however, that stuffing would never occur.
>
> To make this behavior more clear, wrap the enablement of
> X86_FEATURE_RSB_CTXSW and the resulting log message with ifdef
> CONFIG_RETPOLINE, such that it is compiled out along with the
> actions it controls.
>
> This way seems more correct at first glance as this was the way
> the code was originally written in fdf82a7856b; however, when
> enhanced IBRS was added, there was a goto added under
> SPECTRE_V2_CMD_AUTO which bypasses going through retpoline_auto,
> where X86_FEATURE_RETPOLINE is set.
>
> The other option would be to remove the CONFIG_RETPOLINE from
> the code in entry_{32|64}.S, such that it would always be
> compiled no matter what, such that these two areas match.
This kinda dances around the real issue: Does RSB stuffing have
mitigation value on enhanced IBRS systems?
If yes, then we should make the RSB stuffing code in entry*.S available
separately from CONFIG_RETPOLINE.
If no, is it because eIBRS systems are not vulnerable, or because RSB
stuffing has no mitigation value?
Either way, I'm not sure the approach in this patch is the one we want.
> On May 7, 2021, at 12:02 PM, Dave Hansen <[email protected]> wrote:
>
> On 5/7/21 8:53 AM, Jon Kohler wrote:
>> The only place X86_FEATURE_RSB_CTXSW is currently in use is in
>> arch/x86/entry/entry_{32|64}.S, where its use is wrapped with
>> ifdef CONFIG_RETPOLINE. If someone uses a system with
>> X86_FEATURE_IBRS_ENHANCED and compiles without CONFIG_RETPOLINE
>> but still has spectre v2 set to auto, the kernel log will
>> print that eIBRS is enabled and that RSB stuffing is enabled;
>> however, that stuffing would never occur.
>>
>> To make this behavior more clear, wrap the enablement of
>> X86_FEATURE_RSB_CTXSW and the resulting log message with ifdef
>> CONFIG_RETPOLINE, such that it is compiled out along with the
>> actions it controls.
>>
>> This way seems more correct at first glance as this was the way
>> the code was originally written in fdf82a7856b; however, when
>> enhanced IBRS was added, there was a goto added under
>> SPECTRE_V2_CMD_AUTO which bypasses going through retpoline_auto,
>> where X86_FEATURE_RETPOLINE is set.
>>
>> The other option would be to remove the CONFIG_RETPOLINE from
>> the code in entry_{32|64}.S, such that it would always be
>> compiled no matter what, such that these two areas match.
>
> This kinda dances around the real issue: Does RSB stuffing have
> mitigation value on enhanced IBRS systems?
>
> If yes, then we should make the RSB stuffing code in entry*.S available
> separately from CONFIG_RETPOLINE.
>
> If no, is it because eIBRS systems are not vulnerable, or because RSB
> stuffing has no mitigation value?
>
> Either way, I'm not sure the approach in this patch is the one we want.
Thanks, Dave, for the prompt response and thats a great question.
The way the existing documentation reads in bugs.c, all Sklake+
processors are affected by this, so I'd lean towards "yes" to
your question; however, would love the authorative view from Intel
guru's on this, as RSB stuffing is a bit expensive, would be lovely
to drop it on eIBRS systems! :)
In running down the idea for this patch, I did look at Intel's
"Affected Processors: Transient Execution Attacks & Related Security
Issues by CPU" guide, interestingly enough the CVE for spectre RSB
(v5 / 2018-15572) isn't listed, so I had to depend on what was written
in the various previous commits around this topic.
Anyhow, happy to take up a different approach depending on what the
powers that be at Intel say to needing RSB stuffing on eIBRS. In
addition to the chatter here, I'll see if I can poke around more on
my own as well.
Thanks again,
Jon