From ece207a83e464af710d641f29e32b7a144c48e79 Mon Sep 17 00:00:00 2001 From: Miguel Ojeda Date: Thu, 26 Sep 2024 14:47:51 +0200 Subject: [PATCH 1/8] rust: kernel: sort Rust modules Rust modules are intended to be sorted, thus do so. This makes `rustfmtcheck` to pass again. Fixes: 570172569238 ("Merge tag 'rust-6.12' of https://github.com/Rust-for-Linux/linux") Reviewed-by: Alice Ryhl Link: https://lore.kernel.org/r/20240926124751.345471-1-ojeda@kernel.org Signed-off-by: Miguel Ojeda --- rust/kernel/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs index 22a3bfa5a9e9..b5f4b3ce6b48 100644 --- a/rust/kernel/lib.rs +++ b/rust/kernel/lib.rs @@ -44,8 +44,8 @@ pub mod net; pub mod page; pub mod prelude; pub mod print; -pub mod sizes; pub mod rbtree; +pub mod sizes; mod static_assert; #[doc(hidden)] pub mod std_vendor; From a8ee30f45d5d57467ddb7877ed6914d0eba0af7f Mon Sep 17 00:00:00 2001 From: Alice Ryhl Date: Sun, 15 Sep 2024 14:41:28 +0000 Subject: [PATCH 2/8] rust: sync: require `T: Sync` for `LockedBy::access` The `LockedBy::access` method only requires a shared reference to the owner, so if we have shared access to the `LockedBy` from several threads at once, then two threads could call `access` in parallel and both obtain a shared reference to the inner value. Thus, require that `T: Sync` when calling the `access` method. An alternative is to require `T: Sync` in the `impl Sync for LockedBy`. This patch does not choose that approach as it gives up the ability to use `LockedBy` with `!Sync` types, which is okay as long as you only use `access_mut`. Cc: stable@vger.kernel.org Fixes: 7b1f55e3a984 ("rust: sync: introduce `LockedBy`") Signed-off-by: Alice Ryhl Suggested-by: Boqun Feng Reviewed-by: Gary Guo Link: https://lore.kernel.org/r/20240915-locked-by-sync-fix-v2-1-1a8d89710392@google.com Signed-off-by: Miguel Ojeda --- rust/kernel/sync/locked_by.rs | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/rust/kernel/sync/locked_by.rs b/rust/kernel/sync/locked_by.rs index babc731bd5f6..ce2ee8d87865 100644 --- a/rust/kernel/sync/locked_by.rs +++ b/rust/kernel/sync/locked_by.rs @@ -83,8 +83,12 @@ pub struct LockedBy { // SAFETY: `LockedBy` can be transferred across thread boundaries iff the data it protects can. unsafe impl Send for LockedBy {} -// SAFETY: `LockedBy` serialises the interior mutability it provides, so it is `Sync` as long as the -// data it protects is `Send`. +// SAFETY: If `T` is not `Sync`, then parallel shared access to this `LockedBy` allows you to use +// `access_mut` to hand out `&mut T` on one thread at the time. The requirement that `T: Send` is +// sufficient to allow that. +// +// If `T` is `Sync`, then the `access` method also becomes available, which allows you to obtain +// several `&T` from several threads at once. However, this is okay as `T` is `Sync`. unsafe impl Sync for LockedBy {} impl LockedBy { @@ -118,7 +122,10 @@ impl LockedBy { /// /// Panics if `owner` is different from the data protected by the lock used in /// [`new`](LockedBy::new). - pub fn access<'a>(&'a self, owner: &'a U) -> &'a T { + pub fn access<'a>(&'a self, owner: &'a U) -> &'a T + where + T: Sync, + { build_assert!( size_of::() > 0, "`U` cannot be a ZST because `owner` wouldn't be unique" @@ -127,7 +134,10 @@ impl LockedBy { panic!("mismatched owners"); } - // SAFETY: `owner` is evidence that the owner is locked. + // SAFETY: `owner` is evidence that there are only shared references to the owner for the + // duration of 'a, so it's not possible to use `Self::access_mut` to obtain a mutable + // reference to the inner value that aliases with this shared reference. The type is `Sync` + // so there are no other requirements. unsafe { &*self.data.get() } } From 732cd686cdd60f9d8c36edac2a14d8f5eea57ee1 Mon Sep 17 00:00:00 2001 From: Gary Guo Date: Mon, 16 Sep 2024 01:33:46 +0100 Subject: [PATCH 3/8] rust: fix `ARCH_SLAB_MINALIGN` multiple definition error We use const helpers in form of const size_t RUST_CONST_HELPER_ARCH_SLAB_MINALIGN = ARCH_SLAB_MINALIGN; to aid generation of constants by bindgen because it is otherwise a macro definition of an expression and bindgen doesn't expand the constant. The helpers are then have `RUST_CONST_HELPER` prefix stripped and exposed to Rust code as if `ARCH_SLAB_MISALIGN` is generated natively by bindgen. This works well for most constants, but on RISC-V, `ARCH_SLAB_MINALIGN` is defined directly as literal constant if `!CONFIG_MMU`, and bindgen would generate `ARCH_SLAB_MINALIGN` directly, thus conflict with the one generated through the helper. To fix this, we simply need to block bindgen from generating directly without going through helper. Reported-by: kernel test robot Closes: https://lore.kernel.org/oe-kbuild-all/202409160804.eSg9zh1e-lkp@intel.com/ Signed-off-by: Gary Guo Reviewed-by: Christian Schrefl Link: https://lore.kernel.org/r/20240916003347.1744345-1-gary@garyguo.net Signed-off-by: Miguel Ojeda --- rust/bindgen_parameters | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/rust/bindgen_parameters b/rust/bindgen_parameters index a721d466bee4..b7c7483123b7 100644 --- a/rust/bindgen_parameters +++ b/rust/bindgen_parameters @@ -24,3 +24,8 @@ # These functions use the `__preserve_most` calling convention, which neither bindgen # nor Rust currently understand, and which Clang currently declares to be unstable. --blocklist-function __list_.*_report + +# These constants are sometimes not recognized by bindgen depending on config. +# We use const helpers to aid bindgen, to avoid conflicts when constants are +# recognized, block generation of the non-helper constants. +--blocklist-item ARCH_SLAB_MINALIGN From d065cc76054d21e48a839a2a19ba99dbc51a4d11 Mon Sep 17 00:00:00 2001 From: Dirk Behme Date: Mon, 16 Sep 2024 09:37:52 +0200 Subject: [PATCH 4/8] rust: mutex: fix __mutex_init() usage in case of PREEMPT_RT In case CONFIG_PREEMPT_RT is enabled __mutex_init() becomes a macro instead of an extern function (simplified from include/linux/mutex.h): #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 The macro isn't resolved by bindgen, then. What results in a build error: error[E0425]: cannot find function `__mutex_init` in crate `bindings` --> 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` | ::: rust/bindings/bindings_generated.rs:23722:5 | 23722 | / pub fn __mutex_rt_init( 23723 | | lock: *mut mutex, 23724 | | name: *const core::ffi::c_char, 23725 | | key: *mut lock_class_key, 23726 | | ); | |_____- similarly named function `__mutex_rt_init` defined here Fix this by adding a helper. As explained by Gary Guo in [1] no #ifdef CONFIG_PREEMPT_RT is needed here as rust/bindings/lib.rs prefers externed function to helpers if an externed function exists. Reported-by: Conor Dooley Link: https://lore.kernel.org/rust-for-linux/20240913-shack-estate-b376a65921b1@spud/ Link: https://lore.kernel.org/rust-for-linux/20240915123626.1a170103.gary@garyguo.net/ [1] Fixes: 6d20d629c6d8 ("rust: lock: introduce `Mutex`") Signed-off-by: Dirk Behme Tested-by: Conor Dooley Reviewed-by: Gary Guo Link: https://lore.kernel.org/r/20240916073752.3123484-1-dirk.behme@de.bosch.com [ Reworded to include the proper example by Dirk. - Miguel ] Signed-off-by: Miguel Ojeda --- rust/helpers/mutex.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/rust/helpers/mutex.c b/rust/helpers/mutex.c index 200db7e6279f..a17ca8cdb50c 100644 --- a/rust/helpers/mutex.c +++ b/rust/helpers/mutex.c @@ -7,3 +7,9 @@ void rust_helper_mutex_lock(struct mutex *lock) { mutex_lock(lock); } + +void rust_helper___mutex_init(struct mutex *mutex, const char *name, + struct lock_class_key *key) +{ + __mutex_init(mutex, name, key); +} From af6017b6a315e9102582afb92640221d057f84f6 Mon Sep 17 00:00:00 2001 From: Miguel Ojeda Date: Wed, 25 Sep 2024 16:19:44 +0200 Subject: [PATCH 5/8] rust: cfi: fix `patchable-function-entry` starting version The `-Zpatchable-function-entry` flag is available since Rust 1.81.0, not Rust 1.80.0, i.e. commit ac7595fdb1ee ("Support for -Z patchable-function-entry") in upstream Rust. Fixes: ca627e636551 ("rust: cfi: add support for CFI_CLANG with Rust") Reviewed-by: Alice Ryhl Reviewed-by: Fiona Behrens Link: https://lore.kernel.org/r/20240925141944.277936-1-ojeda@kernel.org Signed-off-by: Miguel Ojeda --- init/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/init/Kconfig b/init/Kconfig index fbd0cb06a50a..fa734a144589 100644 --- a/init/Kconfig +++ b/init/Kconfig @@ -1948,7 +1948,7 @@ config RUST depends on !DEBUG_INFO_BTF || PAHOLE_HAS_LANG_EXCLUDE depends on !CFI_CLANG || RUSTC_VERSION >= 107900 && $(cc-option,-fsanitize=kcfi -fsanitize-cfi-icall-experimental-normalize-integers) select CFI_ICALL_NORMALIZE_INTEGERS if CFI_CLANG - depends on !CALL_PADDING || RUSTC_VERSION >= 108000 + depends on !CALL_PADDING || RUSTC_VERSION >= 108100 depends on !KASAN_SW_TAGS help Enables Rust support in the kernel. From 93e34a0b5c0e79ce765f01fd10f7817863fba23d Mon Sep 17 00:00:00 2001 From: Alice Ryhl Date: Thu, 26 Sep 2024 09:38:49 +0000 Subject: [PATCH 6/8] rust: KASAN+RETHUNK requires rustc 1.83.0 When enabling both KASAN and RETHUNK, objtool emits the following warnings: rust/core.o: warning: objtool: asan.module_ctor+0x13: 'naked' return found in MITIGATION_RETHUNK build rust/core.o: warning: objtool: asan.module_dtor+0x13: 'naked' return found in MITIGATION_RETHUNK build This is caused by the -Zfunction-return=thunk-extern flag in rustc not informing LLVM about the mitigation at the module level (it does so at the function level only currently, which covers most cases, but both are required), which means that the KASAN functions asan.module_ctor and asan.module_dtor are generated without the rethunk mitigation. The other mitigations that we enabled for Rust (SLS, RETPOLINE) do not have the same bug, as they're being applied through the target-feature functionality instead. This is being fixed for rustc 1.83.0, so update Kconfig to reject this configuration on older compilers. Link: https://github.com/rust-lang/rust/pull/130824 Fixes: d7868550d573 ("x86/rust: support MITIGATION_RETHUNK") Reported-by: Miguel Ojeda Closes: https://lore.kernel.org/all/CANiq72myZL4_poCMuNFevtpYYc0V0embjSuKb7y=C+m3vVA_8g@mail.gmail.com/ Signed-off-by: Alice Ryhl Link: https://lore.kernel.org/r/20240926093849.1192264-1-aliceryhl@google.com [ Reworded to add the details mentioned in the list. - Miguel ] Signed-off-by: Miguel Ojeda --- init/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/init/Kconfig b/init/Kconfig index fa734a144589..9a2bcba426c5 100644 --- a/init/Kconfig +++ b/init/Kconfig @@ -1950,6 +1950,7 @@ config RUST select CFI_ICALL_NORMALIZE_INTEGERS if CFI_CLANG depends on !CALL_PADDING || RUSTC_VERSION >= 108100 depends on !KASAN_SW_TAGS + depends on !(MITIGATION_RETHUNK && KASAN) || RUSTC_VERSION >= 108300 help Enables Rust support in the kernel. From 4c66f8307ac099f89038878b7789d72163a74751 Mon Sep 17 00:00:00 2001 From: Alice Ryhl Date: Wed, 25 Sep 2024 08:10:18 +0000 Subject: [PATCH 7/8] cfi: encode cfi normalized integers + kasan/gcov bug in Kconfig There is a bug in the LLVM implementation of KASAN and GCOV that makes these options incompatible with the CFI_ICALL_NORMALIZE_INTEGERS option. The bug has already been fixed in llvm/clang [1] and rustc [2]. However, Kconfig currently has no way to gate features on the LLVM version inside rustc, so we cannot write down a precise `depends on` clause in this case. Instead, a `def_bool` option is defined for whether CFI_ICALL_NORMALIZE_INTEGERS is available, and its default value is set to false when GCOV or KASAN are turned on. End users using a patched clang/rustc can turn on the HAVE_CFI_ICALL_NORMALIZE_INTEGERS option directly to override this. An alternative solution is to inspect a binary created by clang or rustc to see whether the faulty CFI tags are in the binary. This would be a precise check, but it would involve hard-coding the *hashed* version of the CFI tag. This is because there's no way to get clang or rustc to output the unhased version of the CFI tag. Relying on the precise hashing algorithm using by CFI seems too fragile, so I have not pursued this option. Besides, this kind of hack is exactly what lead to the LLVM bug in the first place. If the CFI_ICALL_NORMALIZE_INTEGERS option is used without CONFIG_RUST, then we actually can perform a precise check today: just compare the clang version number. This works since clang and llvm are always updated in lockstep. However, encoding this in Kconfig would give the HAVE_CFI_ICALL_NORMALIZE_INTEGERS option a dependency on CONFIG_RUST, which is not possible as the reverse dependency already exists. HAVE_CFI_ICALL_NORMALIZE_INTEGERS is defined to be a `def_bool` instead of `bool` to avoid asking end users whether they want to turn on the option. Turning it on explicitly is something only experts should do, so making it hard to do so is not an issue. I added a `depends on CFI_CLANG` clause to the new Kconfig option. I'm not sure whether that makes sense or not, but it doesn't seem to make a big difference. In a future kernel release, I would like to add a Kconfig option similar to CLANG_VERSION/RUSTC_VERSION for inspecting the version of the LLVM inside rustc. Once that feature lands, this logic will be replaced with a precise version check. This check is not being introduced here to avoid introducing a new _VERSION constant in a fix. Link: https://github.com/llvm/llvm-project/pull/104826 [1] Link: https://github.com/rust-lang/rust/pull/129373 [2] Fixes: ce4a2620985c ("cfi: add CONFIG_CFI_ICALL_NORMALIZE_INTEGERS") Reported-by: kernel test robot Closes: https://lore.kernel.org/oe-lkp/202409231044.4f064459-oliver.sang@intel.com Signed-off-by: Alice Ryhl Reviewed-by: Sami Tolvanen Link: https://lore.kernel.org/r/20240925-cfi-norm-kasan-fix-v1-1-0328985cdf33@google.com Signed-off-by: Miguel Ojeda --- arch/Kconfig | 18 +++++++++++++++++- init/Kconfig | 2 +- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/arch/Kconfig b/arch/Kconfig index 98157b38f5cf..8af374ea1adc 100644 --- a/arch/Kconfig +++ b/arch/Kconfig @@ -838,7 +838,7 @@ config CFI_CLANG config CFI_ICALL_NORMALIZE_INTEGERS bool "Normalize CFI tags for integers" depends on CFI_CLANG - depends on $(cc-option,-fsanitize=kcfi -fsanitize-cfi-icall-experimental-normalize-integers) + depends on HAVE_CFI_ICALL_NORMALIZE_INTEGERS help This option normalizes the CFI tags for integer types so that all integer types of the same size and signedness receive the same CFI @@ -851,6 +851,22 @@ config CFI_ICALL_NORMALIZE_INTEGERS This option is necessary for using CFI with Rust. If unsure, say N. +config HAVE_CFI_ICALL_NORMALIZE_INTEGERS + def_bool !GCOV_KERNEL && !KASAN + depends on CFI_CLANG + depends on $(cc-option,-fsanitize=kcfi -fsanitize-cfi-icall-experimental-normalize-integers) + help + Is CFI_ICALL_NORMALIZE_INTEGERS supported with the set of compilers + currently in use? + + This option defaults to false if GCOV or KASAN is enabled, as there is + an LLVM bug that makes normalized integers tags incompatible with + KASAN and GCOV. Kconfig currently does not have the infrastructure to + detect whether your rustc compiler contains the fix for this bug, so + it is assumed that it doesn't. If your compiler has the fix, you can + explicitly enable this option in your config file. The Kconfig logic + needed to detect this will be added in a future kernel release. + config CFI_PERMISSIVE bool "Use CFI in permissive mode" depends on CFI_CLANG diff --git a/init/Kconfig b/init/Kconfig index 9a2bcba426c5..530a382ee0fe 100644 --- a/init/Kconfig +++ b/init/Kconfig @@ -1946,7 +1946,7 @@ config RUST depends on !GCC_PLUGIN_RANDSTRUCT depends on !RANDSTRUCT depends on !DEBUG_INFO_BTF || PAHOLE_HAS_LANG_EXCLUDE - depends on !CFI_CLANG || RUSTC_VERSION >= 107900 && $(cc-option,-fsanitize=kcfi -fsanitize-cfi-icall-experimental-normalize-integers) + depends on !CFI_CLANG || RUSTC_VERSION >= 107900 && HAVE_CFI_ICALL_NORMALIZE_INTEGERS select CFI_ICALL_NORMALIZE_INTEGERS if CFI_CLANG depends on !CALL_PADDING || RUSTC_VERSION >= 108100 depends on !KASAN_SW_TAGS From 05cef2c4a421ca09ab9761a95e61423e59e5bfb1 Mon Sep 17 00:00:00 2001 From: Miguel Ojeda Date: Fri, 27 Sep 2024 18:44:14 +0200 Subject: [PATCH 8/8] rust: kunit: use C-string literals to clean warning Starting with upstream Rust commit a5e3a3f9b6bd ("move `manual_c_str_literals` to complexity"), to be released in Rust 1.83.0 [1], Clippy now warns on `manual_c_str_literals` by default, e.g.: error: manually constructing a nul-terminated string --> rust/kernel/kunit.rs:21:13 | 21 | b"\x013%pA\0".as_ptr() as _, | ^^^^^^^^^^^^^ help: use a `c""` literal: `c"\x013%pA"` | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#manual_c_str_literals = note: `-D clippy::manual-c-str-literals` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(clippy::manual_c_str_literals)]` Apply the suggestion to clean up the warnings. Link: https://github.com/rust-lang/rust-clippy/pull/13263 [1] Reviewed-by: Trevor Gross Reviewed-by: Benno Lossin Link: https://lore.kernel.org/r/20240927164414.560906-1-ojeda@kernel.org Signed-off-by: Miguel Ojeda --- rust/kernel/kunit.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rust/kernel/kunit.rs b/rust/kernel/kunit.rs index 0ba77276ae7e..824da0e9738a 100644 --- a/rust/kernel/kunit.rs +++ b/rust/kernel/kunit.rs @@ -18,7 +18,7 @@ pub fn err(args: fmt::Arguments<'_>) { #[cfg(CONFIG_PRINTK)] unsafe { bindings::_printk( - b"\x013%pA\0".as_ptr() as _, + c"\x013%pA".as_ptr() as _, &args as *const _ as *const c_void, ); } @@ -34,7 +34,7 @@ pub fn info(args: fmt::Arguments<'_>) { #[cfg(CONFIG_PRINTK)] unsafe { bindings::_printk( - b"\x016%pA\0".as_ptr() as _, + c"\x016%pA".as_ptr() as _, &args as *const _ as *const c_void, ); }