Received-SPF: pass (google.com: domain of linux-kernel+bounces-91784-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Precedence: bulk
MIME-Version: 1.0
References: <CABCJKuem3GbLO-G7+wi8LPA8rFgNzFVjNof7zcAO1UGJR4u44Q@mail.gmail.com>
 <20240304233151.248925-1-kernel@valentinobst.de>
In-Reply-To: <20240304233151.248925-1-kernel@valentinobst.de>
From: Alice Ryhl <aliceryhl@google.com>
Date: Tue, 5 Mar 2024 08:15:42 +0100
Message-ID: <CAH5fLgg0yGbuHnMbMB103Zssg4KSfXUR3kvhr0kuqTSah=6kWg@mail.gmail.com>
Subject: Re: [PATCH] rust: add flags for shadow call stack sanitizer
To: Valentin Obst <kernel@valentinobst.de>
Cc: samitolvanen@google.com, Jamie.Cunliffe@arm.com, a.hindborg@samsung.com, 
	alex.gaynor@gmail.com, ardb@kernel.org, benno.lossin@proton.me, 
	bjorn3_gh@protonmail.com, boqun.feng@gmail.com, broonie@kernel.org, 
	catalin.marinas@arm.com, gary@garyguo.net, keescook@chromium.org, 
	linux-arm-kernel@lists.infradead.org, linux-kbuild@vger.kernel.org, 
	linux-kernel@vger.kernel.org, mark.rutland@arm.com, masahiroy@kernel.org, 
	maz@kernel.org, nathan@kernel.org, ndesaulniers@google.com, nicolas@fjasle.eu, 
	ojeda@kernel.org, rust-for-linux@vger.kernel.org, wedsonaf@gmail.com, 
	will@kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Tue, Mar 5, 2024 at 12:32=E2=80=AFAM Valentin Obst <kernel@valentinobst.=
de> 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 specif=
ied
> > > for `-Ctarget-feature`: `reserve-x18`". However, the compiler still
> > > passes it to the codegen backend, so the flag will work just fine. On=
ce
> > > rustc starts recognizing the flag (or provides another way to enable =
the
> > > feature), it will stop emitting this warning. See [1] for the relevan=
t
> > > issue.
> > >
> > > Currently, the compiler thinks that the aarch64-unknown-none target
> > > doesn't support -Zsanitizer=3Dshadow-call-stack, so the build will fa=
il if
> > > you enable shadow call stack in non-dynamic mode. However, I still th=
ink
> > > it is reasonable to add the flag now, as it will at least fail the bu=
ild
> > > when using an invalid configuration, until the Rust compiler is fixed=
 to
> > > list -Zsanitizer=3Dshadow-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=3D+reserve-x18 flag,=
 the
> > > phone crashes immediately on boot, and with the flag, the phone appea=
rs
> > > to work normally.
> > >
> > > 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>
> > > ---
> > > 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 compi=
le
> > > and work without actually applying SCS to the Rust code, please let m=
e
> > > 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 LKDT=
M?
> >
> > 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 d=
ynamic
>   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 syst=
em
> to emit the missing section for Rust modules.

I suppose the -Cforce-unwind-tables=3Dy flag will most likely do it.
There's also an use_sync_unwind option, but it defaults to no, so it
doesn't seem like we need to set it.

Alice