Received-SPF: pass (google.com: domain of linux-kernel+bounces-92271-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
From: Valentin Obst <kernel@valentinobst.de>
To: aliceryhl@google.com
Cc: 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,
	kernel@valentinobst.de,
	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,
	samitolvanen@google.com,
	wedsonaf@gmail.com,
	will@kernel.org
Subject: Re: [PATCH] rust: add flags for shadow call stack sanitizer
Date: Tue,  5 Mar 2024 12:20:17 +0100
Message-ID: <20240305112017.125061-1-kernel@valentinobst.de>
In-Reply-To: <CAH5fLgg0yGbuHnMbMB103Zssg4KSfXUR3kvhr0kuqTSah=6kWg@mail.gmail.com>
References: <CAH5fLgg0yGbuHnMbMB103Zssg4KSfXUR3kvhr0kuqTSah=6kWg@mail.gmail.com>
Precedence: bulk
MIME-Version: 1.0
UI-OutboundReport: notjunk:1;M01:P0:vedfcIpbmPY=;dZuHOCTtIcJA3uYMlVFkIktOqig
 WT8LPNbCsxSxVCGp2kyR8w2z9GBJsIBfW2JhKFiTeMPbNqhaeBp+aQwHLayiDjVKuCAGd/OAS
 lhpjdAjPhDDvt1F73nZ//+LEL1loUfl2DrImh3PR6xAEPN1CSueOWYG9SCqFuTJv5XMXEdJZ9
 kU+nZ0pZuWkw1MEY0TpS8UcXK4VArFEQ66JXzykqDSZvxDSmLJ6NhQqb4ns6BvuTrkEP3EEp1
 4MUX7YGo/IYFrW4W+VWGubYlFNwvx8S260h4bzq5Tg0dNnHGlV8tI9JBTVo04lTqhXj9WZQWA
 BeSK1vicUOGSGuMmzX6zrMzbs84VikgL6mRszQ+vUpqMavmqla8vNRwJkEVM8gm+aE8ntJ5eq
 CeeeiLZ11z5MMn1NRfBFQCwFmVCXPYH3/6ehf8UISd8L7LjjPDN0cWvta/VJNx+LcMQkpKLYu
 vCHDWUZMWIJ7QvPXl9zxGgtB1mbrnc6phBSHZUArdcJrpZqk4VrtjKyhilotfE9jhuw464HGV
 q+tbJUIifmVzwijpNRJJ/RfubK6GvV7nigG1c2RGVWG7KNDrFnZ/F0l7xv3UYvAC4GFOomWhY
 HBbPBfmv1hZB9FK2VH3fe9xV+tg+0Pw1Pmr8YDldRdCVr0wjcf0P6ynOBKv9Zzcie6VTU51TM
 57iFh+BQHKgGdCcfU01Dm+b8eSKiO1AIdUgWF3Ku7Mjz1OFQl4n9PK3MGniULBX397nd2jL0V
 x+Xauh1zQOIXGjlb0xTFF/uWOb4vioPj5BOFir7oLemipa9aRG4Y4s=

> >>> 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).

> 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.

    - Best Valentin

>
> Alice
>
>