linux-kernel - Re: [PATCH v5] rust: support for shadow call stack sanitizer

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 linux-cve-announce 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]

Message-ID: <20240820143503.GD28338@willie-the-truck>
Date: Tue, 20 Aug 2024 15:35:04 +0100
From: Will Deacon <will@...nel.org>
To: Alice Ryhl <aliceryhl@...gle.com>
Cc: Catalin Marinas <catalin.marinas@....com>,
	Jamie Cunliffe <Jamie.Cunliffe@....com>,
	Sami Tolvanen <samitolvanen@...gle.com>,
	Nathan Chancellor <nathan@...nel.org>,
	Conor Dooley <conor@...nel.org>,
	Masahiro Yamada <masahiroy@...nel.org>,
	Nicolas Schier <nicolas@...sle.eu>,
	Ard Biesheuvel <ardb@...nel.org>, Marc Zyngier <maz@...nel.org>,
	Mark Rutland <mark.rutland@....com>,
	Mark Brown <broonie@...nel.org>,
	Nick Desaulniers <ndesaulniers@...gle.com>,
	Miguel Ojeda <ojeda@...nel.org>,
	Alex Gaynor <alex.gaynor@...il.com>,
	Wedson Almeida Filho <wedsonaf@...il.com>,
	Boqun Feng <boqun.feng@...il.com>, Gary Guo <gary@...yguo.net>,
	Björn Roy Baron <bjorn3_gh@...tonmail.com>,
	Benno Lossin <benno.lossin@...ton.me>,
	Andreas Hindborg <a.hindborg@...sung.com>,
	Valentin Obst <kernel@...entinobst.de>,
	linux-kbuild@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org,
	rust-for-linux@...r.kernel.org, Kees Cook <kees@...nel.org>
Subject: Re: [PATCH v5] rust: support for shadow call stack sanitizer

On Tue, Aug 06, 2024 at 10:01:44AM +0000, Alice Ryhl wrote:
> This patch adds all of the flags that are needed to support the shadow
> call stack (SCS) sanitizer with Rust, and updates Kconfig to allow
> configurations that work.

Minor nit, but some folks have allergic reactions to "This patch".
See:

https://docs.kernel.org/process/submitting-patches.html#describe-your-changes

I think the commit message is much better now, though, so thank you for
adding so much more detail for v5. If you end up respinning anyway, you
could move this all to the imperative.

>  Makefile            | 1 +
>  arch/arm64/Makefile | 3 +++
>  init/Kconfig        | 2 +-
>  3 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/Makefile b/Makefile
> index 44c02a6f60a1..eb01a26d8354 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -927,6 +927,7 @@ ifdef CONFIG_SHADOW_CALL_STACK
>  ifndef CONFIG_DYNAMIC_SCS
>  CC_FLAGS_SCS	:= -fsanitize=shadow-call-stack
>  KBUILD_CFLAGS	+= $(CC_FLAGS_SCS)
> +KBUILD_RUSTFLAGS += -Zsanitizer=shadow-call-stack
>  endif
>  export CC_FLAGS_SCS
>  endif
> diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
> index f6bc3da1ef11..b058c4803efb 100644
> --- a/arch/arm64/Makefile
> +++ b/arch/arm64/Makefile
> @@ -57,9 +57,11 @@ KBUILD_AFLAGS	+= $(call cc-option,-mabi=lp64)
>  ifneq ($(CONFIG_UNWIND_TABLES),y)
>  KBUILD_CFLAGS	+= -fno-asynchronous-unwind-tables -fno-unwind-tables
>  KBUILD_AFLAGS	+= -fno-asynchronous-unwind-tables -fno-unwind-tables
> +KBUILD_RUSTFLAGS += -Cforce-unwind-tables=n
>  else
>  KBUILD_CFLAGS	+= -fasynchronous-unwind-tables
>  KBUILD_AFLAGS	+= -fasynchronous-unwind-tables
> +KBUILD_RUSTFLAGS += -Cforce-unwind-tables=y -Zuse-sync-unwind=n
>  endif
>  
>  ifeq ($(CONFIG_STACKPROTECTOR_PER_TASK),y)
> @@ -114,6 +116,7 @@ endif
>  
>  ifeq ($(CONFIG_SHADOW_CALL_STACK), y)
>  KBUILD_CFLAGS	+= -ffixed-x18
> +KBUILD_RUSTFLAGS += -Zfixed-x18
>  endif
>  
>  ifeq ($(CONFIG_CPU_BIG_ENDIAN), y)
> diff --git a/init/Kconfig b/init/Kconfig
> index fe76c5d0a72e..d857f6f90885 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -1909,7 +1909,7 @@ config RUST
>  	depends on !MODVERSIONS
>  	depends on !GCC_PLUGINS
>  	depends on !RANDSTRUCT
> -	depends on !SHADOW_CALL_STACK
> +	depends on !SHADOW_CALL_STACK || RUSTC_VERSION >= 108000 && UNWIND_PATCH_PAC_INTO_SCS

Sorry, I didn't spot this in v4, but since UNWIND_PATCH_PAC_INTO_SCS is
specific to arm64 and the only other architecture selecting
ARCH_SUPPORTS_SHADOW_CALL_STACK is riscv, I can't help but feel it would
be cleaner to move this logic into the arch code selecting HAVE_RUST.

That is, it's up to the architecture to make sure that it has whatever
it needs for SCS to work with Rust if it claims to support Rust.

What do you think?

Will