From b8673d56935c32a4e0a1a0b40951fdd313dbf340 Mon Sep 17 00:00:00 2001 From: Andreas Hindborg Date: Thu, 15 Aug 2024 07:49:30 +0000 Subject: rust: kbuild: fix export of bss symbols Symbols in the bss segment are not currently exported. This is a problem for Rust modules that link against statics, that are resident in the kernel image. Thus export symbols in the bss segment. Fixes: 2f7ab1267dc9 ("Kbuild: add Rust support") Signed-off-by: Andreas Hindborg Reviewed-by: Alice Ryhl Tested-by: Alice Ryhl Reviewed-by: Gary Guo Link: https://lore.kernel.org/r/20240815074519.2684107-2-nmi@metaspace.dk [ Reworded slightly. - Miguel ] Signed-off-by: Miguel Ojeda --- rust/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'rust') diff --git a/rust/Makefile b/rust/Makefile index 8de3ebba9551..f168d2c98a15 100644 --- a/rust/Makefile +++ b/rust/Makefile @@ -305,7 +305,7 @@ $(obj)/bindings/bindings_helpers_generated.rs: $(src)/helpers.c FORCE quiet_cmd_exports = EXPORTS $@ cmd_exports = \ $(NM) -p --defined-only $< \ - | awk '/ (T|R|D) / {printf "EXPORT_SYMBOL_RUST_GPL(%s);\n",$$3}' > $@ + | awk '/ (T|R|D|B) / {printf "EXPORT_SYMBOL_RUST_GPL(%s);\n",$$3}' > $@ $(obj)/exports_core_generated.h: $(obj)/core.o FORCE $(call if_changed,exports) -- cgit From fd764e74e5b75512be1b55ec9680a6c35885cc63 Mon Sep 17 00:00:00 2001 From: Andreas Hindborg Date: Thu, 15 Aug 2024 07:49:43 +0000 Subject: rust: block: fix wrong usage of lockdep API When allocating `struct gendisk`, `GenDiskBuilder` is using a dynamic lock class key without registering the key. This is an incorrect use of the API, which causes a `WARN` trace. Fix the issue by using a static lock class key, which is more appropriate for the situation anyway. Fixes: 3253aba3408a ("rust: block: introduce `kernel::block::mq` module") Reported-by: Behme Dirk (XC-CP/ESB5) Closes: https://rust-for-linux.zulipchat.com/#narrow/stream/x/topic/x/near/457090036 Signed-off-by: Andreas Hindborg Reviewed-by: Benno Lossin Reviewed-by: Gary Guo Reviewed-by: Alice Ryhl Tested-by: Dirk Behme Link: https://lore.kernel.org/r/20240815074519.2684107-3-nmi@metaspace.dk [ Applied `rustfmt`, reworded slightly and made Zulip link a permalink. - Miguel ] Signed-off-by: Miguel Ojeda --- rust/kernel/block/mq/gen_disk.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) (limited to 'rust') diff --git a/rust/kernel/block/mq/gen_disk.rs b/rust/kernel/block/mq/gen_disk.rs index f548a6199847..708125dce96a 100644 --- a/rust/kernel/block/mq/gen_disk.rs +++ b/rust/kernel/block/mq/gen_disk.rs @@ -6,8 +6,8 @@ //! C header: [`include/linux/blk_mq.h`](srctree/include/linux/blk_mq.h) use crate::block::mq::{raw_writer::RawWriter, Operations, TagSet}; -use crate::error; use crate::{bindings, error::from_err_ptr, error::Result, sync::Arc}; +use crate::{error, static_lock_class}; use core::fmt::{self, Write}; /// A builder for [`GenDisk`]. @@ -93,8 +93,6 @@ impl GenDiskBuilder { name: fmt::Arguments<'_>, tagset: Arc>, ) -> Result> { - let lock_class_key = crate::sync::LockClassKey::new(); - // SAFETY: `bindings::queue_limits` contain only fields that are valid when zeroed. let mut lim: bindings::queue_limits = unsafe { core::mem::zeroed() }; @@ -110,7 +108,7 @@ impl GenDiskBuilder { tagset.raw_tag_set(), &mut lim, core::ptr::null_mut(), - lock_class_key.as_ptr(), + static_lock_class!().as_ptr(), ) })?; -- cgit From 0ff8f3f0979559b0d7494d580f2597beab3f159b Mon Sep 17 00:00:00 2001 From: Michael Vetter Date: Mon, 19 Aug 2024 22:57:31 +0200 Subject: rust: kernel: fix typos in code comments Fix spelling mistakes in code comments. Signed-off-by: Michael Vetter Reviewed-by: Alice Ryhl Reviewed-by: Benno Lossin Link: https://lore.kernel.org/r/20240819205731.2163-1-jubalh@iodoru.org [ Reworded slightly. - Miguel ] Signed-off-by: Miguel Ojeda --- rust/kernel/init/macros.rs | 4 ++-- rust/kernel/net/phy.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) (limited to 'rust') diff --git a/rust/kernel/init/macros.rs b/rust/kernel/init/macros.rs index 02ecedc4ae7a..9a0c4650ef67 100644 --- a/rust/kernel/init/macros.rs +++ b/rust/kernel/init/macros.rs @@ -145,7 +145,7 @@ //! } //! } //! // Implement the internal `PinData` trait that marks the pin-data struct as a pin-data -//! // struct. This is important to ensure that no user can implement a rouge `__pin_data` +//! // struct. This is important to ensure that no user can implement a rogue `__pin_data` //! // function without using `unsafe`. //! unsafe impl ::kernel::init::__internal::PinData for __ThePinData { //! type Datee = Bar; @@ -156,7 +156,7 @@ //! // case no such fields exist, hence this is almost empty. The two phantomdata fields exist //! // for two reasons: //! // - `__phantom`: every generic must be used, since we cannot really know which generics -//! // are used, we declere all and then use everything here once. +//! // are used, we declare all and then use everything here once. //! // - `__phantom_pin`: uses the `'__pin` lifetime and ensures that this struct is invariant //! // over it. The lifetime is needed to work around the limitation that trait bounds must //! // not be trivial, e.g. the user has a `#[pin] PhantomPinned` field -- this is diff --git a/rust/kernel/net/phy.rs b/rust/kernel/net/phy.rs index fd40b703d224..91dac63ffa17 100644 --- a/rust/kernel/net/phy.rs +++ b/rust/kernel/net/phy.rs @@ -491,7 +491,7 @@ impl Adapter { pub struct DriverVTable(Opaque); // SAFETY: `DriverVTable` doesn't expose any &self method to access internal data, so it's safe to -// share `&DriverVTable` across execution context boundries. +// share `&DriverVTable` across execution context boundaries. unsafe impl Sync for DriverVTable {} /// Creates a [`DriverVTable`] instance from [`Driver`]. -- cgit From 0903b9e2a46cb6252a13d6b19d0502da9be191cf Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Thu, 22 Aug 2024 22:03:59 -0700 Subject: rust: alloc: eschew `Box>::write` Upstream Rust's libs-api team has consensus for stabilizing some of `feature(new_uninit)`, but not for `Box>::write`. Instead, we can use `MaybeUninit::write`, so Rust for Linux can drop the feature after stabilization. That will happen after merging, as the FCP has completed [1]. This is required before stabilization because remaining-unstable API will be divided into new features. This code doesn't know about those yet. It can't: they haven't landed, as the relevant PR is blocked on rustc's CI testing Rust-for-Linux without this patch. [ The PR has landed [2] and will be released in Rust 1.82.0 (expected on 2024-10-17), so we could conditionally enable the new unstable feature (`box_uninit_write` [3]) instead, but just for a single `unsafe` block it is probably not worth it. For the time being, I added it to the "nice to have" section of our unstable features list. - Miguel ] Link: https://github.com/rust-lang/rust/issues/63291#issuecomment-2183022955 [1] Link: https://github.com/rust-lang/rust/pull/129416 [2] Link: https://github.com/rust-lang/rust/issues/129397 [3] Signed-off-by: Jubilee Young Reviewed-by: Alice Ryhl Reviewed-by: Trevor Gross [ Reworded slightly. - Miguel ] Signed-off-by: Miguel Ojeda --- rust/kernel/alloc/box_ext.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'rust') diff --git a/rust/kernel/alloc/box_ext.rs b/rust/kernel/alloc/box_ext.rs index 829cb1c1cf9e..9f1c1c489189 100644 --- a/rust/kernel/alloc/box_ext.rs +++ b/rust/kernel/alloc/box_ext.rs @@ -21,8 +21,10 @@ pub trait BoxExt: Sized { impl BoxExt for Box { fn new(x: T, flags: Flags) -> Result { - let b = >::new_uninit(flags)?; - Ok(Box::write(b, x)) + let mut b = >::new_uninit(flags)?; + b.write(x); + // SAFETY: We just wrote to it. + Ok(unsafe { b.assume_init() }) } #[cfg(any(test, testlib))] -- cgit From a5a3c952e82c1ada12bf8c55b73af26f1a454bd2 Mon Sep 17 00:00:00 2001 From: Boqun Feng Date: Wed, 28 Aug 2024 11:01:29 -0700 Subject: rust: macros: provide correct provenance when constructing THIS_MODULE Currently while defining `THIS_MODULE` symbol in `module!()`, the pointer used to construct `ThisModule` is derived from an immutable reference of `__this_module`, which means the pointer doesn't have the provenance for writing, and that means any write to that pointer is UB regardless of data races or not. However, the usage of `THIS_MODULE` includes passing this pointer to functions that may write to it (probably in unsafe code), and this will create soundness issues. One way to fix this is using `addr_of_mut!()` but that requires the unstable feature "const_mut_refs". So instead of `addr_of_mut()!`, an extern static `Opaque` is used here: since `Opaque` is transparent to `T`, an extern static `Opaque` will just wrap the C symbol (defined in a C compile unit) in an `Opaque`, which provides a pointer with writable provenance via `Opaque::get()`. This fix the potential UBs because of pointer provenance unmatched. Reported-by: Alice Ryhl Signed-off-by: Boqun Feng Reviewed-by: Alice Ryhl Reviewed-by: Trevor Gross Reviewed-by: Benno Lossin Reviewed-by: Gary Guo Closes: https://rust-for-linux.zulipchat.com/#narrow/stream/x/topic/x/near/465412664 Fixes: 1fbde52bde73 ("rust: add `macros` crate") Cc: stable@vger.kernel.org # 6.6.x: be2ca1e03965: ("rust: types: Make Opaque::get const") Link: https://lore.kernel.org/r/20240828180129.4046355-1-boqun.feng@gmail.com [ Fixed two typos, reworded title. - Miguel ] Signed-off-by: Miguel Ojeda --- rust/macros/module.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'rust') diff --git a/rust/macros/module.rs b/rust/macros/module.rs index 411dc103d82e..7a5b899e47b7 100644 --- a/rust/macros/module.rs +++ b/rust/macros/module.rs @@ -217,7 +217,11 @@ pub(crate) fn module(ts: TokenStream) -> TokenStream { // freed until the module is unloaded. #[cfg(MODULE)] static THIS_MODULE: kernel::ThisModule = unsafe {{ - kernel::ThisModule::from_ptr(&kernel::bindings::__this_module as *const _ as *mut _) + extern \"C\" {{ + static __this_module: kernel::types::Opaque; + }} + + kernel::ThisModule::from_ptr(__this_module.get()) }}; #[cfg(not(MODULE))] static THIS_MODULE: kernel::ThisModule = unsafe {{ -- cgit