On Fri, May 7, 2021 at 8:08 AM Jon Kohler <[email protected]> wrote:
>
> cpufeatures.h defines X86_FEATURE_RSB_CTXSW as "Fill RSB on context
> switches" which seems more accurate than using X86_FEATURE_RETPOLINE
> in the vmxexit path for RSB stuffing.
>
> X86_FEATURE_RSB_CTXSW is used for FILL_RETURN_BUFFER in
> arch/x86/entry/entry_{32|64}.S. This change makes KVM vmx and svm
> follow that same pattern. This pairs up nicely with the language in
> bugs.c, where this cpu_cap is enabled, which indicates that RSB
> stuffing should be unconditional with spectrev2 enabled.
> /*
> * If spectre v2 protection has been enabled, unconditionally fill
> * RSB during a context switch; this protects against two independent
> * issues:
> *
> * - RSB underflow (and switch to BTB) on Skylake+
> * - SpectreRSB variant of spectre v2 on X86_BUG_SPECTRE_V2 CPUs
> */
> setup_force_cpu_cap(X86_FEATURE_RSB_CTXSW);
>
> Furthermore, on X86_FEATURE_IBRS_ENHANCED CPUs && SPECTRE_V2_CMD_AUTO,
> we're bypassing setting X86_FEATURE_RETPOLINE, where as far as I could
> find, we should still be doing RSB stuffing no matter what when
> CONFIG_RETPOLINE is enabled and spectrev2 is set to auto.
If I'm reading https://software.intel.com/security-software-guidance/deep-dives/deep-dive-indirect-branch-restricted-speculation
correctly, I don't think an RSB fill sequence is required on VMExit on
processors w/ Enhanced IBRS. Specifically:
"""
On processors with enhanced IBRS, an RSB overwrite sequence may not
suffice to prevent the predicted target of a near return from using an
RSB entry created in a less privileged predictor mode. Software can
prevent this by enabling SMEP (for transitions from user mode to
supervisor mode) and by having IA32_SPEC_CTRL.IBRS set during VM exits
"""
On Enhanced IBRS processors, it looks like SPEC_CTRL.IBRS is set
across all #VMExits via x86_virt_spec_ctrl in kvm.
So is this patch needed?
Thanks,
-- vs;
> On May 7, 2021, at 1:22 PM, Venkatesh Srinivas <[email protected]> wrote:
>
> On Fri, May 7, 2021 at 8:08 AM Jon Kohler <[email protected]> wrote:
>>
>> cpufeatures.h defines X86_FEATURE_RSB_CTXSW as "Fill RSB on context
>> switches" which seems more accurate than using X86_FEATURE_RETPOLINE
>> in the vmxexit path for RSB stuffing.
>>
>> X86_FEATURE_RSB_CTXSW is used for FILL_RETURN_BUFFER in
>> arch/x86/entry/entry_{32|64}.S. This change makes KVM vmx and svm
>> follow that same pattern. This pairs up nicely with the language in
>> bugs.c, where this cpu_cap is enabled, which indicates that RSB
>> stuffing should be unconditional with spectrev2 enabled.
>> /*
>> * If spectre v2 protection has been enabled, unconditionally fill
>> * RSB during a context switch; this protects against two independent
>> * issues:
>> *
>> * - RSB underflow (and switch to BTB) on Skylake+
>> * - SpectreRSB variant of spectre v2 on X86_BUG_SPECTRE_V2 CPUs
>> */
>> setup_force_cpu_cap(X86_FEATURE_RSB_CTXSW);
>>
>> Furthermore, on X86_FEATURE_IBRS_ENHANCED CPUs && SPECTRE_V2_CMD_AUTO,
>> we're bypassing setting X86_FEATURE_RETPOLINE, where as far as I could
>> find, we should still be doing RSB stuffing no matter what when
>> CONFIG_RETPOLINE is enabled and spectrev2 is set to auto.
>
> If I'm reading https://urldefense.proofpoint.com/v2/url?u=https-3A__software.intel.com_security-2Dsoftware-2Dguidance_deep-2Ddives_deep-2Ddive-2Dindirect-2Dbranch-2Drestricted-2Dspeculation&d=DwIBaQ&c=s883GpUCOChKOHiocYtGcg&r=NGPRGGo37mQiSXgHKm5rCQ&m=s8fqknrIuUa_jGbbihj0anypC4jz86QQ7UzzAop3B7k&s=oIcZtb8S_gcK5L1yzfPvinSHxjCCsx1PNn-imPMffKU&e=
> correctly, I don't think an RSB fill sequence is required on VMExit on
> processors w/ Enhanced IBRS. Specifically:
> """
> On processors with enhanced IBRS, an RSB overwrite sequence may not
> suffice to prevent the predicted target of a near return from using an
> RSB entry created in a less privileged predictor mode. Software can
> prevent this by enabling SMEP (for transitions from user mode to
> supervisor mode) and by having IA32_SPEC_CTRL.IBRS set during VM exits
> """
> On Enhanced IBRS processors, it looks like SPEC_CTRL.IBRS is set
> across all #VMExits via x86_virt_spec_ctrl in kvm.
>
> So is this patch needed?
>
> Thanks,
> -- vs;
Venkatesh - Thanks for the reply. I read that the other way around, wherein
RSB overwrite still isn't good enough on eIBRS, so one would need to do all
three of the following to be in good shape:
a. RSB overwrite sequence
b. enable SMEP
c. toggle IA32_SPEC_CTRL.IBRS on vmexits
Said another way, the document reads like one would always need to do the
RSB overwrite sequence no matter what. Happy to hear if that is not the
case though, since RSB stuffing is a little expensive.
Note: I also checked the Intel SDM to see if perhaps there was something
there about this, but the document you linked is the only one I could
find on the topic.
On Fri, May 07, 2021, Venkatesh Srinivas wrote:
> On Fri, May 7, 2021 at 8:08 AM Jon Kohler <[email protected]> wrote:
> >
> > cpufeatures.h defines X86_FEATURE_RSB_CTXSW as "Fill RSB on context
> > switches" which seems more accurate than using X86_FEATURE_RETPOLINE
> > in the vmxexit path for RSB stuffing.
> >
> > X86_FEATURE_RSB_CTXSW is used for FILL_RETURN_BUFFER in
> > arch/x86/entry/entry_{32|64}.S. This change makes KVM vmx and svm
> > follow that same pattern. This pairs up nicely with the language in
> > bugs.c, where this cpu_cap is enabled, which indicates that RSB
> > stuffing should be unconditional with spectrev2 enabled.
> > /*
> > * If spectre v2 protection has been enabled, unconditionally fill
> > * RSB during a context switch; this protects against two independent
> > * issues:
> > *
> > * - RSB underflow (and switch to BTB) on Skylake+
> > * - SpectreRSB variant of spectre v2 on X86_BUG_SPECTRE_V2 CPUs
> > */
> > setup_force_cpu_cap(X86_FEATURE_RSB_CTXSW);
> >
> > Furthermore, on X86_FEATURE_IBRS_ENHANCED CPUs && SPECTRE_V2_CMD_AUTO,
> > we're bypassing setting X86_FEATURE_RETPOLINE, where as far as I could
> > find, we should still be doing RSB stuffing no matter what when
> > CONFIG_RETPOLINE is enabled and spectrev2 is set to auto.
>
> If I'm reading https://software.intel.com/security-software-guidance/deep-dives/deep-dive-indirect-branch-restricted-speculation
> correctly, I don't think an RSB fill sequence is required on VMExit on
> processors w/ Enhanced IBRS. Specifically:
> """
> On processors with enhanced IBRS, an RSB overwrite sequence may not
> suffice to prevent the predicted target of a near return from using an
> RSB entry created in a less privileged predictor mode. Software can
> prevent this by enabling SMEP (for transitions from user mode to
> supervisor mode) and by having IA32_SPEC_CTRL.IBRS set during VM exits
> """
> On Enhanced IBRS processors, it looks like SPEC_CTRL.IBRS is set
> across all #VMExits via x86_virt_spec_ctrl in kvm.
>
> So is this patch needed?
Venkatesh belatedly pointed out (off list) that KVM VMX stops intercepting
MSR_IA32_SPEC_CTRL after the first (successful) write by the guest. But, I
believe that's a non-issue for ENHANCED_IBRS because of this blurb in Intel's
documentation[*]:
Processors with enhanced IBRS still support the usage model where IBRS is set
only in the OS/VMM for OSes that enable SMEP. To do this, such processors will
ensure that guest behavior cannot control the RSB after a VM exit once IBRS is
set, even if IBRS was not set at the time of the VM exit.
The code and changelog for commit 706d51681d63 ("x86/speculation: Support
Enhanced IBRS on future CPUs") is more than a little confusing:
spectre_v2_select_mitigation():
if (boot_cpu_has(X86_FEATURE_IBRS_ENHANCED)) {
mode = SPECTRE_V2_IBRS_ENHANCED;
/* Force it so VMEXIT will restore correctly */
x86_spec_ctrl_base |= SPEC_CTRL_IBRS;
wrmsrl(MSR_IA32_SPEC_CTRL, x86_spec_ctrl_base);
goto specv2_set_mode;
}
changelog:
Kernel also has to make sure that IBRS bit remains set after
VMEXIT because the guest might have cleared the bit. This is already
covered by the existing x86_spec_ctrl_set_guest() and
x86_spec_ctrl_restore_host() speculation control functions.
but I _think_ that is simply saying that MSR_IA32_SPEC_CTRL.IBRS needs to be
restored in order to keep the mitigations active in the host. I don't think it
contradicts the documentation that says VM-Exit is automagically mitigated if
IBRS has _ever_ been set.
[*] https://software.intel.com/security-software-guidance/deep-dives/deep-dive-indirect-branch-restricted-speculation
> On May 7, 2021, at 1:58 PM, Sean Christopherson <[email protected]> wrote:
>
> On Fri, May 07, 2021, Venkatesh Srinivas wrote:
>> On Fri, May 7, 2021 at 8:08 AM Jon Kohler <[email protected]> wrote:
>>>
>>> cpufeatures.h defines X86_FEATURE_RSB_CTXSW as "Fill RSB on context
>>> switches" which seems more accurate than using X86_FEATURE_RETPOLINE
>>> in the vmxexit path for RSB stuffing.
>>>
>>> X86_FEATURE_RSB_CTXSW is used for FILL_RETURN_BUFFER in
>>> arch/x86/entry/entry_{32|64}.S. This change makes KVM vmx and svm
>>> follow that same pattern. This pairs up nicely with the language in
>>> bugs.c, where this cpu_cap is enabled, which indicates that RSB
>>> stuffing should be unconditional with spectrev2 enabled.
>>> /*
>>> * If spectre v2 protection has been enabled, unconditionally fill
>>> * RSB during a context switch; this protects against two independent
>>> * issues:
>>> *
>>> * - RSB underflow (and switch to BTB) on Skylake+
>>> * - SpectreRSB variant of spectre v2 on X86_BUG_SPECTRE_V2 CPUs
>>> */
>>> setup_force_cpu_cap(X86_FEATURE_RSB_CTXSW);
>>>
>>> Furthermore, on X86_FEATURE_IBRS_ENHANCED CPUs && SPECTRE_V2_CMD_AUTO,
>>> we're bypassing setting X86_FEATURE_RETPOLINE, where as far as I could
>>> find, we should still be doing RSB stuffing no matter what when
>>> CONFIG_RETPOLINE is enabled and spectrev2 is set to auto.
>>
>> If I'm reading https://urldefense.proofpoint.com/v2/url?u=https-3A__software.intel.com_security-2Dsoftware-2Dguidance_deep-2Ddives_deep-2Ddive-2Dindirect-2Dbranch-2Drestricted-2Dspeculation&d=DwIBAg&c=s883GpUCOChKOHiocYtGcg&r=NGPRGGo37mQiSXgHKm5rCQ&m=d1CkIBCdVFwUtKYx3SRW9dZD0kA_IX9VKEPG2-x4kBo&s=KlK_T41o6UVpLhMKDcK9iZfsJnop72K3CveJVIak5K8&e=
>> correctly, I don't think an RSB fill sequence is required on VMExit on
>> processors w/ Enhanced IBRS. Specifically:
>> """
>> On processors with enhanced IBRS, an RSB overwrite sequence may not
>> suffice to prevent the predicted target of a near return from using an
>> RSB entry created in a less privileged predictor mode. Software can
>> prevent this by enabling SMEP (for transitions from user mode to
>> supervisor mode) and by having IA32_SPEC_CTRL.IBRS set during VM exits
>> """
>> On Enhanced IBRS processors, it looks like SPEC_CTRL.IBRS is set
>> across all #VMExits via x86_virt_spec_ctrl in kvm.
>>
>> So is this patch needed?
>
> Venkatesh belatedly pointed out (off list) that KVM VMX stops intercepting
> MSR_IA32_SPEC_CTRL after the first (successful) write by the guest. But, I
> believe that's a non-issue for ENHANCED_IBRS because of this blurb in Intel's
> documentation[*]:
>
> Processors with enhanced IBRS still support the usage model where IBRS is set
> only in the OS/VMM for OSes that enable SMEP. To do this, such processors will
> ensure that guest behavior cannot control the RSB after a VM exit once IBRS is
> set, even if IBRS was not set at the time of the VM exit.
>
> The code and changelog for commit 706d51681d63 ("x86/speculation: Support
> Enhanced IBRS on future CPUs") is more than a little confusing:
>
> spectre_v2_select_mitigation():
> if (boot_cpu_has(X86_FEATURE_IBRS_ENHANCED)) {
> mode = SPECTRE_V2_IBRS_ENHANCED;
> /* Force it so VMEXIT will restore correctly */
> x86_spec_ctrl_base |= SPEC_CTRL_IBRS;
> wrmsrl(MSR_IA32_SPEC_CTRL, x86_spec_ctrl_base);
> goto specv2_set_mode;
> }
Thanks Sean, that makes sense. The handling of the MSR part of things
is separate from what I was looking at, which is where it seems like
Intel is still recommending doing an RSB overwrite/stuff even with
eIBRS; however, in KVM we use X86_FEATURE_RETPOLINE to figure
that out, but in bugs.c eIBRS systems skip having set, as
goto specv2_set_mode skips cover retpoline_auto now.
>
> changelog:
> Kernel also has to make sure that IBRS bit remains set after
> VMEXIT because the guest might have cleared the bit. This is already
> covered by the existing x86_spec_ctrl_set_guest() and
> x86_spec_ctrl_restore_host() speculation control functions.
>
> but I _think_ that is simply saying that MSR_IA32_SPEC_CTRL.IBRS needs to be
> restored in order to keep the mitigations active in the host. I don't think it
> contradicts the documentation that says VM-Exit is automagically mitigated if
> IBRS has _ever_ been set.
>
> [*] https://urldefense.proofpoint.com/v2/url?u=https-3A__software.intel.com_security-2Dsoftware-2Dguidance_deep-2Ddives_deep-2Ddive-2Dindirect-2Dbranch-2Drestricted-2Dspeculation&d=DwIBAg&c=s883GpUCOChKOHiocYtGcg&r=NGPRGGo37mQiSXgHKm5rCQ&m=d1CkIBCdVFwUtKYx3SRW9dZD0kA_IX9VKEPG2-x4kBo&s=KlK_T41o6UVpLhMKDcK9iZfsJnop72K3CveJVIak5K8&e=