Re: [PATCH v2 03/14] x86/shstk: Introduce Supervisor Shadow Stack support

[Jan Beulich <jbeulich@xxxxxxxx>]

On 27.05.2020 21:18, Andrew Cooper wrote:
> Introduce CONFIG_HAS_AS_CET to determine whether CET instructions are
> supported in the assembler, and CONFIG_XEN_SHSTK as the main build option.
> 
> Introduce cet={no-,}shstk to for a user to select whether or not to use shadow
> stacks at runtime, and X86_FEATURE_XEN_SHSTK to determine Xen's overall
> enablement of shadow stacks.
> 
> 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>
> 
> LLVM 6 supports CET-SS instructions while only LLVM 7 supports CET-IBT
> instructions.  We'd need to split HAS_AS_CET into two if we want to support
> supervisor shadow stacks with LLVM 6.  (This demonstrates exactly why picking
> a handful of instructions to test is the right approach.)

In this case I agree with splitting; I wasn't aware clang implemented
the respective insns piecemeal.

> --- a/docs/misc/xen-command-line.pandoc
> +++ b/docs/misc/xen-command-line.pandoc
> @@ -270,6 +270,23 @@ and not running softirqs. Reduce this if softirqs are 
> not being run frequently
>  enough. Setting this to a high value may cause boot failure, particularly if
>  the NMI watchdog is also enabled.
>  
> +### cet
> +    = List of [ shstk=<bool> ]
> +
> +    Applicability: x86
> +
> +Controls for the use of Control-flow Enforcement Technology.  CET is group of

Nit: "... is a group of ..."

> --- a/xen/arch/x86/Kconfig
> +++ b/xen/arch/x86/Kconfig
> @@ -34,6 +34,10 @@ config ARCH_DEFCONFIG
>  config INDIRECT_THUNK
>       def_bool $(cc-option,-mindirect-branch-register)
>  
> +config HAS_AS_CET
> +     # binutils >= 2.29 and LLVM >= 7
> +     def_bool $(as-instr,wrssq %rax$(comma)0;setssbsy;endbr64)

So you put me in a really awkward position: I'd really like to see
this series go in for 4.14, yet I've previously indicated I want the
underlying concept to first be agreed upon, before any uses get
introduced.

A fundamental problem with this, at least as long as (a) more of
Anthony's series hasn't been committed and (b) we re-build Xen upon
installing (as root), even if it was fully built (as non-root) and
is ready without any re-building. Each of these aspects means that
what you've configured and built may not be what gets installed, by
virtue of the tool chains differing. (a) in addition may lead to
the install-time rebuild to actually go wrong, due to there being
dependency tracking issues on at least {xen,efi}.lds (which I've
noticed in a different context yesterday).

> --- a/xen/scripts/Kconfig.include
> +++ b/xen/scripts/Kconfig.include
> @@ -31,6 +31,10 @@ cc-option = $(success,$(CC) -Werror $(CLANG_FLAGS) $(1) -E 
> -x c /dev/null -o /de
>  # Return y if the linker supports <flag>, n otherwise
>  ld-option = $(success,$(LD) -v $(1))
>  
> +# $(as-instr,<instr>)
> +# Return y if the assembler supports <instr>, n otherwise
> +as-instr = $(success,printf "%b\n" "$(1)" | $(CC) $(CLANG_FLAGS) -c -x 
> assembler -o /dev/null -)

Is this actually checking the right thing in the clang case? I.e.
doesn't "-x assembler" make clang invoke the system assembler rather
than use its integrated one, whereas you need the insns to be
recognized by the integrated assembler unless we find a need to pass
-no-integrated-as?

Jan