Message ID | 20220805103814.23032-1-andrew.cooper3@citrix.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | x86/spec-ctrl: Use IST RSB protection for !SVM systems | expand |
On 05.08.2022 12:38, Andrew Cooper wrote: > There is a corner case where a VT-x guest which manages to reliably trigger > non-fatal #MC's could evade the rogue RSB speculation protections that were > supposed to be in place. > > This is a lack of defence in depth; Xen does not architecturally execute more > RET than CALL instructions, so an attacker would have to locate a different > gadget (e.g. SpectreRSB) first to execute a transient path of excess RET > instructions. > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com>
On 05.08.2022 12:38, Andrew Cooper wrote: > --- a/xen/arch/x86/spec_ctrl.c > +++ b/xen/arch/x86/spec_ctrl.c > @@ -1327,8 +1327,24 @@ void __init init_speculation_mitigations(void) > * mappings. > */ > if ( opt_rsb_hvm ) > + { > setup_force_cpu_cap(X86_FEATURE_SC_RSB_HVM); > > + /* > + * For SVM, Xen's RSB safety actions are performed before STGI, so > + * behave atomically with respect to IST sources. > + * > + * For VT-x, NMIs are atomic with VMExit (the NMI gets queued but not > + * delivered) whereas other IST sources are not atomic. Specifically, > + * #MC can hit ahead the RSB safety action in the vmexit path. > + * > + * Therefore, it is necessary for the IST logic to protect Xen against > + * possible rogue RSB speculation. > + */ > + if ( !cpu_has_svm ) > + default_spec_ctrl_flags |= SCF_ist_rsb; Only now, when I'm about to backport this, it occurs to me to ask: Why is this !cpu_has_svm rather than cpu_has_vmx? Plus shouldn't this further be gated upon HVM actually being in use (i.e. in particular CONFIG_HVM=y in the first place)? Jan
On 15/08/2022 09:26, Jan Beulich wrote: > On 05.08.2022 12:38, Andrew Cooper wrote: >> --- a/xen/arch/x86/spec_ctrl.c >> +++ b/xen/arch/x86/spec_ctrl.c >> @@ -1327,8 +1327,24 @@ void __init init_speculation_mitigations(void) >> * mappings. >> */ >> if ( opt_rsb_hvm ) >> + { >> setup_force_cpu_cap(X86_FEATURE_SC_RSB_HVM); >> >> + /* >> + * For SVM, Xen's RSB safety actions are performed before STGI, so >> + * behave atomically with respect to IST sources. >> + * >> + * For VT-x, NMIs are atomic with VMExit (the NMI gets queued but not >> + * delivered) whereas other IST sources are not atomic. Specifically, >> + * #MC can hit ahead the RSB safety action in the vmexit path. >> + * >> + * Therefore, it is necessary for the IST logic to protect Xen against >> + * possible rogue RSB speculation. >> + */ >> + if ( !cpu_has_svm ) >> + default_spec_ctrl_flags |= SCF_ist_rsb; > Only now, when I'm about to backport this, it occurs to me to ask: Why > is this !cpu_has_svm rather than cpu_has_vmx? Because it is only SVM known to be safe. > Plus shouldn't this further > be gated upon HVM actually being in use (i.e. in particular CONFIG_HVM=y > in the first place)? Perhaps, but not locally here. All of init_speculation_mitigations() wants reconsidering if you're going down that route. ~Andrew
On 15.08.2022 11:33, Andrew Cooper wrote: > On 15/08/2022 09:26, Jan Beulich wrote: >> On 05.08.2022 12:38, Andrew Cooper wrote: >>> --- a/xen/arch/x86/spec_ctrl.c >>> +++ b/xen/arch/x86/spec_ctrl.c >>> @@ -1327,8 +1327,24 @@ void __init init_speculation_mitigations(void) >>> * mappings. >>> */ >>> if ( opt_rsb_hvm ) >>> + { >>> setup_force_cpu_cap(X86_FEATURE_SC_RSB_HVM); >>> >>> + /* >>> + * For SVM, Xen's RSB safety actions are performed before STGI, so >>> + * behave atomically with respect to IST sources. >>> + * >>> + * For VT-x, NMIs are atomic with VMExit (the NMI gets queued but not >>> + * delivered) whereas other IST sources are not atomic. Specifically, >>> + * #MC can hit ahead the RSB safety action in the vmexit path. >>> + * >>> + * Therefore, it is necessary for the IST logic to protect Xen against >>> + * possible rogue RSB speculation. >>> + */ >>> + if ( !cpu_has_svm ) >>> + default_spec_ctrl_flags |= SCF_ist_rsb; >> Only now, when I'm about to backport this, it occurs to me to ask: Why >> is this !cpu_has_svm rather than cpu_has_vmx? > > Because it is only SVM known to be safe. Yes. Which amounts to only VT-x being unsafe. And in particular PV alone (e.g. shim, from the perspective of the shim itself) is safe as well, no matter what CPU we're on. >> Plus shouldn't this further >> be gated upon HVM actually being in use (i.e. in particular CONFIG_HVM=y >> in the first place)? > > Perhaps, but not locally here. All of init_speculation_mitigations() > wants reconsidering if you're going down that route. Not sure - many of the settings (like X86_FEATURE_SC_RSB_HVM also being set in the enclosing if()) only affect HVM-specific code paths, so which way they are set wouldn't matter when !CONFIG_HVM. But the one here clearly affects a common code path, for no gains at all. It's not an overly hot code path, sure, but it still strikes me as odd. Jan
diff --git a/xen/arch/x86/spec_ctrl.c b/xen/arch/x86/spec_ctrl.c index 44e86f3d674d..d2cd5459739f 100644 --- a/xen/arch/x86/spec_ctrl.c +++ b/xen/arch/x86/spec_ctrl.c @@ -1327,8 +1327,24 @@ void __init init_speculation_mitigations(void) * mappings. */ if ( opt_rsb_hvm ) + { setup_force_cpu_cap(X86_FEATURE_SC_RSB_HVM); + /* + * For SVM, Xen's RSB safety actions are performed before STGI, so + * behave atomically with respect to IST sources. + * + * For VT-x, NMIs are atomic with VMExit (the NMI gets queued but not + * delivered) whereas other IST sources are not atomic. Specifically, + * #MC can hit ahead the RSB safety action in the vmexit path. + * + * Therefore, it is necessary for the IST logic to protect Xen against + * possible rogue RSB speculation. + */ + if ( !cpu_has_svm ) + default_spec_ctrl_flags |= SCF_ist_rsb; + } + ibpb_calculations(); /* Check whether Eager FPU should be enabled by default. */
There is a corner case where a VT-x guest which manages to reliably trigger non-fatal #MC's could evade the rogue RSB speculation protections that were supposed to be in place. This is a lack of defence in depth; Xen does not architecturally execute more RET than CALL instructions, so an attacker would have to locate a different gadget (e.g. SpectreRSB) first to execute a transient path of excess RET instructions. Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Jan Beulich <JBeulich@suse.com> CC: Roger Pau Monné <roger.pau@citrix.com> CC: Wei Liu <wl@xen.org> --- xen/arch/x86/spec_ctrl.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)