linux-kernel - Re: [PATCH v7] rust: support for shadow call stack sanitizer

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <c7c5a16b-d033-4b46-8994-259c55bfd8b1@gmail.com>
Date: Sun, 15 Sep 2024 09:32:44 +0200
From: Dirk Behme <dirk.behme@...il.com>
To: Alice Ryhl <aliceryhl@...gle.com>, Conor Dooley <conor@...nel.org>
Cc: Miguel Ojeda <miguel.ojeda.sandonis@...il.com>,
 Paul Walmsley <paul.walmsley@...ive.com>, Palmer Dabbelt
 <palmer@...belt.com>, Albert Ou <aou@...s.berkeley.edu>,
 Catalin Marinas <catalin.marinas@....com>, Will Deacon <will@...nel.org>,
 Ard Biesheuvel <ardb@...nel.org>, Jamie Cunliffe <Jamie.Cunliffe@....com>,
 Sami Tolvanen <samitolvanen@...gle.com>,
 Nathan Chancellor <nathan@...nel.org>, Kees Cook <kees@...nel.org>,
 Masahiro Yamada <masahiroy@...nel.org>, Nicolas Schier <nicolas@...sle.eu>,
 Marc Zyngier <maz@...nel.org>, Mark Rutland <mark.rutland@....com>,
 Mark Brown <broonie@...nel.org>, Nick Desaulniers <ndesaulniers@...gle.com>,
 Miguel Ojeda <ojeda@...nel.org>, Alex Gaynor <alex.gaynor@...il.com>,
 Wedson Almeida Filho <wedsonaf@...il.com>, Boqun Feng
 <boqun.feng@...il.com>, Gary Guo <gary@...yguo.net>,
 Björn Roy Baron <bjorn3_gh@...tonmail.com>,
 Benno Lossin <benno.lossin@...ton.me>,
 Andreas Hindborg <a.hindborg@...sung.com>,
 Valentin Obst <kernel@...entinobst.de>, linux-kbuild@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
 rust-for-linux@...r.kernel.org, linux-riscv <linux-riscv@...ts.infradead.org>
Subject: Re: [PATCH v7] rust: support for shadow call stack sanitizer

Am 13.09.24 um 23:44 schrieb Alice Ryhl:
> On Fri, Sep 13, 2024 at 11:18 PM Conor Dooley <conor@...nel.org> wrote:
>>
>> On Fri, Sep 13, 2024 at 12:08:20AM +0200, Miguel Ojeda wrote:
>>> On Thu, Aug 29, 2024 at 10:23 AM Alice Ryhl <aliceryhl@...gle.com> wrote:
>>>>
>>>> Add all of the flags that are needed to support the shadow call stack
>>>> (SCS) sanitizer with Rust, and updates Kconfig to allow only
>>>> configurations that work.
>>>
>>> Applied to `rust-next` -- thanks everyone!
>>>
>>> Paul/Palmer/Albert/RISC-V: I think you were not Cc'd (at least in this
>>> version?), so please shout if you have a problem with this.
>>
>> For some reason I deleted the series from my mailbox, must've been in
>> dt-binding review mode and hit ctrl + d. I've been away and busy, so my
>> apologies Alice for not trying this out sooner.
>> It's sorta annoying to test rust + scs on riscv, cos you need (unless I
>> am mistaken) llvm-19. llvm-18 + rust built fine, but has no SCS.
>>
>> llvm-19 + rust failed to build for me riscv, producing:
>>
>> In file included from /stuff/linux/rust/helpers/helpers.c:22:
>> /stuff/linux/rust/helpers/spinlock.c:10:23: error: call to undeclared function 'spinlock_check'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
>> __raw_spin_lock_init(spinlock_check(lock), name, key, LD_WAIT_CONFIG);
>> ^
>> /stuff/linux/rust/helpers/spinlock.c:10:23: error: incompatible integer to pointer conversion passing 'int' to parameter of type 'raw_spinlock_t *' (aka 'struct raw_spinlock *') [-Wint-conversion]
>> __raw_spin_lock_init(spinlock_check(lock), name, key, LD_WAIT_CONFIG);
>> ^~~~~~~~~~~~~~~~~~~~
>> /stuff/linux/include/linux/spinlock.h:101:52: note: passing argument to parameter 'lock' here
>> extern void __raw_spin_lock_init(raw_spinlock_t *lock, const char *name,
>> ^
>> 2 errors generated.
>>
>> This occurs because I have DEBUG_SPINLOCK enabled. I didn't check why,
>> but Andreas seems to have introduced that code - luckily he's already on
>> CC here :)
>>
>> With that disabled, there are dozens of warnings along the lines of:
>> /stuff/linux/rust/helpers/err.c:6:14: warning: symbol 'rust_helper_ERR_PTR' was not declared. Should it be static?
>> If those are okay for rust code, it would be rather helpful if the
>> warnings could be disabled - otherwise they should really be fixed.
>>
>> Following that, I got a build error:
>>
>> error[E0425]: cannot find function `__mutex_init` in crate `bindings`
>> --> /stuff/linux/rust/kernel/sync/lock/mutex.rs:104:28
>> |
>> 104   |           unsafe { bindings::__mutex_init(ptr, name, key) }
>> |                              ^^^^^^^^^^^^ help: a function with a similar name exists: `__mutex_rt_init`
>> |
>> ::: /stuff/brsdk/work/linux/rust/bindings/bindings_generated.rs:12907:5
>> |
>> 12907 | /     pub fn __mutex_rt_init(
>> 12908 | |         lock: *mut mutex,
>> 12909 | |         name: *const core::ffi::c_char,
>> 12910 | |         key: *mut lock_class_key,
>> 12911 | |     );
>> | |_____- similarly named function `__mutex_rt_init` defined here
>>
>> error: aborting due to 1 previous error
> 
> This looks like an unrelated problem to me. 


Yes, it is unrelated to this change. It is PREEMPT_RT usage related. I 
think we could add something like

#ifdef CONFIG_PREEMPT_RT
void rust_helper___mutex_init(struct mutex *mutex, const char *name,
			 struct lock_class_key *key)
{
	return __mutex_init(mutex, name, key);
}
#endif

to helpers to fix

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/rust/kernel/sync/lock/mutex.rs?&id=6d20d629c6d8575be98eeebe49a16fb2d7b32350

?

Explanation: Looking at

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/mutex.h?#n52

we have (simplified)

#ifndef CONFIG_PREEMPT_RT
extern void __mutex_init(struct mutex *lock, const char *name,
			 struct lock_class_key *key);
#else
#define __mutex_init(mutex, name, key)			\
do {							\
	rt_mutex_base_init(&(mutex)->rtmutex);		\
	__mutex_rt_init((mutex), name, key);		\
} while (0)
#endif

So in the CONFIG_PREEMPT_RT case bindgen doesn't resolve the macro 
what could be fixed by adding a helper.

Dirk


> This patch only changes
> the rustc flags, but these errors have to do with the Rust
> helpers/bindings, which get generated before the rustc flags are used
> at all. Most likely, there is a problem under the particular
> configuration you are using. Were you able to reproduce these errors
> without this patch?
> 
>> I stopped there, Space Marine 2 awaits.
>>
>> Hopefully I'll get to say hello next week,
>> Conor.
> 
> Thanks for taking a look, and see you at Plumbers!
> 
> Alice
>