Re: [Xen-devel] [PATCH v11 7/11] x86/entry: Avoid using alternatives in NMI/#MC paths

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 25.01.18 at 18:21, <andrew.cooper3@xxxxxxxxxx> wrote:
> This patch is deliberately arranged to be easy to revert if/when alternatives
> patching becomes NMI/#MC safe.
> 
> For safety, there must be a dispatch serialising instruction in (what is
> logically) DO_SPEC_CTRL_ENTRY so that, in the case that Xen needs IBRS set in
> context, an attacker can't speculate around the WRMSR and reach an indirect
> branch within the speculation window.
> 
> Using conditionals opens this attack vector up, so the else clause gets an
> LFENCE to force the pipeline to catch up before continuing.  This also covers
> the safety of RSB conditional, as execution it is guaranteed to either hit the
> WRMSR or LFENCE.
> 
> One downside of not using alternatives is that there unconditionally an LFENCE
> in the IST path in cases where we are not using the features from IBRS-capable
> microcode.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

Despite this yet another remark:

> @@ -255,6 +260,69 @@
>          DO_SPEC_CTRL_EXIT_TO_GUEST, X86_FEATURE_XEN_IBRS_SET,           \
>          DO_SPEC_CTRL_EXIT_TO_GUEST, X86_FEATURE_XEN_IBRS_CLEAR
>  
> +/* TODO: Drop these when the alternatives infrastructure is NMI/#MC safe. */
> +.macro SPEC_CTRL_ENTRY_FROM_INTR_IST
> +/*
> + * Requires %rsp=regs, %r14=stack_end
> + * Clobbers %rax, %rcx, %rdx
> + *
> + * This is logical merge of DO_OVERWRITE_RSB and DO_SPEC_CTRL_ENTRY
> + * maybexen=1, but with conditionals rather than alternatives.
> + */
> +    movzbl STACK_CPUINFO_FIELD(bti_ist_info)(%r14), %eax
> +
> +    testb $BTI_IST_RSB, %al
> +    jz .L\@_skip_rsb
> +
> +    DO_OVERWRITE_RSB
> +
> +.L\@_skip_rsb:
> +
> +    testb $BTI_IST_WRMSR, %al
> +    jz .L\@_skip_wrmsr
> +
> +    testb $3, UREGS_cs(%rsp)
> +    jz .L\@_entry_from_xen
> +
> +    movb $0, STACK_CPUINFO_FIELD(use_shadow_spec_ctrl)(%r14)

I take it you deliberately didn't want to use the same setz/and
pair here to avoid at least the second branch here too, as we
very much hope to only rarely hit this code path?

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel