linux-hardening - Re: [PATCH] [PATCH] AARCH64: Add gcc Shadow Call Stack support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 22 Feb 2022 09:16:06 -0700
From:   Nathan Chancellor <nathan@...nel.org>
To:     Dan Li <ashimida@...ux.alibaba.com>
Cc:     catalin.marinas@....com, will@...nel.org, ndesaulniers@...gle.com,
        keescook@...omium.org, masahiroy@...nel.org, tglx@...utronix.de,
        akpm@...ux-foundation.org, mark.rutland@....com,
        samitolvanen@...gle.com, npiggin@...il.com, linux@...ck-us.net,
        mhiramat@...nel.org, ojeda@...nel.org, luc.vanoostenryck@...il.com,
        elver@...gle.com, linux-kernel@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org, llvm@...ts.linux.dev,
        linux-hardening@...r.kernel.org
Subject: Re: [PATCH] [PATCH] AARCH64: Add gcc Shadow Call Stack support

On Tue, Feb 22, 2022 at 01:57:36AM -0800, Dan Li wrote:
> Shadow call stack is available in GCC > 11.2.0, this patch makes
> the corresponding kernel configuration available when compiling
> the kernel with gcc.
> 
> Note that the implementation in GCC is slightly different from Clang.
> With SCS enabled, functions will only pop x30 once in the epilogue,
> like:
> 
>    str     x30, [x18], #8
>    stp     x29, x30, [sp, #-16]!
>    ......
> -  ldp     x29, x30, [sp], #16	  //clang
> +  ldr     x29, [sp], #16	  //GCC
>    ldr     x30, [x18, #-8]!
> 
> Link: https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=ce09ab17ddd21f73ff2caf6eec3b0ee9b0e1a11e
> 
> Signed-off-by: Dan Li <ashimida@...ux.alibaba.com>

Reviewed-by: Nathan Chancellor <nathan@...nel.org>

A few open-ended comments below.

> ---
> FYI:
> This function can be used to test if the shadow call stack works:
> //noinline void __noscs scs_test(void)
> noinline void scs_test(void)
> {
>     register unsigned long *sp asm("sp");
>     unsigned long * lr = sp + 1;
> 
>     asm volatile("":::"x30");
>     *lr = 0;
> }
> 
> ffff800008012704:       d503233f        paciasp
> ffff800008012708:       f800865e        str     x30, [x18], #8
> ffff80000801270c:       a9bf7bfd        stp     x29, x30, [sp, #-16]!
> ffff800008012710:       910003fd        mov     x29, sp
> ffff800008012714:       910003e0        mov     x0, sp
> ffff800008012718:       f900041f        str     xzr, [x0, #8]
> ffff80000801271c:       f85f8e5e        ldr     x30, [x18, #-8]!
> ffff800008012720:       f84107fd        ldr     x29, [sp], #16
> ffff800008012724:       d50323bf        autiasp
> ffff800008012728:       d65f03c0        ret
> 
> If SCS protection is enabled, this function will return normally.
> If the function has __noscs attribute (scs disabled), it will crash due to 0
> address access.
> 
>  arch/Kconfig                 | 6 +++---
>  arch/arm64/Kconfig           | 2 +-
>  include/linux/compiler-gcc.h | 4 ++++
>  3 files changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/Kconfig b/arch/Kconfig
> index 678a80713b21..35db7b72bdb0 100644
> --- a/arch/Kconfig
> +++ b/arch/Kconfig
> @@ -604,11 +604,11 @@ config ARCH_SUPPORTS_SHADOW_CALL_STACK
>  	  switching.
>  
>  config SHADOW_CALL_STACK
> -	bool "Clang Shadow Call Stack"
> -	depends on CC_IS_CLANG && ARCH_SUPPORTS_SHADOW_CALL_STACK
> +	bool "Shadow Call Stack"
> +	depends on ARCH_SUPPORTS_SHADOW_CALL_STACK
>  	depends on DYNAMIC_FTRACE_WITH_REGS || !FUNCTION_GRAPH_TRACER
>  	help
> -	  This option enables Clang's Shadow Call Stack, which uses a
> +	  This option enables Clang/GCC's Shadow Call Stack, which uses a

I wonder if we want to just ditch the mention of the compiler if both
support it?

>  	  shadow stack to protect function return addresses from being
>  	  overwritten by an attacker. More information can be found in
>  	  Clang's documentation:
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 09b885cc4db5..a48a604301aa 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -1255,7 +1255,7 @@ config HW_PERF_EVENTS
>  config ARCH_HAS_FILTER_PGPROT
>  	def_bool y
>  
> -# Supported by clang >= 7.0
> +# Supported by clang >= 7.0 or GCC > 11.2.0

Same thing here, although eventually there may be a minimum GCC version
bump to something newer than 11.2.0, which would allow us to just drop
CONFIG_CC_HAVE_SHADOW_CALL_STACK altogether. No strong opinion.

>  config CC_HAVE_SHADOW_CALL_STACK
>  	def_bool $(cc-option, -fsanitize=shadow-call-stack -ffixed-x18)
>  
> diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
> index ccbbd31b3aae..deff5b308470 100644
> --- a/include/linux/compiler-gcc.h
> +++ b/include/linux/compiler-gcc.h
> @@ -97,6 +97,10 @@
>  #define KASAN_ABI_VERSION 4
>  #endif
>  
> +#ifdef CONFIG_SHADOW_CALL_STACK
> +#define __noscs __attribute__((__no_sanitize__("shadow-call-stack")))
> +#endif
> +
>  #if __has_attribute(__no_sanitize_address__)
>  #define __no_sanitize_address __attribute__((no_sanitize_address))
>  #else
> -- 
> 2.17.1
>