| Message ID | 20240729-shadow-call-stack-v4-2-2a664b082ea4@google.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Rust and the shadow call stack sanitizer | expand |
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
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
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
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.
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(-)