Re: [Xen-devel] [PATCH for-4.13 2/2] xen/nospec: Introduce CONFIG_SPECULATIVE_BRANCH_HARDEN and disable it

[Julien Grall <julien.grall@xxxxxxx>]

Hi,

On 9/30/19 7:24 PM, Andrew Cooper wrote:
> The code generation for barrier_nospec_true() is not correct.  We are taking a
> perf it from the added fences, but not gaining any speculative safety.
s/it/hit/?

> This is caused by inline assembly trying to fight the compiler optimiser, and
> the optimiser winning.  There is no clear way to achieve safety, so turn the
> perf hit off for now.
>
> This also largely reverts 3860d5534df4.  The name 'l1tf-barrier', and making
> barrier_nospec_true() depend on CONFIG_HVM was constrained by what could be
> discussed publicly at the time.  Now that MDS is public, neither aspects are
> correct.
>
> As l1tf-barrier hasn't been in a release of Xen, and
> CONFIG_SPECULATIVE_BRANCH_HARDEN is disabled until we can find a safe way of
> implementing the functionality, remove the l1tf-barrier command line option.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> ---
> CC: Jan Beulich <JBeulich@xxxxxxxx>
> CC: Wei Liu <wl@xxxxxxx>
> CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> CC: Juergen Gross <jgross@xxxxxxxx>
> CC: Norbert Manthey <nmanthey@xxxxxxxxx>
> ---
>   docs/misc/xen-command-line.pandoc |  8 +-------
>   xen/arch/x86/spec_ctrl.c          | 17 ++---------------
>   xen/common/Kconfig                | 17 +++++++++++++++++
I think this wanted to have "THE REST" CCed.

>   xen/include/asm-x86/cpufeatures.h |  2 +-
>   xen/include/asm-x86/nospec.h      |  4 ++--
>   xen/include/asm-x86/spec_ctrl.h   |  1 -
>   6 files changed, 23 insertions(+), 26 deletions(-)
[...]

> diff --git a/xen/common/Kconfig b/xen/common/Kconfig
> index 9644cc9911..d851e63083 100644
> --- a/xen/common/Kconfig
> +++ b/xen/common/Kconfig
> @@ -96,6 +96,23 @@ config SPECULATIVE_ARRAY_HARDEN
>   
>   	  If unsure, say Y.
>   
> +config SPECULATIVE_BRANCH_HARDEN
> +       bool "Speculative Branch Hardening"
> +       depends on BROKEN
> +        ---help---
> +         Contemporary processors may use speculative execution as a
> +         performance optimisation, but this can potentially be abused by an
> +         attacker to leak data via speculative sidechannels.
> +
> +         One source of misbehaviour is by executing the wrong basic block
> +         following a conditional jump.
> +
> +         When enabled, specific conditions which have been deemed liable to
> +         be speculatively abused will be hardened to avoid entering the wrong
> +         basic block.
> +
> +         !!! WARNING - This is broken and doesn't generate safe code !!!
Any reason to add that in common code when this is x86 only? My worry is 
this gate config gate nothing on Arm so the user may have a false sense 
that it can be used (even though it is clearly BROKEN).
Also the name is quite close to the CONFIG_HARDEN_PREDICTOR on Arm and 
may confuse user. Although, I don't have a better name so far :/
Cheers,

--
Julien Grall

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