On 02/12/2018 11:55 PM, Ingo Molnar wrote:
>
> * Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote:
>
>> On Mon, Feb 12, 2018 at 08:13:31AM -0800, Dave Hansen wrote:
>>> On 02/12/2018 02:22 AM, Ingo Molnar wrote:
>>>>> +static inline void firmware_restrict_branch_speculation_end(void)
>>>>> +{
>>>>> + alternative_msr_write(MSR_IA32_SPEC_CTRL, 0,
>>>>> + X86_FEATURE_USE_IBRS_FW);
>>>> BTW., there's a detail that only occurred to me today, this enabling/disabling
>>>> sequence is not NMI safe, and it might be called from NMI context:
>>>
>>> FWIW, Tim Chen and I talked about this a bunch. We ended up just
>>> saving/restoring the MSR verbatim in the NMI handler the same way we do
>>> CR3, stashing it in a high general-purpose-register (r%12?). That costs
>>> a RDMSR (at least) and an WRMSR (which you can optimize out). We have a
>>> patch for that somewhere if anybody wants it.
>>
>> I would really rather not do that on the NMI path.. And if we _have_ to,
>> please keep a software shadow of that MSR state, such that we can avoid
>> touching that MSR 99% of the time.
>
> Yeah, I'd rather avoid doing firmware calls from NMI context altogether.
>
> Thanks,
>
> Ingo
>
Dave Hansen and I had some discussions on how to handle the nested NMI
and firmware calls. We thought of using
a per cpu counter to record the nesting call depth
and toggle IBRS appropriately for the depth 0->1 and 1->0
transition. Will this change be sufficient?
Thanks.
Tim
commit 55546c27a006198630c57b900abcbd3baaabf63a
Author: Tim Chen <tim.c.chen@xxxxxxxxxxxxxxx>
Date: Tue Feb 13 04:10:41 2018 -0800
x86/firmware: Prevent IBRS from being turned off prematurely.
Dave Woodhoue proposed to use IBRS to protect the firmware
call path against Spectre exploit. However, firmware path
can go through NMI and we can get nested calls, causing
unsafe firmware calls with missing IBRS as illustrated below:
firmware_restrict_branch_speculation_start (set IBRS=1)
NMI
firmware_restrict_branch_speculation_start (set IBRS=1)
firmware call
firmware_restrict_branch_speculation_end (set IBRS=0)
NMI return
firmware call (with IBRS=0) <---- unsafe call, premature IBRS disabling
firmware_restrict_branch_speculation_end (set IBRS=0)
This patch proposes using a per cpu counter to track the IBRS
firmware call nesting depth, to ensure that we don't turn off
IBRS prematurely before calling firmware.
Signed-off-by: Tim Chen <tim.c.chen@xxxxxxxxxxxxxxx>
---
diff --git a/arch/x86/include/asm/nospec-branch.h b/arch/x86/include/asm/nospec-branch.h
index 297d457..1e9c828 100644
--- a/arch/x86/include/asm/nospec-branch.h
+++ b/arch/x86/include/asm/nospec-branch.h
@@ -146,6 +146,8 @@ enum spectre_v2_mitigation {
extern char __indirect_thunk_start[];
extern char __indirect_thunk_end[];
+DECLARE_PER_CPU(int, spec_ctrl_ibrs_fw_depth);
+
/*
* On VMEXIT we must ensure that no RSB predictions learned in the guest
* can be followed in the host, by overwriting the RSB completely. Both
@@ -186,14 +188,16 @@ static inline void indirect_branch_prediction_barrier(void)
*/
static inline void firmware_restrict_branch_speculation_start(void)
{
- alternative_msr_write(MSR_IA32_SPEC_CTRL, SPEC_CTRL_IBRS,
+ if (this_cpu_inc_return(spec_ctrl_ibrs_fw_depth) == 1)
+ alternative_msr_write(MSR_IA32_SPEC_CTRL, SPEC_CTRL_IBRS,
X86_FEATURE_USE_IBRS_FW);
}
static inline void firmware_restrict_branch_speculation_end(void)
{
- alternative_msr_write(MSR_IA32_SPEC_CTRL, 0,
- X86_FEATURE_USE_IBRS_FW);
+ if (this_cpu_dec_return(spec_ctrl_ibrs_fw_depth) == 0)
+ alternative_msr_write(MSR_IA32_SPEC_CTRL, 0,
+ X86_FEATURE_USE_IBRS_FW);
}
#endif /* __ASSEMBLY__ */
#endif /* _ASM_X86_NOSPEC_BRANCH_H_ */
diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
index c994dab..4ab13f0 100644
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -27,6 +27,8 @@
#include <asm/intel-family.h>
static void __init spectre_v2_select_mitigation(void);
+DEFINE_PER_CPU(int, spec_ctrl_ibrs_fw_depth);
+EXPORT_PER_CPU_SYMBOL(spec_ctrl_ibrs_fw_depth);
void __init check_bugs(void)
{
--
To unsubscribe from this list: send the line "unsubscribe linux-tip-commits" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
|