Messages in this thread | | | Date | Tue, 9 Apr 2024 11:31:20 +0100 | From | Will Deacon <> | Subject | Re: [PATCH v2] rust: add flags for shadow call stack sanitizer |
| |
On Tue, Mar 05, 2024 at 01:14:19PM +0100, Miguel Ojeda wrote: > On Tue, Mar 5, 2024 at 12:58 PM Alice Ryhl <aliceryhl@google.com> wrote: > > > > Add flags to support the shadow call stack sanitizer, both in the > > dynamic and non-dynamic modes. > > > > Right now, the compiler will emit the warning "unknown feature specified > > for `-Ctarget-feature`: `reserve-x18`". However, the compiler still > > passes it to the codegen backend, so the flag will work just fine. Once > > rustc starts recognizing the flag (or provides another way to enable the > > feature), it will stop emitting this warning. See [1] for the relevant > > issue. > > > > 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. However, I still think > > it is reasonable to add the flag now, as it will at least fail the build > > when using an invalid configuration, until the Rust compiler is fixed to > > list -Zsanitizer=shadow-call-stack as supported for the target. See [2] > > for the feature request to add this. > > > > I have tested this change with Rust Binder on an Android device using > > CONFIG_DYNAMIC_SCS. Without the -Ctarget-feature=+reserve-x18 flag, the > > phone crashes immediately on boot, and with the flag, the phone appears > > to work normally. > > > > This contains a TODO to add the -Zuse-sync-unwind=n flag. The flag > > defaults to n, so it isn't a problem today, but the flag is unstable, so > > the default could change in a future compiler release. > > > > Link: https://github.com/rust-lang/rust/issues/121970 [1] > > Link: https://github.com/rust-lang/rust/issues/121972 [2] > > Signed-off-by: Alice Ryhl <aliceryhl@google.com> > > --- > > This patch raises the question of whether we should change the Rust > > aarch64 support to use a custom target.json specification. If we do > > that, then we can fix both the warning for dynamic SCS and the > > build-failure for non-dynamic SCS without waiting for a new version of > > rustc with the mentioned issues fixed. > > If the arm64 maintainers are OK with the warning being triggered in that case:
Sorry, I meant to reply on this at the time...
> Acked-by: Miguel Ojeda <ojeda@kernel.org> > > Otherwise partially reverting to the `target.json` approach sounds good too. > > I added the `-Zuse-sync-unwind=n` to the list at > https://github.com/Rust-for-Linux/linux/issues/2. Given the default is > what we want, I have put it in the "Good to have" section.
I think we have time to do this properly, like we did for the clang enablement a few years ago. In hindsight, avoiding hacks for the early toolchains back then was a really good idea because it meant we could rely on a solid baseline set of compiler features from the start.
So, please can we fix this in rustc and just have SCS dependent on that?
Cheers,
Will
| |