[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAH5fLggdR1UPNUa4R4CeJr+0cVRRr7cvyEQ3XhYaEqVD1cRtYA@mail.gmail.com>
Date: Tue, 5 Mar 2024 13:08:22 +0100
From: Alice Ryhl <aliceryhl@...gle.com>
To: Valentin Obst <kernel@...entinobst.de>
Cc: Jamie.Cunliffe@....com, a.hindborg@...sung.com, alex.gaynor@...il.com,
ardb@...nel.org, benno.lossin@...ton.me, bjorn3_gh@...tonmail.com,
boqun.feng@...il.com, broonie@...nel.org, catalin.marinas@....com,
gary@...yguo.net, keescook@...omium.org, linux-arm-kernel@...ts.infradead.org,
linux-kbuild@...r.kernel.org, linux-kernel@...r.kernel.org,
mark.rutland@....com, masahiroy@...nel.org, maz@...nel.org, nathan@...nel.org,
ndesaulniers@...gle.com, nicolas@...sle.eu, ojeda@...nel.org,
rust-for-linux@...r.kernel.org, samitolvanen@...gle.com, wedsonaf@...il.com,
will@...nel.org
Subject: Re: [PATCH] rust: add flags for shadow call stack sanitizer
On Tue, Mar 5, 2024 at 12:31 PM Alice Ryhl <aliceryhl@...gle.com> wrote:
>
> On Tue, Mar 5, 2024 at 12:28 PM Alice Ryhl <aliceryhl@...gle.com> wrote:
> >
> > On Tue, Mar 5, 2024 at 12:20 PM Valentin Obst <kernel@...entinobst.de> wrote:
> > >
> > > > >>> It's not 100% clear to me whether this patch is enough for full SCS
> > > > >>> support in Rust. If there is some issue where this makes things compile
> > > > >>> and work without actually applying SCS to the Rust code, please let me
> > > > >>> know. Is there some way to verify that it is actually working?
> > > > >>
> > > > >> Perhaps you could write a Rust version of the CFI_BACKWARD test in LKDTM?
> > > > >>
> > > > >> Alternatively, the simplest way to verify this is to look at the
> > > > >> disassembly and verify that shadow stack instructions are emitted to
> > > > >> Rust functions too. In case of dynamic SCS, you might need to dump
> > > > >> function memory in a debugger to verify that PAC instructions were
> > > > >> patched correctly. If they're not, the code will just quietly continue
> > > > >> working without using shadow stacks.
> > > > >
> > > > > Was just in the process of doing that:
> > > > >
> > > > > - `paciasp`/`autiasp` pairs are emitted for functions in Rust modules.
> > > > > - Rust modules have no `.init.eh_frame` section, which implies that
> > > > > `module_finalize` is _not_ rewriting the pac insns when SCS is dynamic.
> > > > > - Confirmed that behavior in the debugger (C modules and the C part of the
> > > > > kernel are correctly rewritten, Rust modules execute with
> > > > > `paciasp`/`autiasp` still in place).
> > > > > - Kernel boots just fine with Rust kunit tests, tested with and without dynamic
> > > > > SCS, i.e., on a CPU that supports PAC/BTI and one that does not.
> > > > > - Rust sample modules load and unload without problems as well.
> > > > > - `x18` is indeed not used in the codegen.
> > > > >
> > > > > I guess we might be able to get this working when we tweak the build system
> > > > > to emit the missing section for Rust modules.
> > > >
> > > > I suppose the -Cforce-unwind-tables=y flag will most likely do it.
> > >
> > > Yes, enabling this means that `.eh_frame` sections, which are converted to
> > > `.init.eh_frame` sections for loadable modules, are generated for Rust
> > > objects.
> > >
> > > Tested booting, kunit tests, sample modules (as builtin and loadable) for
> > > both, dynamic SCS active and inactive. Backtraces on Rust panicks also look
> > > normal.
> > >
> > > Confirmed that in the debugger that builtin and external modules are
> > > rewritten (or not rewritten if no dynamic SCS). Did not check that the
> > > `eh_frame` sections are exhaustive, i.e., cover all `paciasp`/`autiasp`
> > > pairs, only verified a few functions (in init text and normal text).
> >
> > Thank you for checking that!
> >
> > > > There's also an use_sync_unwind option, but it defaults to no, so it
> > > > doesn't seem like we need to set it.
> > >
> > > Are those defaults stable or will we notice if they change? If not it might
> > > make sense to set it explicitly anyways to avoid surprises in the future.
> >
> > The flag itself is unstable, so I imagine that nothing is promised about it.
> >
> > I tried it, but I wasn't actually able to find a way to set it. I can
> > see the flag in the rustc source code, but passing -Zuse-sync-unwind=n
> > results in "error: unknown unstable option: `use-sync-unwind`". Not
> > sure what the issue is.
>
> Oh, I understand now. It's really recent and not in 1.73.0, which is
> what I'm using in the Android build.
Sent v2 with what we have learned:
https://lore.kernel.org/rust-for-linux/20240305-shadow-call-stack-v2-1-c7b4a3f4d616@google.com/
Thanks!
Powered by blists - more mailing lists