[PATCH v4 0/2] Rust and the shadow call stack sanitizer

[PATCH v4 0/2] Rust and the shadow call stack sanitizer

[Alice Ryhl]

This patch series makes it possible to use Rust together with the shadow
call stack sanitizer. The first patch is intended to be backported to
ensure that people don't try to use SCS with Rust on older kernel
versions. The second patch makes it possible to use Rust with the shadow
call stack sanitizer.

The second patch in this series depends on the next version of [1],
which Miguel will send soon.

Link: https://lore.kernel.org/rust-for-linux/20240709160615.998336-12-ojeda@kernel.org/ [1]
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
Changes in v4:
- Move `depends on` to CONFIG_RUST.
- Rewrite commit messages to include more context.
- Link to v3: https://lore.kernel.org/r/20240704-shadow-call-stack-v3-0-d11c7a6ebe30@google.com

Changes in v3:
- Use -Zfixed-x18.
- Add logic to reject unsupported rustc versions.
- Also include a fix to be backported.
- Link to v2: https://lore.kernel.org/rust-for-linux/20240305-shadow-call-stack-v2-1-c7b4a3f4d616@google.com/

Changes in v2:
- Add -Cforce-unwind-tables flag.
- Link to v1: https://lore.kernel.org/rust-for-linux/20240304-shadow-call-stack-v1-1-f055eaf40a2c@google.com/

---
Alice Ryhl (2):
      rust: SHADOW_CALL_STACK is incompatible with Rust
      rust: support for shadow call stack sanitizer

 Makefile            | 1 +
 arch/arm64/Makefile | 3 +++
 init/Kconfig        | 1 +
 3 files changed, 5 insertions(+)
---
base-commit: 9cde54ad2f7ac3cf84f65df605570c5a00afc82f
change-id: 20240304-shadow-call-stack-9c197a4361d9

Best regards,
-- 
Alice Ryhl <aliceryhl@google.com>

[PATCH v4 1/2] rust: SHADOW_CALL_STACK is incompatible with Rust

[Alice Ryhl]

When using the shadow call stack sanitizer, all code must be compiled
with the -ffixed-x18 flag, but this flag is not currently being passed
to Rust. This results in crashes that are extremely difficult to debug.

To ensure that nobody else has to go through the same debugging session
that I had to, prevent configurations that enable both SHADOW_CALL_STACK
and RUST.

It is rather common for people to backport 724a75ac9542 ("arm64: rust:
Enable Rust support for AArch64"), so I recommend applying this fix all
the way back to 6.1.

Cc: <stable@vger.kernel.org> # 6.1 and later
Fixes: 724a75ac9542 ("arm64: rust: Enable Rust support for AArch64")
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
 init/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/init/Kconfig b/init/Kconfig
index b0238c4b6e79..914edf51b068 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1909,6 +1909,7 @@ config RUST
 	depends on !MODVERSIONS
 	depends on !GCC_PLUGINS
 	depends on !RANDSTRUCT
+	depends on !SHADOW_CALL_STACK
 	depends on !DEBUG_INFO_BTF || PAHOLE_HAS_LANG_EXCLUDE
 	help
 	  Enables Rust support in the kernel.

-- 
2.46.0.rc1.232.g9752f9e123-goog

[PATCH v4 2/2] rust: support for shadow call stack sanitizer

[Alice Ryhl]

To use the shadow call stack sanitizer, you must pass special flags:

* On arm64, you must pass -ffixed-x18 to your compiler.
* On riscv, you must pass --no-relax-gp to your linker.

These requirements also apply to Rust code. When using Rust on arm64,
you must pass the -Zfixed-x18 flag to rustc, which has the same effect
as the -ffixed-x18 flag does for C code. The -Zfixed-x18 flag requires
rustc version 1.80.0 or greater.

There is no need to pass any flags to rustc on riscv as only the linker
requires additional flags on this platform.

On older versions of Rust, it is still possible to use shadow call stack
by passing -Ctarget-feature=+reserve-x18 instead of -Zfixed-x18.
However, this flag emits a warning during the build, so this patch does
not add support for using it.

Currently, the compiler thinks that the aarch64-unknown-none target
doesn't support -Zsanitizer=shadow-call-stack, so the build will fail if
you enable shadow call stack in non-dynamic mode. See [1] for the
relevant feature request. To avoid this compilation failure, Kconfig is
set up to reject such configurations.

The `depends on` clause is placed on `config RUST` to avoid a situation
where enabling Rust silently turns off the sanitizer. Instead, turning
on the sanitizer results in Rust being disabled. We generally do not
want changes to CONFIG_RUST to result in any mitigations being changed
or turned off.

Link: https://github.com/rust-lang/rust/issues/121972 [1]
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
 Makefile            | 1 +
 arch/arm64/Makefile | 3 +++
 init/Kconfig        | 2 +-
 3 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 2b5f9f098b6f..66daca7a9b57 100644
--- a/Makefile
+++ b/Makefile
@@ -928,6 +928,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 914edf51b068..103957466cee 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
 	depends on !DEBUG_INFO_BTF || PAHOLE_HAS_LANG_EXCLUDE
 	help
 	  Enables Rust support in the kernel.

-- 
2.46.0.rc1.232.g9752f9e123-goog

Re: [PATCH v4 1/2] rust: SHADOW_CALL_STACK is incompatible with Rust

[Miguel Ojeda]

On Mon, Jul 29, 2024 at 4:23 PM Alice Ryhl <aliceryhl@google.com> wrote:
>
> When using the shadow call stack sanitizer, all code must be compiled
> with the -ffixed-x18 flag, but this flag is not currently being passed
> to Rust. This results in crashes that are extremely difficult to debug.
>
> To ensure that nobody else has to go through the same debugging session
> that I had to, prevent configurations that enable both SHADOW_CALL_STACK
> and RUST.
>
> It is rather common for people to backport 724a75ac9542 ("arm64: rust:
> Enable Rust support for AArch64"), so I recommend applying this fix all
> the way back to 6.1.

It should not hurt, and it is useful out-of-tree just in case -- if
arm64 is picking it up (which would be ideal):

Acked-by: Miguel Ojeda <ojeda@kernel.org>

Otherwise, please let us know. Thanks!

Cheers,
Miguel

Re: [PATCH v4 2/2] rust: support for shadow call stack sanitizer

[Miguel Ojeda]

On Mon, Jul 29, 2024 at 4:23 PM Alice Ryhl <aliceryhl@google.com> wrote:
>
> To use the shadow call stack sanitizer, you must pass special flags:

Just in case, if arm64 is picking patch #1 as a fix, please do not
pick this one -- it requires another series as mentioned in the cover
letter. We can do this one via `rust-next` if that is easier.

Thanks!

Cheers,
Miguel

Re: (subset) [PATCH v4 0/2] Rust and the shadow call stack sanitizer

[Catalin Marinas]

On Mon, 29 Jul 2024 14:22:48 +0000, Alice Ryhl wrote:
> This patch series makes it possible to use Rust together with the shadow
> call stack sanitizer. The first patch is intended to be backported to
> ensure that people don't try to use SCS with Rust on older kernel
> versions. The second patch makes it possible to use Rust with the shadow
> call stack sanitizer.
> 
> The second patch in this series depends on the next version of [1],
> which Miguel will send soon.
> 
> [...]

Applied to arm64 (for-next/fixes), thanks!

[1/2] rust: SHADOW_CALL_STACK is incompatible with Rust
      https://git.kernel.org/arm64/c/f126745da817

-- 
Catalin

Re: [PATCH v4 2/2] rust: support for shadow call stack sanitizer

[Will Deacon]

Hi Alice,

Just some minor comments on this:

On Mon, Jul 29, 2024 at 02:22:50PM +0000, Alice Ryhl wrote:
> To use the shadow call stack sanitizer, you must pass special flags:
> 
> * On arm64, you must pass -ffixed-x18 to your compiler.
> * On riscv, you must pass --no-relax-gp to your linker.

Since this patch doesn't touch riscv, I think you can just talk about
arm64 in the commit message.

> These requirements also apply to Rust code. When using Rust on arm64,
> you must pass the -Zfixed-x18 flag to rustc, which has the same effect
> as the -ffixed-x18 flag does for C code. The -Zfixed-x18 flag requires
> rustc version 1.80.0 or greater.
> 
> There is no need to pass any flags to rustc on riscv as only the linker
> requires additional flags on this platform.
> 
> On older versions of Rust, it is still possible to use shadow call stack
> by passing -Ctarget-feature=+reserve-x18 instead of -Zfixed-x18.
> However, this flag emits a warning during the build, so this patch does
> not add support for using it.
> 
> Currently, the compiler thinks that the aarch64-unknown-none target

"Currently" will probably age badly -- can you talk about a compiler
version instead (e.g. "prior to version nnn, the compiler thinks...").

> doesn't support -Zsanitizer=shadow-call-stack, so the build will fail if
> you enable shadow call stack in non-dynamic mode. See [1] for the
> relevant feature request. To avoid this compilation failure, Kconfig is
> set up to reject such configurations.
> 
> The `depends on` clause is placed on `config RUST` to avoid a situation
> where enabling Rust silently turns off the sanitizer. Instead, turning
> on the sanitizer results in Rust being disabled. We generally do not
> want changes to CONFIG_RUST to result in any mitigations being changed
> or turned off.
> 
> Link: https://github.com/rust-lang/rust/issues/121972 [1]
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---
>  Makefile            | 1 +
>  arch/arm64/Makefile | 3 +++
>  init/Kconfig        | 2 +-
>  3 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/Makefile b/Makefile
> index 2b5f9f098b6f..66daca7a9b57 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -928,6 +928,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

These unwind entries aren't mentioned at all in the commit message.
Please can you explain what you're doing here? I guess it's something to
do with the PAC patching? Maybe this hunk would be better as a separate
patch?

Will

Re: [PATCH v4 2/2] rust: support for shadow call stack sanitizer

[Alice Ryhl]

On Mon, Aug 5, 2024 at 7:13 PM Will Deacon <will@kernel.org> wrote:
>
> Hi Alice,
>
> Just some minor comments on this:
>
> On Mon, Jul 29, 2024 at 02:22:50PM +0000, Alice Ryhl wrote:
> > To use the shadow call stack sanitizer, you must pass special flags:
> >
> > * On arm64, you must pass -ffixed-x18 to your compiler.
> > * On riscv, you must pass --no-relax-gp to your linker.
>
> Since this patch doesn't touch riscv, I think you can just talk about
> arm64 in the commit message.

On the previous version, I was asked whether the patch should be
modified to only allow the configuration on arm64, since there were no
changes in the patch to support riscv. This part was added to justify
why the patch doesn't need to be arm64-specific. I will reword to make
it more clear why I am mentioning riscv.

> > These requirements also apply to Rust code. When using Rust on arm64,
> > you must pass the -Zfixed-x18 flag to rustc, which has the same effect
> > as the -ffixed-x18 flag does for C code. The -Zfixed-x18 flag requires
> > rustc version 1.80.0 or greater.
> >
> > There is no need to pass any flags to rustc on riscv as only the linker
> > requires additional flags on this platform.
> >
> > On older versions of Rust, it is still possible to use shadow call stack
> > by passing -Ctarget-feature=+reserve-x18 instead of -Zfixed-x18.
> > However, this flag emits a warning during the build, so this patch does
> > not add support for using it.
> >
> > Currently, the compiler thinks that the aarch64-unknown-none target
>
> "Currently" will probably age badly -- can you talk about a compiler
> version instead (e.g. "prior to version nnn, the compiler thinks...").

I will reword, but unfortunately I don't have a version number I can
put as it's still unfixed. (But we are actively working on it!)

> > doesn't support -Zsanitizer=shadow-call-stack, so the build will fail if
> > you enable shadow call stack in non-dynamic mode. See [1] for the
> > relevant feature request. To avoid this compilation failure, Kconfig is
> > set up to reject such configurations.
> >
> > The `depends on` clause is placed on `config RUST` to avoid a situation
> > where enabling Rust silently turns off the sanitizer. Instead, turning
> > on the sanitizer results in Rust being disabled. We generally do not
> > want changes to CONFIG_RUST to result in any mitigations being changed
> > or turned off.
> >
> > Link: https://github.com/rust-lang/rust/issues/121972 [1]
> > Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> > ---
> >  Makefile            | 1 +
> >  arch/arm64/Makefile | 3 +++
> >  init/Kconfig        | 2 +-
> >  3 files changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/Makefile b/Makefile
> > index 2b5f9f098b6f..66daca7a9b57 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -928,6 +928,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
>
> These unwind entries aren't mentioned at all in the commit message.
> Please can you explain what you're doing here? I guess it's something to
> do with the PAC patching? Maybe this hunk would be better as a separate
> patch?

It's because the PAC patching uses the unwind tables to find the
relevant places to patch. I will explain why in the commit message. If
you still think it needs to be separate after I add an explanation,
please let me know on the next version, which I will submit shortly.

Alice