* Re: [PATCH] KEYS: replace -EEXIST with -EBUSY
From: Lucas De Marchi @ 2026-01-06 15:04 UTC (permalink / raw)
To: David Howells
Cc: Daniel Gomez, Lukas Wunner, Ignat Korchagin, Herbert Xu,
David S. Miller, Jarkko Sakkinen, Paul Moore, James Morris,
Serge E. Hallyn, Luis Chamberlain, Petr Pavlu, Sami Tolvanen,
Aaron Tomlin, keyrings, linux-crypto, linux-modules, linux-kernel,
linux-security-module, Daniel Gomez, Greg Kroah-Hartman
In-Reply-To: <1793804.1767607035@warthog.procyon.org.uk>
+Greg, to consolidate from the othe thread.
On Mon, Jan 05, 2026 at 09:57:15AM +0000, David Howells wrote:
>Daniel Gomez <da.gomez@kernel.org> wrote:
>
>> From: Daniel Gomez <da.gomez@samsung.com>
>>
>> The -EEXIST error code is reserved by the module loading infrastructure
>> to indicate that a module is already loaded.
>
>EEXIST means a file exists when you're trying to create it. Granted we abuse
>that somewhat rather than add ever more error codes, but you cannot reserve it
>for indicating that a module exists.
EEXIST from [f]init_module() means "module is already loaded" and it
can't mean something else for this syscall. Other return codes are
explained in the man page, but aren't that special from the userspace
pov.
This doesn't mean we need to replace all the EBUSY throughout the call
chain with EEXIST, but the return from the syscall needs to remain
consistent if that was the case for it failing. Ideally that mapping
would come from the module init (and not from other functions it calls)
because that is the place that has that knowledge.
If a generic EBUSY->EEXIST mapping is desired, as it seems to be the
case from
https://lore.kernel.org/all/2025122212-fiction-setback-ede5@gregkh/,
then do_init_module() can do it, but in practice that means reserving 2
error codes rather than 1.
>
>> When a module's init
>> function returns -EEXIST, userspace tools like kmod interpret this as
>> "module already loaded" and treat the operation as successful, returning
>> 0 to the user even though the module initialization actually failed.
>>
>> This follows the precedent set by commit 54416fd76770 ("netfilter:
>> conntrack: helper: Replace -EEXIST by -EBUSY") which fixed the same
>> issue in nf_conntrack_helper_register().
>>
>> Affected modules:
>> * pkcs8_key_parser x509_key_parser asymmetric_keys dns_resolver
>> * nvme_keyring pkcs7_test_key rxrpc turris_signing_key
>>
>> Signed-off-by: Daniel Gomez <da.gomez@samsung.com>
>
>Please don't. Userspace can always check /proc/modules (assuming procfs is
>enabled, I suppose).
EEXIST is already there with that meaning. Checking procfs (or sysfs as
kmod currently does) is racy and doesn't look like a good API - why
would userspace have to check if the module is loaded when the syscall
that loads the module failed? EEXIST is special exactly to resolve races
with 2 threads trying to load the same module.
Lucas De Marchi
>
>David
>
^ permalink raw reply
* [PATCH RFC v2 00/11] rust: Reimplement ThisModule to fix ownership problems
From: Kari Argillander @ 2026-01-06 16:11 UTC (permalink / raw)
To: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Alexandre Courbot
Cc: Greg Kroah-Hartman, rust-for-linux, linux-kernel, linux-modules,
Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
Aaron Tomlin, Kari Argillander, Youseok Yang, Yuheng Su
Still RFC. Not all people for each subsystems are not included yet as
this touch quite lot of things. Introducing new THIS_MODULE and
ThisModule still change configfs in same patch which is not ideal.
So currently we have problem that we are not always filling .owner field
for file_operations. I think we can enable const_refs_to_static already
as that is in 1.78 and is stable in 1.83. So that fits perfecly for us.
This also seems to be quite request feature but I did not found that no
one has ever suggested that we just enable this.
So basic idea is that we will have ThisModule trait which is used kernel
side. Module side we will always use THIS_MODULE. That is completly
private for modules and kernel cannot use it. So this unifies ways of
using cofing ThisModule things. Currently we have THIS_MODULE and also
module: &' static ThisModule
on init functions. As we anyway need THIS_MODULE just use that.
Argillander
To: Miguel Ojeda <ojeda@kernel.org>
To: Boqun Feng <boqun.feng@gmail.com>
To: Gary Guo <gary@garyguo.net>
To: Björn Roy Baron <bjorn3_gh@protonmail.com>
To: Benno Lossin <lossin@kernel.org>
To: Andreas Hindborg <a.hindborg@kernel.org>
To: Alice Ryhl <aliceryhl@google.com>
To: Trevor Gross <tmgross@umich.edu>
To: Danilo Krummrich <dakr@kernel.org>
To: Alexandre Courbot <acourbot@nvidia.com>
Cc: Luis Chamberlain <mcgrof@kernel.org>
Cc: Petr Pavlu <petr.pavlu@suse.com>
Cc: Daniel Gomez <da.gomez@kernel.org>
Cc: Sami Tolvanen <samitolvanen@google.com>
Cc: Aaron Tomlin <atomlin@atomlin.com>
Signed-off-by: Kari Argillander <kari.argillander@gmail.com>
---
Changes in v2:
- Patches are now sepereted properly.
- Removed debugfs changes as that is not so clear to me.
- Remove module parameter and just used THIS_MODULE everywhere.
- Made macro to make THIS_MODULE.
- Doc tests also have THIS_MODULE.
- Link to v1: https://lore.kernel.org/r/20260101-this_module_fix-v1-0-46ae3e5605a0@gmail.com
---
Kari Argillander (11):
rust: enable const_refs_to_static feature
rust: add new ThisModule trait and THIS_MODULE impl
rust: miscdevice: fix use after free because missing .owner
rust: block: fix missing owner field in block_device_operations
rust: drm: fix missing owner in file_operations
rust: driver: make RegistrationOps::register() to use new ThisModule
rust: phy: make Registration::register() use new ThisModule
rust: binder: use new THIS_MODULE
rust: remove module argument from InPlaceModule::init()
rust: remove kernel::ModuleMetadata
rust: remove old version of ThisModule
drivers/android/binder/rust_binder_main.rs | 5 +-
drivers/block/rnull/configfs.rs | 2 +-
drivers/block/rnull/rnull.rs | 3 +-
drivers/gpu/drm/nova/driver.rs | 2 +
drivers/gpu/drm/tyr/driver.rs | 2 +
drivers/gpu/nova-core/nova_core.rs | 2 +-
lib/find_bit_benchmark_rust.rs | 3 +-
rust/kernel/auxiliary.rs | 16 +--
rust/kernel/block/mq.rs | 1 +
rust/kernel/block/mq/gen_disk.rs | 30 +-----
rust/kernel/block/mq/operations.rs | 30 ++++++
rust/kernel/configfs.rs | 49 ++++-----
rust/kernel/driver.rs | 31 +++---
rust/kernel/drm/device.rs | 2 +-
rust/kernel/drm/driver.rs | 4 +
rust/kernel/drm/gem/mod.rs | 5 +-
rust/kernel/firmware.rs | 4 +-
rust/kernel/i2c.rs | 11 +-
rust/kernel/lib.rs | 161 ++++++++++++++++++++++++-----
rust/kernel/miscdevice.rs | 5 +
rust/kernel/net/phy.rs | 29 ++++--
rust/kernel/pci.rs | 15 +--
rust/kernel/platform.rs | 12 +--
rust/kernel/prelude.rs | 2 +-
rust/kernel/sync/lock/global.rs | 4 +-
rust/kernel/usb.rs | 13 +--
rust/macros/lib.rs | 4 +-
rust/macros/module.rs | 24 +----
samples/rust/rust_configfs.rs | 2 +-
samples/rust/rust_debugfs_scoped.rs | 2 +-
samples/rust/rust_driver_auxiliary.rs | 8 +-
samples/rust/rust_driver_faux.rs | 2 +-
samples/rust/rust_minimal.rs | 2 +-
samples/rust/rust_misc_device.rs | 3 +-
samples/rust/rust_print_main.rs | 2 +-
scripts/rustdoc_test_gen.rs | 2 +
36 files changed, 298 insertions(+), 196 deletions(-)
---
base-commit: 6cd6c12031130a349a098dbeb19d8c3070d2dfbe
change-id: 20251230-this_module_fix-a390bff24897
Best regards,
--
Kari Argillander <kari.argillander@gmail.com>
^ permalink raw reply
* [PATCH RFC v2 01/11] rust: enable const_refs_to_static feature
From: Kari Argillander @ 2026-01-06 16:11 UTC (permalink / raw)
To: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Alexandre Courbot
Cc: Greg Kroah-Hartman, rust-for-linux, linux-kernel, linux-modules,
Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
Aaron Tomlin, Kari Argillander
In-Reply-To: <20260106-this_module_fix-v2-0-842ac026f00b@gmail.com>
Enable the const_refs_to_static Rust feature to allow taking
references to static items in const contexts. This is required for
using ThisModule when constructing static Rust structures.
The Rust support already relies on features available in Rust 1.83, and
const_refs_to_static has been available since Rust 1.78.
Signed-off-by: Kari Argillander <kari.argillander@gmail.com>
---
rust/kernel/lib.rs | 1 +
1 file changed, 1 insertion(+)
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 6d637e2fed1b..510d4bfc7c2b 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -36,6 +36,7 @@
#![feature(const_option)]
#![feature(const_ptr_write)]
#![feature(const_refs_to_cell)]
+#![feature(const_refs_to_static)]
//
// Expected to become stable.
#![feature(arbitrary_self_types)]
--
2.43.0
^ permalink raw reply related
* [PATCH RFC v2 02/11] rust: add new ThisModule trait and THIS_MODULE impl
From: Kari Argillander @ 2026-01-06 16:11 UTC (permalink / raw)
To: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Alexandre Courbot
Cc: Greg Kroah-Hartman, rust-for-linux, linux-kernel, linux-modules,
Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
Aaron Tomlin, Kari Argillander
In-Reply-To: <20260106-this_module_fix-v2-0-842ac026f00b@gmail.com>
To make clear separation between module crates and kernel crate we
introduce ThisModule trait which is meant to be used by kernel space.
THIS_MODULE is meant to be used by modules. So kernel create will be
unable to even accidentally use THIS_MODULE.
As ThisModule is trait we can pass that around in const context. This is
needed so that we can read ownership information in const context when
we create example file_operations structs for modules.
New ThisModule will also eventually replace kernel::ModuleMetadata trait
and for this reason it also have NAME field.
To make transition smooth use mod this_module so we can have two
ThisModule same time. Also some functionality is added to THIS_MODULE
temporarily so that we do not have to change everything at once.
Also examples will need THIS_MODULE so also define that in docs.
Signed-off-by: Kari Argillander <kari.argillander@gmail.com>
---
drivers/block/rnull/configfs.rs | 2 +-
rust/kernel/configfs.rs | 46 ++++++------
rust/kernel/lib.rs | 159 ++++++++++++++++++++++++++++++++++++++++
rust/macros/module.rs | 16 +---
scripts/rustdoc_test_gen.rs | 2 +
5 files changed, 188 insertions(+), 37 deletions(-)
diff --git a/drivers/block/rnull/configfs.rs b/drivers/block/rnull/configfs.rs
index 2f5a7da03af5..7223ee7c3032 100644
--- a/drivers/block/rnull/configfs.rs
+++ b/drivers/block/rnull/configfs.rs
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: GPL-2.0
-use super::{NullBlkDevice, THIS_MODULE};
+use super::NullBlkDevice;
use kernel::{
block::mq::gen_disk::{GenDisk, GenDiskBuilder},
c_str,
diff --git a/rust/kernel/configfs.rs b/rust/kernel/configfs.rs
index 466fb7f40762..908cb98d404f 100644
--- a/rust/kernel/configfs.rs
+++ b/rust/kernel/configfs.rs
@@ -110,16 +110,21 @@
//! [C documentation]: srctree/Documentation/filesystems/configfs.rst
//! [`rust_configfs.rs`]: srctree/samples/rust/rust_configfs.rs
-use crate::alloc::flags;
-use crate::container_of;
-use crate::page::PAGE_SIZE;
-use crate::prelude::*;
-use crate::str::CString;
-use crate::sync::Arc;
-use crate::sync::ArcBorrow;
-use crate::types::Opaque;
-use core::cell::UnsafeCell;
-use core::marker::PhantomData;
+use crate::{
+ alloc::flags,
+ container_of,
+ page::PAGE_SIZE,
+ prelude::*,
+ str::CString,
+ sync::Arc,
+ sync::ArcBorrow,
+ this_module::ThisModule,
+ types::Opaque, //
+};
+use core::{
+ cell::UnsafeCell,
+ marker::PhantomData, //
+};
/// A configfs subsystem.
///
@@ -744,8 +749,7 @@ macro_rules! impl_item_type {
($tpe:ty) => {
impl<Data> ItemType<$tpe, Data> {
#[doc(hidden)]
- pub const fn new_with_child_ctor<const N: usize, Child>(
- owner: &'static ThisModule,
+ pub const fn new_with_child_ctor<const N: usize, Child, TM: ThisModule>(
attributes: &'static AttributeList<N, Data>,
) -> Self
where
@@ -754,7 +758,7 @@ pub const fn new_with_child_ctor<const N: usize, Child>(
{
Self {
item_type: Opaque::new(bindings::config_item_type {
- ct_owner: owner.as_ptr(),
+ ct_owner: TM::OWNER.as_ptr(),
ct_group_ops: GroupOperationsVTable::<Data, Child>::vtable_ptr().cast_mut(),
ct_item_ops: ItemOperationsVTable::<$tpe, Data>::vtable_ptr().cast_mut(),
ct_attrs: core::ptr::from_ref(attributes).cast_mut().cast(),
@@ -765,13 +769,12 @@ pub const fn new_with_child_ctor<const N: usize, Child>(
}
#[doc(hidden)]
- pub const fn new<const N: usize>(
- owner: &'static ThisModule,
+ pub const fn new<const N: usize, TM: ThisModule>(
attributes: &'static AttributeList<N, Data>,
) -> Self {
Self {
item_type: Opaque::new(bindings::config_item_type {
- ct_owner: owner.as_ptr(),
+ ct_owner: TM::OWNER.as_ptr(),
ct_group_ops: core::ptr::null_mut(),
ct_item_ops: ItemOperationsVTable::<$tpe, Data>::vtable_ptr().cast_mut(),
ct_attrs: core::ptr::from_ref(attributes).cast_mut().cast(),
@@ -875,8 +878,7 @@ fn as_ptr(&self) -> *const bindings::config_item_type {
/// = kernel::configfs::ItemType::<
/// configfs::Subsystem<Configuration>,
/// Configuration
-/// >::new_with_child_ctor::<N,Child>(
-/// &THIS_MODULE,
+/// >::new_with_child_ctor::<N, Child, crate::THIS_MODULE>(
/// &CONFIGURATION_ATTRS
/// );
///
@@ -1019,8 +1021,8 @@ macro_rules! configfs_attrs {
const [<$no_child:upper>]: bool = true;
static [< $data:upper _TPE >] : $crate::configfs::ItemType<$container, $data> =
- $crate::configfs::ItemType::<$container, $data>::new::<N>(
- &THIS_MODULE, &[<$ data:upper _ATTRS >]
+ $crate::configfs::ItemType::<$container, $data>::new::<N, crate::THIS_MODULE>(
+ &[<$ data:upper _ATTRS >]
);
)?
@@ -1028,8 +1030,8 @@ macro_rules! configfs_attrs {
static [< $data:upper _TPE >]:
$crate::configfs::ItemType<$container, $data> =
$crate::configfs::ItemType::<$container, $data>::
- new_with_child_ctor::<N, $child>(
- &THIS_MODULE, &[<$ data:upper _ATTRS >]
+ new_with_child_ctor::<N, $child, crate::THIS_MODULE>(
+ &[<$ data:upper _ATTRS >]
);
)?
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 510d4bfc7c2b..2ccd75f68f03 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -233,6 +233,165 @@ pub const fn as_ptr(&self) -> *mut bindings::module {
}
}
+pub mod this_module {
+ //! Access to the module identity and ownership information.
+ //!
+ //! This module provides the Rust equivalent of the kernel’s `THIS_MODULE`
+ //! symbol from the [C API](srctree/include/linux/init.h).
+ //!
+ //! # For driver creators
+ //!
+ //! If you see ThisModule you need to pass THIS_NODULE for it so it can
+ //! track module ownership.
+ //!
+ //! Each Rust module defines its own `THIS_MODULE` using the
+ //! [`create_this_module`] macro. The generated `THIS_MODULE` identifies the
+ //! owning kernel module and expose some metadata about it.
+ //!
+ //! # For abstraction creators
+ //!
+ //! Many times C-apis expect a `struct module *` pointer so they can
+ //! increase the module reference count. This is because module could be
+ //! unloaded while example file operations are in progress. Many times
+ //! structs which needs owner fields should also be const. For this reason
+ //! ThisModule is usually passes as a type parameter `TM` to abstractions
+ //! which need to know the module owner. In vtables ThisModule is usually
+ //! used as name.
+ //!
+ //! ## Example
+ //!
+ //! ```
+ //! # use kernel::{bindings, this_module::ThisModule};
+ //! # use core::marker::PhantomData;
+ //!
+ //! // Example function signature which needs ThisModule.
+ //! pub fn create_device<TM: ThisModule>() {}
+ //!
+ //! // Example of a vtable which uses ThisModule.
+ //! #[vtable]
+ //! pub trait MyStruct {
+ //! type ThisModule: ThisModule;
+ //! }
+ //!
+ //! pub(crate) struct MyStructVTable<T: MyStruct>(PhantomData<T>);
+ //!
+ //! impl<T: MyStruct> MyStructVTable<T> {
+ //! const FOPS: bindings::file_operations = bindings::file_operations {
+ //! owner: T::ThisModule::OWNER.as_ptr(),
+ //! ..pin_init::zeroed()
+ //! };
+ //! }
+ //! ```
+
+ /// See [`this_module`]
+ pub trait ThisModule {
+ /// Wrapper around the owning `struct module` pointer.
+ ///
+ /// This is null for built-in code and non-null for loadable modules.
+ const OWNER: ModuleWrapper;
+ /// Name of the module.
+ const NAME: &'static kernel::str::CStr;
+ }
+
+ /// Wrapper around a pointer to `struct module`.
+ ///
+ /// This type exists as a workaround for the lack of `const fn` methods in
+ /// traits. It allows the module pointer to be stored as an associated
+ /// constant while still providing a `const` accessor.
+ pub struct ModuleWrapper {
+ ptr: *mut bindings::module,
+ }
+
+ impl ModuleWrapper {
+ /// Get the raw pointer to the underlying `struct module`.
+ ///
+ /// TODO: Should be only available for kernel create.
+ pub const fn as_ptr(&self) -> *mut bindings::module {
+ self.ptr
+ }
+
+ /// Only meant to be used from [`create_this_module`].
+ ///
+ /// # Safety
+ ///
+ /// - Only modules are allowed to create non null `ModuleWrapper`s.
+ /// - The non null pointer must point to a valid `struct module`
+ /// provided by the kernel.
+ #[doc(hidden)]
+ pub const unsafe fn from_ptr(ptr: *mut bindings::module) -> Self {
+ ModuleWrapper { ptr }
+ }
+ }
+
+ /// Creates the `THIS_MODULE` definition for a Rust module.
+ ///
+ /// This macro is an internal building block and is not intended to be used
+ /// directly by module authors. It is invoked by [`macros::module::module`]
+ /// and by kernel doctests.
+ ///
+ /// A macro is required so that `cfg(MODULE)` is evaluated in the context of
+ /// the consuming crate, and to prevent accidental use of THIS_MODULE from
+ /// within the kernel crate itself.
+ #[macro_export]
+ #[doc(hidden)]
+ macro_rules! create_this_module {
+ ($name:literal) => {
+ /// THIS_MODULE for module `{name}`. See [`kernel::this_module`].
+ #[allow(non_camel_case_types)]
+ pub struct THIS_MODULE;
+
+ impl ::kernel::this_module::ThisModule for THIS_MODULE {
+ #[cfg(not(MODULE))]
+ /// SAFETY: TODO
+ const OWNER: ::kernel::this_module::ModuleWrapper = unsafe {
+ ::kernel::this_module::ModuleWrapper::from_ptr(::core::ptr::null_mut())
+ };
+
+ #[cfg(MODULE)]
+ // SAFETY:
+ // - `__this_module` is constructed by the kernel at module load time.
+ const OWNER: ::kernel::this_module::ModuleWrapper = unsafe {
+ extern "C" {
+ static __this_module: ::kernel::types::Opaque<::kernel::bindings::module>;
+ }
+
+ ::kernel::this_module::ModuleWrapper::from_ptr(__this_module.get())
+ };
+
+ const NAME: &'static ::kernel::str::CStr = $crate::c_str!($name);
+ }
+
+ impl THIS_MODULE {
+ /// Returns the name of this module.
+ pub const fn name() -> &'static ::kernel::str::CStr {
+ $crate::c_str!($name)
+ }
+
+ // TODO: Temporary to provide functionality old `THIS_MODULE` provided.
+ // SAFETY: `__this_module` is constructed by the kernel at load time and
+ // will not be freed until the module is unloaded.
+ const ThisModule: ::kernel::ThisModule = unsafe {{
+ ::kernel::ThisModule::from_ptr(
+ <Self as ::kernel::this_module::ThisModule>::OWNER.as_ptr()
+ )
+ }};
+
+ /// Gets a pointer to the underlying `struct module`.
+ // TODO: Temporary to provide functionality old `THIS_MODULE` provided.
+ pub const fn as_ptr(&self) -> *mut ::kernel::bindings::module {{
+ Self::ThisModule.as_ptr()
+ }}
+
+ /// Gets a reference to the underlying `ThisModule`.
+ /// TODO: Temporary to provide functionality old `THIS_MODULE` provided.
+ pub const fn as_ref(&self) -> &'static ::kernel::ThisModule {{
+ &Self::ThisModule
+ }}
+ }
+ };
+ }
+}
+
#[cfg(not(testlib))]
#[panic_handler]
fn panic(info: &core::panic::PanicInfo<'_>) -> ! {
diff --git a/rust/macros/module.rs b/rust/macros/module.rs
index 80cb9b16f5aa..1bcd703735fe 100644
--- a/rust/macros/module.rs
+++ b/rust/macros/module.rs
@@ -371,20 +371,8 @@ pub(crate) fn module(ts: TokenStream) -> TokenStream {
/// Used by the printing macros, e.g. [`info!`].
const __LOG_PREFIX: &[u8] = b\"{name}\\0\";
- // SAFETY: `__this_module` is constructed by the kernel at load time and will not be
- // freed until the module is unloaded.
- #[cfg(MODULE)]
- static THIS_MODULE: ::kernel::ThisModule = unsafe {{
- extern \"C\" {{
- static __this_module: ::kernel::types::Opaque<::kernel::bindings::module>;
- }}
+ ::kernel::create_this_module!(\"{name}\");
- ::kernel::ThisModule::from_ptr(__this_module.get())
- }};
- #[cfg(not(MODULE))]
- static THIS_MODULE: ::kernel::ThisModule = unsafe {{
- ::kernel::ThisModule::from_ptr(::core::ptr::null_mut())
- }};
/// The `LocalModule` type is the type of the module created by `module!`,
/// `module_pci_driver!`, `module_platform_driver!`, etc.
@@ -502,7 +490,7 @@ mod __module_init {{
/// This function must only be called once.
unsafe fn __init() -> ::kernel::ffi::c_int {{
let initer =
- <{type_} as ::kernel::InPlaceModule>::init(&super::super::THIS_MODULE);
+ <{type_} as ::kernel::InPlaceModule>::init(&super::super::THIS_MODULE.as_ref());
// SAFETY: No data race, since `__MOD` can only be accessed by this module
// and there only `__init` and `__exit` access it. These functions are only
// called once and `__exit` cannot be called before or during `__init`.
diff --git a/scripts/rustdoc_test_gen.rs b/scripts/rustdoc_test_gen.rs
index 6fd9f5c84e2e..089e38b49cdd 100644
--- a/scripts/rustdoc_test_gen.rs
+++ b/scripts/rustdoc_test_gen.rs
@@ -232,6 +232,8 @@ macro_rules! assert_eq {{
const __LOG_PREFIX: &[u8] = b"rust_doctests_kernel\0";
+::kernel::create_this_module!("rust_doctests_kernel");
+
{rust_tests}
"#
)
--
2.43.0
^ permalink raw reply related
* [PATCH RFC v2 03/11] rust: miscdevice: fix use after free because missing .owner
From: Kari Argillander @ 2026-01-06 16:11 UTC (permalink / raw)
To: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Alexandre Courbot
Cc: Greg Kroah-Hartman, rust-for-linux, linux-kernel, linux-modules,
Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
Aaron Tomlin, Kari Argillander, Youseok Yang
In-Reply-To: <20260106-this_module_fix-v2-0-842ac026f00b@gmail.com>
Currently if miscdevice driver is compiled as module it can cause use
after free when unloading. To reproduce problem with Rust sample driver
we can do:
tail -f /dev/rust-misc-device
# And same time as device is open
sudo rmmod rust_misc_device_module
This will crash system. Fix is to have .owner field filled with module
information. We pass this owner information through vtable.
Reported-by: Youseok Yang <ileixe@gmail.com>
Closes: https://github.com/Rust-for-Linux/linux/issues/1182
Fixes: f893691e7426 ("rust: miscdevice: add base miscdevice abstraction")
Signed-off-by: Kari Argillander <kari.argillander@gmail.com>
---
rust/kernel/miscdevice.rs | 5 +++++
samples/rust/rust_misc_device.rs | 1 +
2 files changed, 6 insertions(+)
diff --git a/rust/kernel/miscdevice.rs b/rust/kernel/miscdevice.rs
index ba64c8a858f0..d4b0c35c4b60 100644
--- a/rust/kernel/miscdevice.rs
+++ b/rust/kernel/miscdevice.rs
@@ -18,6 +18,7 @@
mm::virt::VmaNew,
prelude::*,
seq_file::SeqFile,
+ this_module::ThisModule,
types::{ForeignOwnable, Opaque},
};
use core::{marker::PhantomData, pin::Pin};
@@ -112,6 +113,9 @@ fn drop(self: Pin<&mut Self>) {
/// Trait implemented by the private data of an open misc device.
#[vtable]
pub trait MiscDevice: Sized {
+ /// Module ownership for this device, provided via `THIS_MODULE`.
+ type ThisModule: ThisModule;
+
/// What kind of pointer should `Self` be wrapped in.
type Ptr: ForeignOwnable + Send + Sync;
@@ -388,6 +392,7 @@ impl<T: MiscDevice> MiscdeviceVTable<T> {
}
const VTABLE: bindings::file_operations = bindings::file_operations {
+ owner: T::ThisModule::OWNER.as_ptr(),
open: Some(Self::open),
release: Some(Self::release),
mmap: if T::HAS_MMAP { Some(Self::mmap) } else { None },
diff --git a/samples/rust/rust_misc_device.rs b/samples/rust/rust_misc_device.rs
index 49dd5814e1ab..464e3026e6e3 100644
--- a/samples/rust/rust_misc_device.rs
+++ b/samples/rust/rust_misc_device.rs
@@ -155,6 +155,7 @@ struct RustMiscDevice {
#[vtable]
impl MiscDevice for RustMiscDevice {
+ type ThisModule = THIS_MODULE;
type Ptr = Pin<KBox<Self>>;
fn open(_file: &File, misc: &MiscDeviceRegistration<Self>) -> Result<Pin<KBox<Self>>> {
--
2.43.0
^ permalink raw reply related
* [PATCH RFC v2 04/11] rust: block: fix missing owner field in block_device_operations
From: Kari Argillander @ 2026-01-06 16:11 UTC (permalink / raw)
To: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Alexandre Courbot
Cc: Greg Kroah-Hartman, rust-for-linux, linux-kernel, linux-modules,
Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
Aaron Tomlin, Kari Argillander
In-Reply-To: <20260106-this_module_fix-v2-0-842ac026f00b@gmail.com>
Kernel has now enabled "const_refs_to_static" feature. We can fix TODO
item now. Fix this by defining owner in vtable so we can read it from
there. As this table needs to be const we need to define it in
operations so we do not need pass THIS_MODULE alongside with
GenDiskBuilder::build().
This will probably fix some use after free.
Fixes: 3253aba3408a ("rust: block: introduce `kernel::block::mq` module")
Signed-off-by: Kari Argillander <kari.argillander@gmail.com>
---
drivers/block/rnull/rnull.rs | 1 +
rust/kernel/block/mq.rs | 1 +
rust/kernel/block/mq/gen_disk.rs | 30 ++++--------------------------
rust/kernel/block/mq/operations.rs | 30 ++++++++++++++++++++++++++++++
4 files changed, 36 insertions(+), 26 deletions(-)
diff --git a/drivers/block/rnull/rnull.rs b/drivers/block/rnull/rnull.rs
index a9d5e575a2c4..862369ab9b5c 100644
--- a/drivers/block/rnull/rnull.rs
+++ b/drivers/block/rnull/rnull.rs
@@ -74,6 +74,7 @@ struct QueueData {
#[vtable]
impl Operations for NullBlkDevice {
+ type ThisModule = THIS_MODULE;
type QueueData = KBox<QueueData>;
#[inline(always)]
diff --git a/rust/kernel/block/mq.rs b/rust/kernel/block/mq.rs
index 1fd0d54dd549..0c8e9e316952 100644
--- a/rust/kernel/block/mq.rs
+++ b/rust/kernel/block/mq.rs
@@ -68,6 +68,7 @@
//!
//! #[vtable]
//! impl Operations for MyBlkDevice {
+//! type ThisModule = THIS_MODULE;
//! type QueueData = ();
//!
//! fn queue_rq(_queue_data: (), rq: ARef<Request<Self>>, _is_last: bool) -> Result {
diff --git a/rust/kernel/block/mq/gen_disk.rs b/rust/kernel/block/mq/gen_disk.rs
index 1ce815c8cdab..4d5d378577ec 100644
--- a/rust/kernel/block/mq/gen_disk.rs
+++ b/rust/kernel/block/mq/gen_disk.rs
@@ -7,7 +7,7 @@
use crate::{
bindings,
- block::mq::{Operations, TagSet},
+ block::mq::{operations::OperationsVTable, Operations, TagSet},
error::{self, from_err_ptr, Result},
fmt::{self, Write},
prelude::*,
@@ -126,32 +126,10 @@ pub fn build<T: Operations>(
)
})?;
- const TABLE: bindings::block_device_operations = bindings::block_device_operations {
- submit_bio: None,
- open: None,
- release: None,
- ioctl: None,
- compat_ioctl: None,
- check_events: None,
- unlock_native_capacity: None,
- getgeo: None,
- set_read_only: None,
- swap_slot_free_notify: None,
- report_zones: None,
- devnode: None,
- alternative_gpt_sector: None,
- get_unique_id: None,
- // TODO: Set to THIS_MODULE. Waiting for const_refs_to_static feature to
- // be merged (unstable in rustc 1.78 which is staged for linux 6.10)
- // <https://github.com/rust-lang/rust/issues/119618>
- owner: core::ptr::null_mut(),
- pr_ops: core::ptr::null_mut(),
- free_disk: None,
- poll_bio: None,
- };
-
// SAFETY: `gendisk` is a valid pointer as we initialized it above
- unsafe { (*gendisk).fops = &TABLE };
+ unsafe {
+ (*gendisk).fops = OperationsVTable::<T>::build_block_device_operations();
+ }
let mut writer = NullTerminatedFormatter::new(
// SAFETY: `gendisk` points to a valid and initialized instance. We
diff --git a/rust/kernel/block/mq/operations.rs b/rust/kernel/block/mq/operations.rs
index 8ad46129a52c..0f8f616590fb 100644
--- a/rust/kernel/block/mq/operations.rs
+++ b/rust/kernel/block/mq/operations.rs
@@ -10,6 +10,7 @@
error::{from_result, Result},
prelude::*,
sync::{aref::ARef, Refcount},
+ this_module::ThisModule,
types::ForeignOwnable,
};
use core::marker::PhantomData;
@@ -28,6 +29,9 @@
/// [module level documentation]: kernel::block::mq
#[macros::vtable]
pub trait Operations: Sized {
+ /// Module ownership for this device, provided via `THIS_MODULE`.
+ type ThisModule: ThisModule;
+
/// Data associated with the `struct request_queue` that is allocated for
/// the `GenDisk` associated with this `Operations` implementation.
type QueueData: ForeignOwnable;
@@ -280,7 +284,33 @@ impl<T: Operations> OperationsVTable<T> {
show_rq: None,
};
+ const BLOCK_OPS: bindings::block_device_operations = bindings::block_device_operations {
+ submit_bio: None,
+ open: None,
+ release: None,
+ ioctl: None,
+ compat_ioctl: None,
+ check_events: None,
+ unlock_native_capacity: None,
+ getgeo: None,
+ set_read_only: None,
+ swap_slot_free_notify: None,
+ report_zones: None,
+ devnode: None,
+ alternative_gpt_sector: None,
+ get_unique_id: None,
+ owner: T::ThisModule::OWNER.as_ptr(),
+ pr_ops: core::ptr::null_mut(),
+ free_disk: None,
+ poll_bio: None,
+ };
+
pub(crate) const fn build() -> &'static bindings::blk_mq_ops {
&Self::VTABLE
}
+
+ pub(crate) const fn build_block_device_operations() -> &'static bindings::block_device_operations
+ {
+ &Self::BLOCK_OPS
+ }
}
--
2.43.0
^ permalink raw reply related
* [PATCH RFC v2 05/11] rust: drm: fix missing owner in file_operations
From: Kari Argillander @ 2026-01-06 16:11 UTC (permalink / raw)
To: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Alexandre Courbot
Cc: Greg Kroah-Hartman, rust-for-linux, linux-kernel, linux-modules,
Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
Aaron Tomlin, Kari Argillander
In-Reply-To: <20260106-this_module_fix-v2-0-842ac026f00b@gmail.com>
Fix missing .owner field in file_operations. This has been previosly
left out because Rust feature `const_refs_to_static` has not been
enabled. Now that it is we can make define owner even in const context.
This should probably fix use-after-free problems in situations where
file is opened and module driver is unloaded during that.
Signed-off-by: Kari Argillander <kari.argillander@gmail.com>
---
drivers/gpu/drm/nova/driver.rs | 2 ++
drivers/gpu/drm/tyr/driver.rs | 2 ++
rust/kernel/drm/device.rs | 2 +-
rust/kernel/drm/driver.rs | 4 ++++
rust/kernel/drm/gem/mod.rs | 5 +++--
5 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/nova/driver.rs b/drivers/gpu/drm/nova/driver.rs
index b1af0a099551..7ce505802716 100644
--- a/drivers/gpu/drm/nova/driver.rs
+++ b/drivers/gpu/drm/nova/driver.rs
@@ -14,6 +14,7 @@
use crate::file::File;
use crate::gem::NovaObject;
+use crate::THIS_MODULE;
pub(crate) struct NovaDriver {
#[expect(unused)]
@@ -65,6 +66,7 @@ fn probe(adev: &auxiliary::Device<Core>, _info: &Self::IdInfo) -> impl PinInit<S
#[vtable]
impl drm::Driver for NovaDriver {
+ type ThisModule = THIS_MODULE;
type Data = NovaData;
type File = File;
type Object = gem::Object<NovaObject>;
diff --git a/drivers/gpu/drm/tyr/driver.rs b/drivers/gpu/drm/tyr/driver.rs
index f0da58932702..11932d3f03ff 100644
--- a/drivers/gpu/drm/tyr/driver.rs
+++ b/drivers/gpu/drm/tyr/driver.rs
@@ -25,6 +25,7 @@
use crate::gpu;
use crate::gpu::GpuInfo;
use crate::regs;
+use crate::THIS_MODULE;
pub(crate) type IoMem = kernel::io::mem::IoMem<SZ_2M>;
@@ -179,6 +180,7 @@ fn drop(self: Pin<&mut Self>) {
#[vtable]
impl drm::Driver for TyrDriver {
+ type ThisModule = THIS_MODULE;
type Data = TyrData;
type File = File;
type Object = drm::gem::Object<TyrObject>;
diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs
index 3ce8f62a0056..a740c87933d0 100644
--- a/rust/kernel/drm/device.rs
+++ b/rust/kernel/drm/device.rs
@@ -92,7 +92,7 @@ impl<T: drm::Driver> Device<T> {
fops: &Self::GEM_FOPS,
};
- const GEM_FOPS: bindings::file_operations = drm::gem::create_fops();
+ const GEM_FOPS: bindings::file_operations = drm::gem::create_fops::<T::ThisModule>();
/// Create a new `drm::Device` for a `drm::Driver`.
pub fn new(dev: &device::Device, data: impl PinInit<T::Data, Error>) -> Result<ARef<Self>> {
diff --git a/rust/kernel/drm/driver.rs b/rust/kernel/drm/driver.rs
index f30ee4c6245c..a157db2ea02b 100644
--- a/rust/kernel/drm/driver.rs
+++ b/rust/kernel/drm/driver.rs
@@ -9,6 +9,7 @@
error::{to_result, Result},
prelude::*,
sync::aref::ARef,
+ this_module::ThisModule,
};
use macros::vtable;
@@ -99,6 +100,9 @@ pub trait AllocImpl: super::private::Sealed + drm::gem::IntoGEMObject {
/// drm_driver` to be registered in the DRM subsystem.
#[vtable]
pub trait Driver {
+ /// Module ownership for this device, provided via `THIS_MODULE`.
+ type ThisModule: ThisModule;
+
/// Context data associated with the DRM driver
type Data: Sync + Send;
diff --git a/rust/kernel/drm/gem/mod.rs b/rust/kernel/drm/gem/mod.rs
index bdaac839dacc..9980cebec96b 100644
--- a/rust/kernel/drm/gem/mod.rs
+++ b/rust/kernel/drm/gem/mod.rs
@@ -11,6 +11,7 @@
error::{to_result, Result},
prelude::*,
sync::aref::{ARef, AlwaysRefCounted},
+ this_module::ThisModule,
types::Opaque,
};
use core::{ops::Deref, ptr::NonNull};
@@ -292,10 +293,10 @@ impl<T: DriverObject> AllocImpl for Object<T> {
};
}
-pub(super) const fn create_fops() -> bindings::file_operations {
+pub(super) const fn create_fops<M: ThisModule>() -> bindings::file_operations {
let mut fops: bindings::file_operations = pin_init::zeroed();
- fops.owner = core::ptr::null_mut();
+ fops.owner = M::OWNER.as_ptr();
fops.open = Some(bindings::drm_open);
fops.release = Some(bindings::drm_release);
fops.unlocked_ioctl = Some(bindings::drm_ioctl);
--
2.43.0
^ permalink raw reply related
* [PATCH RFC v2 06/11] rust: driver: make RegistrationOps::register() to use new ThisModule
From: Kari Argillander @ 2026-01-06 16:11 UTC (permalink / raw)
To: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Alexandre Courbot
Cc: Greg Kroah-Hartman, rust-for-linux, linux-kernel, linux-modules,
Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
Aaron Tomlin, Kari Argillander
In-Reply-To: <20260106-this_module_fix-v2-0-842ac026f00b@gmail.com>
New version of ThisModule is trait which can be passed in const context.
To have unified way to pass THIS_MODULE to abstactions have const
parameter which can be used to get owner and name.
Signed-off-by: Kari Argillander <kari.argillander@gmail.com>
---
rust/kernel/auxiliary.rs | 16 ++++++++--------
rust/kernel/driver.rs | 29 +++++++++++++----------------
rust/kernel/i2c.rs | 11 ++++-------
rust/kernel/pci.rs | 15 +++++----------
rust/kernel/platform.rs | 12 ++++--------
rust/kernel/usb.rs | 13 ++++---------
samples/rust/rust_driver_auxiliary.rs | 6 +++---
7 files changed, 41 insertions(+), 61 deletions(-)
diff --git a/rust/kernel/auxiliary.rs b/rust/kernel/auxiliary.rs
index 56f3c180e8f6..102b0349af16 100644
--- a/rust/kernel/auxiliary.rs
+++ b/rust/kernel/auxiliary.rs
@@ -11,8 +11,8 @@
driver,
error::{from_result, to_result, Result},
prelude::*,
+ this_module::ThisModule,
types::Opaque,
- ThisModule,
};
use core::{
marker::PhantomData,
@@ -28,14 +28,10 @@
unsafe impl<T: Driver + 'static> driver::RegistrationOps for Adapter<T> {
type RegType = bindings::auxiliary_driver;
- unsafe fn register(
- adrv: &Opaque<Self::RegType>,
- name: &'static CStr,
- module: &'static ThisModule,
- ) -> Result {
+ unsafe fn register<TM: ThisModule>(adrv: &Opaque<Self::RegType>) -> Result {
// SAFETY: It's safe to set the fields of `struct auxiliary_driver` on initialization.
unsafe {
- (*adrv.get()).name = name.as_char_ptr();
+ (*adrv.get()).name = TM::NAME.as_char_ptr();
(*adrv.get()).probe = Some(Self::probe_callback);
(*adrv.get()).remove = Some(Self::remove_callback);
(*adrv.get()).id_table = T::ID_TABLE.as_ptr();
@@ -43,7 +39,11 @@ unsafe fn register(
// SAFETY: `adrv` is guaranteed to be a valid `RegType`.
to_result(unsafe {
- bindings::__auxiliary_driver_register(adrv.get(), module.0, name.as_char_ptr())
+ bindings::__auxiliary_driver_register(
+ adrv.get(),
+ TM::OWNER.as_ptr(),
+ TM::NAME.as_char_ptr(),
+ )
})
}
diff --git a/rust/kernel/driver.rs b/rust/kernel/driver.rs
index 649d06468f41..dc7522c4ebda 100644
--- a/rust/kernel/driver.rs
+++ b/rust/kernel/driver.rs
@@ -94,10 +94,14 @@
//! [`device_id`]: kernel::device_id
//! [`module_driver`]: kernel::module_driver
-use crate::error::{Error, Result};
-use crate::{acpi, device, of, str::CStr, try_pin_init, types::Opaque, ThisModule};
-use core::pin::Pin;
-use pin_init::{pin_data, pinned_drop, PinInit};
+use crate::{
+ acpi,
+ device,
+ of,
+ prelude::*,
+ this_module::ThisModule,
+ types::Opaque, //
+};
/// The [`RegistrationOps`] trait serves as generic interface for subsystems (e.g., PCI, Platform,
/// Amba, etc.) to provide the corresponding subsystem specific implementation to register /
@@ -122,11 +126,7 @@ pub unsafe trait RegistrationOps {
///
/// On success, `reg` must remain pinned and valid until the matching call to
/// [`RegistrationOps::unregister`].
- unsafe fn register(
- reg: &Opaque<Self::RegType>,
- name: &'static CStr,
- module: &'static ThisModule,
- ) -> Result;
+ unsafe fn register<TM: ThisModule>(reg: &Opaque<Self::RegType>) -> Result;
/// Unregisters a driver previously registered with [`RegistrationOps::register`].
///
@@ -159,7 +159,7 @@ unsafe impl<T: RegistrationOps> Send for Registration<T> {}
impl<T: RegistrationOps> Registration<T> {
/// Creates a new instance of the registration object.
- pub fn new(name: &'static CStr, module: &'static ThisModule) -> impl PinInit<Self, Error> {
+ pub fn new<TM: ThisModule>() -> impl PinInit<Self, Error> {
try_pin_init!(Self {
reg <- Opaque::try_ffi_init(|ptr: *mut T::RegType| {
// SAFETY: `try_ffi_init` guarantees that `ptr` is valid for write.
@@ -170,7 +170,7 @@ pub fn new(name: &'static CStr, module: &'static ThisModule) -> impl PinInit<Sel
let drv = unsafe { &*(ptr as *const Opaque<T::RegType>) };
// SAFETY: `drv` is guaranteed to be pinned until `T::unregister`.
- unsafe { T::register(drv, name, module) }
+ unsafe { T::register::<TM>(drv) }
}),
})
}
@@ -202,13 +202,10 @@ struct DriverModule {
impl $crate::InPlaceModule for DriverModule {
fn init(
- module: &'static $crate::ThisModule
+ _module: &'static $crate::ThisModule
) -> impl ::pin_init::PinInit<Self, $crate::error::Error> {
$crate::try_pin_init!(Self {
- _driver <- $crate::driver::Registration::new(
- <Self as $crate::ModuleMetadata>::NAME,
- module,
- ),
+ _driver <- $crate::driver::Registration::new::<crate::THIS_MODULE>(),
})
}
}
diff --git a/rust/kernel/i2c.rs b/rust/kernel/i2c.rs
index 491e6cc25cf4..b23a26a445cd 100644
--- a/rust/kernel/i2c.rs
+++ b/rust/kernel/i2c.rs
@@ -16,6 +16,7 @@
error::*,
of,
prelude::*,
+ this_module::ThisModule,
types::{
AlwaysRefCounted,
Opaque, //
@@ -97,11 +98,7 @@ macro_rules! i2c_device_table {
unsafe impl<T: Driver + 'static> driver::RegistrationOps for Adapter<T> {
type RegType = bindings::i2c_driver;
- unsafe fn register(
- idrv: &Opaque<Self::RegType>,
- name: &'static CStr,
- module: &'static ThisModule,
- ) -> Result {
+ unsafe fn register<TM: ThisModule>(idrv: &Opaque<Self::RegType>) -> Result {
build_assert!(
T::ACPI_ID_TABLE.is_some() || T::OF_ID_TABLE.is_some() || T::I2C_ID_TABLE.is_some(),
"At least one of ACPI/OF/Legacy tables must be present when registering an i2c driver"
@@ -124,7 +121,7 @@ unsafe fn register(
// SAFETY: It's safe to set the fields of `struct i2c_client` on initialization.
unsafe {
- (*idrv.get()).driver.name = name.as_char_ptr();
+ (*idrv.get()).driver.name = TM::NAME.as_char_ptr();
(*idrv.get()).probe = Some(Self::probe_callback);
(*idrv.get()).remove = Some(Self::remove_callback);
(*idrv.get()).shutdown = Some(Self::shutdown_callback);
@@ -134,7 +131,7 @@ unsafe fn register(
}
// SAFETY: `idrv` is guaranteed to be a valid `RegType`.
- to_result(unsafe { bindings::i2c_register_driver(module.0, idrv.get()) })
+ to_result(unsafe { bindings::i2c_register_driver(TM::OWNER.as_ptr(), idrv.get()) })
}
unsafe fn unregister(idrv: &Opaque<Self::RegType>) {
diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
index 82e128431f08..88a5416fb44b 100644
--- a/rust/kernel/pci.rs
+++ b/rust/kernel/pci.rs
@@ -18,9 +18,8 @@
to_result, //
},
prelude::*,
- str::CStr,
- types::Opaque,
- ThisModule, //
+ this_module::ThisModule,
+ types::Opaque, //
};
use core::{
marker::PhantomData,
@@ -55,14 +54,10 @@
unsafe impl<T: Driver + 'static> driver::RegistrationOps for Adapter<T> {
type RegType = bindings::pci_driver;
- unsafe fn register(
- pdrv: &Opaque<Self::RegType>,
- name: &'static CStr,
- module: &'static ThisModule,
- ) -> Result {
+ unsafe fn register<TM: ThisModule>(pdrv: &Opaque<Self::RegType>) -> Result {
// SAFETY: It's safe to set the fields of `struct pci_driver` on initialization.
unsafe {
- (*pdrv.get()).name = name.as_char_ptr();
+ (*pdrv.get()).name = TM::NAME.as_char_ptr();
(*pdrv.get()).probe = Some(Self::probe_callback);
(*pdrv.get()).remove = Some(Self::remove_callback);
(*pdrv.get()).id_table = T::ID_TABLE.as_ptr();
@@ -70,7 +65,7 @@ unsafe fn register(
// SAFETY: `pdrv` is guaranteed to be a valid `RegType`.
to_result(unsafe {
- bindings::__pci_register_driver(pdrv.get(), module.0, name.as_char_ptr())
+ bindings::__pci_register_driver(pdrv.get(), TM::OWNER.as_ptr(), TM::NAME.as_char_ptr())
})
}
diff --git a/rust/kernel/platform.rs b/rust/kernel/platform.rs
index bddb593cee7b..a4678af3b891 100644
--- a/rust/kernel/platform.rs
+++ b/rust/kernel/platform.rs
@@ -13,8 +13,8 @@
irq::{self, IrqRequest},
of,
prelude::*,
+ this_module::ThisModule,
types::Opaque,
- ThisModule,
};
use core::{
@@ -31,11 +31,7 @@
unsafe impl<T: Driver + 'static> driver::RegistrationOps for Adapter<T> {
type RegType = bindings::platform_driver;
- unsafe fn register(
- pdrv: &Opaque<Self::RegType>,
- name: &'static CStr,
- module: &'static ThisModule,
- ) -> Result {
+ unsafe fn register<TM: ThisModule>(pdrv: &Opaque<Self::RegType>) -> Result {
let of_table = match T::OF_ID_TABLE {
Some(table) => table.as_ptr(),
None => core::ptr::null(),
@@ -48,7 +44,7 @@ unsafe fn register(
// SAFETY: It's safe to set the fields of `struct platform_driver` on initialization.
unsafe {
- (*pdrv.get()).driver.name = name.as_char_ptr();
+ (*pdrv.get()).driver.name = TM::NAME.as_char_ptr();
(*pdrv.get()).probe = Some(Self::probe_callback);
(*pdrv.get()).remove = Some(Self::remove_callback);
(*pdrv.get()).driver.of_match_table = of_table;
@@ -56,7 +52,7 @@ unsafe fn register(
}
// SAFETY: `pdrv` is guaranteed to be a valid `RegType`.
- to_result(unsafe { bindings::__platform_driver_register(pdrv.get(), module.0) })
+ to_result(unsafe { bindings::__platform_driver_register(pdrv.get(), TM::OWNER.as_ptr()) })
}
unsafe fn unregister(pdrv: &Opaque<Self::RegType>) {
diff --git a/rust/kernel/usb.rs b/rust/kernel/usb.rs
index d10b65e9fb6a..e7e07360f953 100644
--- a/rust/kernel/usb.rs
+++ b/rust/kernel/usb.rs
@@ -11,9 +11,8 @@
driver,
error::{from_result, to_result, Result},
prelude::*,
- str::CStr,
+ this_module::ThisModule,
types::{AlwaysRefCounted, Opaque},
- ThisModule,
};
use core::{
marker::PhantomData,
@@ -32,14 +31,10 @@
unsafe impl<T: Driver + 'static> driver::RegistrationOps for Adapter<T> {
type RegType = bindings::usb_driver;
- unsafe fn register(
- udrv: &Opaque<Self::RegType>,
- name: &'static CStr,
- module: &'static ThisModule,
- ) -> Result {
+ unsafe fn register<TM: ThisModule>(udrv: &Opaque<Self::RegType>) -> Result {
// SAFETY: It's safe to set the fields of `struct usb_driver` on initialization.
unsafe {
- (*udrv.get()).name = name.as_char_ptr();
+ (*udrv.get()).name = TM::NAME.as_char_ptr();
(*udrv.get()).probe = Some(Self::probe_callback);
(*udrv.get()).disconnect = Some(Self::disconnect_callback);
(*udrv.get()).id_table = T::ID_TABLE.as_ptr();
@@ -47,7 +42,7 @@ unsafe fn register(
// SAFETY: `udrv` is guaranteed to be a valid `RegType`.
to_result(unsafe {
- bindings::usb_register_driver(udrv.get(), module.0, name.as_char_ptr())
+ bindings::usb_register_driver(udrv.get(), TM::OWNER.as_ptr(), TM::NAME.as_char_ptr())
})
}
diff --git a/samples/rust/rust_driver_auxiliary.rs b/samples/rust/rust_driver_auxiliary.rs
index 1e4fb23cfcb0..28a25e540298 100644
--- a/samples/rust/rust_driver_auxiliary.rs
+++ b/samples/rust/rust_driver_auxiliary.rs
@@ -111,10 +111,10 @@ struct SampleModule {
}
impl InPlaceModule for SampleModule {
- fn init(module: &'static kernel::ThisModule) -> impl PinInit<Self, Error> {
+ fn init(_module: &'static kernel::ThisModule) -> impl PinInit<Self, Error> {
try_pin_init!(Self {
- _pci_driver <- driver::Registration::new(MODULE_NAME, module),
- _aux_driver <- driver::Registration::new(MODULE_NAME, module),
+ _pci_driver <- driver::Registration::new::<THIS_MODULE>(),
+ _aux_driver <- driver::Registration::new::<THIS_MODULE>(),
})
}
}
--
2.43.0
^ permalink raw reply related
* [PATCH RFC v2 07/11] rust: phy: make Registration::register() use new ThisModule
From: Kari Argillander @ 2026-01-06 16:11 UTC (permalink / raw)
To: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Alexandre Courbot
Cc: Greg Kroah-Hartman, rust-for-linux, linux-kernel, linux-modules,
Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
Aaron Tomlin, Kari Argillander
In-Reply-To: <20260106-this_module_fix-v2-0-842ac026f00b@gmail.com>
Switch `Registration::register()` to take the owning module via the
`ThisModule` abstraction instead of an explicit module parameter.
The function is now generic over `TM: ThisModule`, allowing the module
owner to be resolved at compile time through `TM::OWNER`. This unifies
the way `THIS_MODULE` is passed to Rust abstractions and avoids
threading module pointers manually through the API.
This also removes redundant parameters and prevents accidental
mismatches between the registered drivers and their owning module.
Signed-off-by: Kari Argillander <kari.argillander@gmail.com>
---
rust/kernel/net/phy.rs | 29 +++++++++++++++++++----------
1 file changed, 19 insertions(+), 10 deletions(-)
diff --git a/rust/kernel/net/phy.rs b/rust/kernel/net/phy.rs
index bf6272d87a7b..b6c99bf7e97b 100644
--- a/rust/kernel/net/phy.rs
+++ b/rust/kernel/net/phy.rs
@@ -6,8 +6,17 @@
//!
//! C headers: [`include/linux/phy.h`](srctree/include/linux/phy.h).
-use crate::{device_id::RawDeviceId, error::*, prelude::*, types::Opaque};
-use core::{marker::PhantomData, ptr::addr_of_mut};
+use crate::{
+ device_id::RawDeviceId,
+ error::*,
+ prelude::*,
+ this_module::ThisModule,
+ types::Opaque, //
+};
+use core::{
+ marker::PhantomData,
+ ptr::addr_of_mut, //
+};
pub mod reg;
@@ -648,10 +657,7 @@ unsafe impl Send for Registration {}
impl Registration {
/// Registers a PHY driver.
- pub fn register(
- module: &'static crate::ThisModule,
- drivers: Pin<&'static mut [DriverVTable]>,
- ) -> Result<Self> {
+ pub fn register<TM: ThisModule>(drivers: Pin<&'static mut [DriverVTable]>) -> Result<Self> {
if drivers.is_empty() {
return Err(code::EINVAL);
}
@@ -659,7 +665,11 @@ pub fn register(
// the `drivers` slice are initialized properly. `drivers` will not be moved.
// So it's just an FFI call.
to_result(unsafe {
- bindings::phy_drivers_register(drivers[0].0.get(), drivers.len().try_into()?, module.0)
+ bindings::phy_drivers_register(
+ drivers[0].0.get(),
+ drivers.len().try_into()?,
+ TM::OWNER.as_ptr(),
+ )
})?;
// INVARIANT: The `drivers` slice is successfully registered to the kernel via `phy_drivers_register`.
Ok(Registration { drivers })
@@ -891,12 +901,11 @@ struct Module {
[$($crate::net::phy::create_phy_driver::<$driver>()),+];
impl $crate::Module for Module {
- fn init(module: &'static $crate::ThisModule) -> Result<Self> {
+ fn init(_module: &'static $crate::ThisModule) -> Result<Self> {
// SAFETY: The anonymous constant guarantees that nobody else can access
// the `DRIVERS` static. The array is used only in the C side.
let drivers = unsafe { &mut DRIVERS };
- let mut reg = $crate::net::phy::Registration::register(
- module,
+ let mut reg = $crate::net::phy::Registration::register::<crate::THIS_MODULE>(
::core::pin::Pin::static_mut(drivers),
)?;
Ok(Module { _reg: reg })
--
2.43.0
^ permalink raw reply related
* [PATCH RFC v2 08/11] rust: binder: use new THIS_MODULE
From: Kari Argillander @ 2026-01-06 16:11 UTC (permalink / raw)
To: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Alexandre Courbot
Cc: Greg Kroah-Hartman, rust-for-linux, linux-kernel, linux-modules,
Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
Aaron Tomlin, Kari Argillander
In-Reply-To: <20260106-this_module_fix-v2-0-842ac026f00b@gmail.com>
We have new THIS_MODULE. ThisModule is now crate. This is ugly for
reason that drivers should not use as_ptr() directly. Currently binder
still needs it so ugly cast is totally ok.
Signed-off-by: Kari Argillander <kari.argillander@gmail.com>
---
drivers/android/binder/rust_binder_main.rs | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/android/binder/rust_binder_main.rs b/drivers/android/binder/rust_binder_main.rs
index d84c3c360be0..fc921c0e1116 100644
--- a/drivers/android/binder/rust_binder_main.rs
+++ b/drivers/android/binder/rust_binder_main.rs
@@ -21,6 +21,7 @@
sync::poll::PollTable,
sync::Arc,
task::Pid,
+ this_module::ThisModule,
transmute::AsBytes,
types::ForeignOwnable,
uaccess::UserSliceWriter,
@@ -319,7 +320,7 @@ unsafe impl<T> Sync for AssertSync<T> {}
let zeroed_ops = unsafe { core::mem::MaybeUninit::zeroed().assume_init() };
let ops = kernel::bindings::file_operations {
- owner: THIS_MODULE.as_ptr(),
+ owner: <THIS_MODULE as ThisModule>::OWNER.as_ptr(),
poll: Some(rust_binder_poll),
unlocked_ioctl: Some(rust_binder_ioctl),
#[cfg(CONFIG_COMPAT)]
--
2.43.0
^ permalink raw reply related
* [PATCH RFC v2 09/11] rust: remove module argument from InPlaceModule::init()
From: Kari Argillander @ 2026-01-06 16:11 UTC (permalink / raw)
To: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Alexandre Courbot
Cc: Greg Kroah-Hartman, rust-for-linux, linux-kernel, linux-modules,
Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
Aaron Tomlin, Kari Argillander, Yuheng Su
In-Reply-To: <20260106-this_module_fix-v2-0-842ac026f00b@gmail.com>
InPlaceModule::init() has ThisModule argument. However modules always
have THIS_MODULE made by module! macro. So it is unnecessary to pass
same information through this function. End goal is to make idea of
THIS_MODULE simpler. Driver getting this THIS_MODULE from multiple
places is confusing. So let's just stick with THIS_MODULE as that also
works in const context very easily.
Reported-by: Yuheng Su <gipsyh.icu@gmail.com>
Closes: https://github.com/Rust-for-Linux/linux/issues/720
Signed-off-by: Kari Argillander <kari.argillander@gmail.com>
---
drivers/android/binder/rust_binder_main.rs | 2 +-
drivers/block/rnull/rnull.rs | 2 +-
lib/find_bit_benchmark_rust.rs | 3 +--
rust/kernel/configfs.rs | 3 +--
rust/kernel/driver.rs | 4 +---
rust/kernel/firmware.rs | 2 +-
rust/kernel/lib.rs | 8 ++++----
rust/kernel/net/phy.rs | 2 +-
rust/kernel/sync/lock/global.rs | 4 ++--
rust/macros/lib.rs | 4 ++--
rust/macros/module.rs | 2 +-
samples/rust/rust_configfs.rs | 2 +-
samples/rust/rust_debugfs_scoped.rs | 2 +-
samples/rust/rust_driver_auxiliary.rs | 2 +-
samples/rust/rust_driver_faux.rs | 2 +-
samples/rust/rust_minimal.rs | 2 +-
samples/rust/rust_misc_device.rs | 2 +-
samples/rust/rust_print_main.rs | 2 +-
18 files changed, 23 insertions(+), 27 deletions(-)
diff --git a/drivers/android/binder/rust_binder_main.rs b/drivers/android/binder/rust_binder_main.rs
index fc921c0e1116..8b865112e60c 100644
--- a/drivers/android/binder/rust_binder_main.rs
+++ b/drivers/android/binder/rust_binder_main.rs
@@ -291,7 +291,7 @@ fn ptr_align(value: usize) -> Option<usize> {
struct BinderModule {}
impl kernel::Module for BinderModule {
- fn init(_module: &'static kernel::ThisModule) -> Result<Self> {
+ fn init() -> Result<Self> {
// SAFETY: The module initializer never runs twice, so we only call this once.
unsafe { crate::context::CONTEXTS.init() };
diff --git a/drivers/block/rnull/rnull.rs b/drivers/block/rnull/rnull.rs
index 862369ab9b5c..a9be1b2187f4 100644
--- a/drivers/block/rnull/rnull.rs
+++ b/drivers/block/rnull/rnull.rs
@@ -36,7 +36,7 @@ struct NullBlkModule {
}
impl kernel::InPlaceModule for NullBlkModule {
- fn init(_module: &'static ThisModule) -> impl PinInit<Self, Error> {
+ fn init() -> impl PinInit<Self, Error> {
pr_info!("Rust null_blk loaded\n");
try_pin_init!(Self {
diff --git a/lib/find_bit_benchmark_rust.rs b/lib/find_bit_benchmark_rust.rs
index 6bdc51de2f30..5c231569d887 100644
--- a/lib/find_bit_benchmark_rust.rs
+++ b/lib/find_bit_benchmark_rust.rs
@@ -7,7 +7,6 @@
use kernel::error::{code, Result};
use kernel::prelude::module;
use kernel::time::{Instant, Monotonic};
-use kernel::ThisModule;
use kernel::{pr_cont, pr_err};
const BITMAP_LEN: usize = 4096 * 8 * 10;
@@ -88,7 +87,7 @@ fn find_bit_test() {
}
impl kernel::Module for Benchmark {
- fn init(_module: &'static ThisModule) -> Result<Self> {
+ fn init() -> Result<Self> {
find_bit_test();
// Return error so test module can be inserted again without rmmod.
Err(code::EINVAL)
diff --git a/rust/kernel/configfs.rs b/rust/kernel/configfs.rs
index 908cb98d404f..2af63f7daef2 100644
--- a/rust/kernel/configfs.rs
+++ b/rust/kernel/configfs.rs
@@ -27,7 +27,6 @@
//! use kernel::new_mutex;
//! use kernel::page::PAGE_SIZE;
//! use kernel::sync::Mutex;
-//! use kernel::ThisModule;
//!
//! #[pin_data]
//! struct RustConfigfs {
@@ -36,7 +35,7 @@
//! }
//!
//! impl kernel::InPlaceModule for RustConfigfs {
-//! fn init(_module: &'static ThisModule) -> impl PinInit<Self, Error> {
+//! fn init() -> impl PinInit<Self, Error> {
//! pr_info!("Rust configfs sample (init)\n");
//!
//! let item_type = configfs_attrs! {
diff --git a/rust/kernel/driver.rs b/rust/kernel/driver.rs
index dc7522c4ebda..de77f95d7fe0 100644
--- a/rust/kernel/driver.rs
+++ b/rust/kernel/driver.rs
@@ -201,9 +201,7 @@ struct DriverModule {
}
impl $crate::InPlaceModule for DriverModule {
- fn init(
- _module: &'static $crate::ThisModule
- ) -> impl ::pin_init::PinInit<Self, $crate::error::Error> {
+ fn init() -> impl ::pin_init::PinInit<Self, $crate::error::Error> {
$crate::try_pin_init!(Self {
_driver <- $crate::driver::Registration::new::<crate::THIS_MODULE>(),
})
diff --git a/rust/kernel/firmware.rs b/rust/kernel/firmware.rs
index 71168d8004e2..11372a8f7be4 100644
--- a/rust/kernel/firmware.rs
+++ b/rust/kernel/firmware.rs
@@ -157,7 +157,7 @@ unsafe impl Sync for Firmware {}
/// # struct MyModule;
/// #
/// # impl kernel::Module for MyModule {
-/// # fn init(_module: &'static ThisModule) -> Result<Self> {
+/// # fn init() -> Result<Self> {
/// # Ok(Self)
/// # }
/// # }
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 2ccd75f68f03..e7bc52a6ad42 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -175,7 +175,7 @@ pub trait Module: Sized + Sync + Send {
/// should do.
///
/// Equivalent to the `module_init` macro in the C API.
- fn init(module: &'static ThisModule) -> error::Result<Self>;
+ fn init() -> error::Result<Self>;
}
/// A module that is pinned and initialised in-place.
@@ -183,13 +183,13 @@ pub trait InPlaceModule: Sync + Send {
/// Creates an initialiser for the module.
///
/// It is called when the module is loaded.
- fn init(module: &'static ThisModule) -> impl pin_init::PinInit<Self, error::Error>;
+ fn init() -> impl pin_init::PinInit<Self, error::Error>;
}
impl<T: Module> InPlaceModule for T {
- fn init(module: &'static ThisModule) -> impl pin_init::PinInit<Self, error::Error> {
+ fn init() -> impl pin_init::PinInit<Self, error::Error> {
let initer = move |slot: *mut Self| {
- let m = <Self as Module>::init(module)?;
+ let m = <Self as Module>::init()?;
// SAFETY: `slot` is valid for write per the contract with `pin_init_from_closure`.
unsafe { slot.write(m) };
diff --git a/rust/kernel/net/phy.rs b/rust/kernel/net/phy.rs
index b6c99bf7e97b..7d467d42e951 100644
--- a/rust/kernel/net/phy.rs
+++ b/rust/kernel/net/phy.rs
@@ -901,7 +901,7 @@ struct Module {
[$($crate::net::phy::create_phy_driver::<$driver>()),+];
impl $crate::Module for Module {
- fn init(_module: &'static $crate::ThisModule) -> Result<Self> {
+ fn init() -> Result<Self> {
// SAFETY: The anonymous constant guarantees that nobody else can access
// the `DRIVERS` static. The array is used only in the C side.
let drivers = unsafe { &mut DRIVERS };
diff --git a/rust/kernel/sync/lock/global.rs b/rust/kernel/sync/lock/global.rs
index eab48108a4ae..7fde464462d1 100644
--- a/rust/kernel/sync/lock/global.rs
+++ b/rust/kernel/sync/lock/global.rs
@@ -203,7 +203,7 @@ pub fn get_mut(&mut self) -> &mut T {
/// }
///
/// impl kernel::Module for MyModule {
-/// fn init(_module: &'static ThisModule) -> Result<Self> {
+/// fn init() -> Result<Self> {
/// // SAFETY: Called exactly once.
/// unsafe { MY_COUNTER.init() };
///
@@ -243,7 +243,7 @@ pub fn get_mut(&mut self) -> &mut T {
/// }
///
/// impl kernel::Module for MyModule {
-/// fn init(_module: &'static ThisModule) -> Result<Self> {
+/// fn init() -> Result<Self> {
/// // SAFETY: Called exactly once.
/// unsafe { MY_MUTEX.init() };
///
diff --git a/rust/macros/lib.rs b/rust/macros/lib.rs
index b38002151871..d22a93696209 100644
--- a/rust/macros/lib.rs
+++ b/rust/macros/lib.rs
@@ -80,7 +80,7 @@
/// struct MyModule(i32);
///
/// impl kernel::Module for MyModule {
-/// fn init(_module: &'static ThisModule) -> Result<Self> {
+/// fn init() -> Result<Self> {
/// let foo: i32 = 42;
/// pr_info!("I contain: {}\n", foo);
/// pr_info!("i32 param is: {}\n", module_parameters::my_parameter.read());
@@ -114,7 +114,7 @@
/// struct MyDeviceDriverModule;
///
/// impl kernel::Module for MyDeviceDriverModule {
-/// fn init(_module: &'static ThisModule) -> Result<Self> {
+/// fn init() -> Result<Self> {
/// Ok(Self)
/// }
/// }
diff --git a/rust/macros/module.rs b/rust/macros/module.rs
index 1bcd703735fe..7473a377a3bd 100644
--- a/rust/macros/module.rs
+++ b/rust/macros/module.rs
@@ -490,7 +490,7 @@ mod __module_init {{
/// This function must only be called once.
unsafe fn __init() -> ::kernel::ffi::c_int {{
let initer =
- <{type_} as ::kernel::InPlaceModule>::init(&super::super::THIS_MODULE.as_ref());
+ <{type_} as ::kernel::InPlaceModule>::init();
// SAFETY: No data race, since `__MOD` can only be accessed by this module
// and there only `__init` and `__exit` access it. These functions are only
// called once and `__exit` cannot be called before or during `__init`.
diff --git a/samples/rust/rust_configfs.rs b/samples/rust/rust_configfs.rs
index 0ccc7553ef39..f34260793677 100644
--- a/samples/rust/rust_configfs.rs
+++ b/samples/rust/rust_configfs.rs
@@ -42,7 +42,7 @@ fn new() -> impl PinInit<Self, Error> {
}
impl kernel::InPlaceModule for RustConfigfs {
- fn init(_module: &'static ThisModule) -> impl PinInit<Self, Error> {
+ fn init() -> impl PinInit<Self, Error> {
pr_info!("Rust configfs sample (init)\n");
// Define a subsystem with the data type `Configuration`, two
diff --git a/samples/rust/rust_debugfs_scoped.rs b/samples/rust/rust_debugfs_scoped.rs
index 6a575a15a2c2..75897e02766b 100644
--- a/samples/rust/rust_debugfs_scoped.rs
+++ b/samples/rust/rust_debugfs_scoped.rs
@@ -134,7 +134,7 @@ fn init_control(base_dir: &Dir, dyn_dirs: Dir) -> impl PinInit<Scope<ModuleData>
}
impl kernel::Module for RustScopedDebugFs {
- fn init(_module: &'static kernel::ThisModule) -> Result<Self> {
+ fn init() -> Result<Self> {
let base_dir = Dir::new(c"rust_scoped_debugfs");
let dyn_dirs = base_dir.subdir(c"dynamic");
Ok(Self {
diff --git a/samples/rust/rust_driver_auxiliary.rs b/samples/rust/rust_driver_auxiliary.rs
index 28a25e540298..7b729687811d 100644
--- a/samples/rust/rust_driver_auxiliary.rs
+++ b/samples/rust/rust_driver_auxiliary.rs
@@ -111,7 +111,7 @@ struct SampleModule {
}
impl InPlaceModule for SampleModule {
- fn init(_module: &'static kernel::ThisModule) -> impl PinInit<Self, Error> {
+ fn init() -> impl PinInit<Self, Error> {
try_pin_init!(Self {
_pci_driver <- driver::Registration::new::<THIS_MODULE>(),
_aux_driver <- driver::Registration::new::<THIS_MODULE>(),
diff --git a/samples/rust/rust_driver_faux.rs b/samples/rust/rust_driver_faux.rs
index 5330b77ea986..2653b2ec3338 100644
--- a/samples/rust/rust_driver_faux.rs
+++ b/samples/rust/rust_driver_faux.rs
@@ -21,7 +21,7 @@ struct SampleModule {
}
impl Module for SampleModule {
- fn init(_module: &'static ThisModule) -> Result<Self> {
+ fn init() -> Result<Self> {
pr_info!("Initialising Rust Faux Device Sample\n");
let reg = faux::Registration::new(c"rust-faux-sample-device", None)?;
diff --git a/samples/rust/rust_minimal.rs b/samples/rust/rust_minimal.rs
index 8eb9583571d7..c024f8083499 100644
--- a/samples/rust/rust_minimal.rs
+++ b/samples/rust/rust_minimal.rs
@@ -23,7 +23,7 @@ struct RustMinimal {
}
impl kernel::Module for RustMinimal {
- fn init(_module: &'static ThisModule) -> Result<Self> {
+ fn init() -> Result<Self> {
pr_info!("Rust minimal sample (init)\n");
pr_info!("Am I built-in? {}\n", !cfg!(MODULE));
pr_info!(
diff --git a/samples/rust/rust_misc_device.rs b/samples/rust/rust_misc_device.rs
index 464e3026e6e3..709adf4a6026 100644
--- a/samples/rust/rust_misc_device.rs
+++ b/samples/rust/rust_misc_device.rs
@@ -128,7 +128,7 @@ struct RustMiscDeviceModule {
}
impl kernel::InPlaceModule for RustMiscDeviceModule {
- fn init(_module: &'static ThisModule) -> impl PinInit<Self, Error> {
+ fn init() -> impl PinInit<Self, Error> {
pr_info!("Initialising Rust Misc Device Sample\n");
let options = MiscDeviceOptions {
diff --git a/samples/rust/rust_print_main.rs b/samples/rust/rust_print_main.rs
index 4095c72afeab..de1bf7b80153 100644
--- a/samples/rust/rust_print_main.rs
+++ b/samples/rust/rust_print_main.rs
@@ -59,7 +59,7 @@ fn arc_dyn_print(arc: &Arc<dyn Display>) {
}
impl kernel::Module for RustPrint {
- fn init(_module: &'static ThisModule) -> Result<Self> {
+ fn init() -> Result<Self> {
pr_info!("Rust printing macros sample (init)\n");
pr_emerg!("Emergency message (level 0) without args\n");
--
2.43.0
^ permalink raw reply related
* [PATCH RFC v2 10/11] rust: remove kernel::ModuleMetadata
From: Kari Argillander @ 2026-01-06 16:11 UTC (permalink / raw)
To: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Alexandre Courbot
Cc: Greg Kroah-Hartman, rust-for-linux, linux-kernel, linux-modules,
Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
Aaron Tomlin, Kari Argillander
In-Reply-To: <20260106-this_module_fix-v2-0-842ac026f00b@gmail.com>
We have all information available also in THIS_MODULE. Use that instead.
This way we do not need to do ugly casts from driver struct.
Signed-off-by: Kari Argillander <kari.argillander@gmail.com>
---
drivers/gpu/nova-core/nova_core.rs | 2 +-
rust/kernel/firmware.rs | 2 +-
rust/kernel/lib.rs | 6 ------
rust/macros/module.rs | 8 --------
samples/rust/rust_driver_auxiliary.rs | 2 +-
5 files changed, 3 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/nova-core/nova_core.rs b/drivers/gpu/nova-core/nova_core.rs
index b98a1c03f13d..fbfbcc9446c0 100644
--- a/drivers/gpu/nova-core/nova_core.rs
+++ b/drivers/gpu/nova-core/nova_core.rs
@@ -19,7 +19,7 @@
mod util;
mod vbios;
-pub(crate) const MODULE_NAME: &kernel::str::CStr = <LocalModule as kernel::ModuleMetadata>::NAME;
+pub(crate) const MODULE_NAME: &kernel::str::CStr = THIS_MODULE::name();
kernel::module_pci_driver! {
type: driver::NovaCore,
diff --git a/rust/kernel/firmware.rs b/rust/kernel/firmware.rs
index 11372a8f7be4..42bae71f6af1 100644
--- a/rust/kernel/firmware.rs
+++ b/rust/kernel/firmware.rs
@@ -206,7 +206,7 @@ macro_rules! module_firmware {
const __MODULE_FIRMWARE_PREFIX: &'static $crate::str::CStr = if cfg!(MODULE) {
c""
} else {
- <LocalModule as $crate::ModuleMetadata>::NAME
+ crate::THIS_MODULE::name()
};
#[link_section = ".modinfo"]
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index e7bc52a6ad42..dec1d05ebe7b 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -201,12 +201,6 @@ fn init() -> impl pin_init::PinInit<Self, error::Error> {
}
}
-/// Metadata attached to a [`Module`] or [`InPlaceModule`].
-pub trait ModuleMetadata {
- /// The name of the module as specified in the `module!` macro.
- const NAME: &'static crate::str::CStr;
-}
-
/// Equivalent to `THIS_MODULE` in the C API.
///
/// C header: [`include/linux/init.h`](srctree/include/linux/init.h)
diff --git a/rust/macros/module.rs b/rust/macros/module.rs
index 7473a377a3bd..97635aed1598 100644
--- a/rust/macros/module.rs
+++ b/rust/macros/module.rs
@@ -374,14 +374,6 @@ pub(crate) fn module(ts: TokenStream) -> TokenStream {
::kernel::create_this_module!(\"{name}\");
- /// The `LocalModule` type is the type of the module created by `module!`,
- /// `module_pci_driver!`, `module_platform_driver!`, etc.
- type LocalModule = {type_};
-
- impl ::kernel::ModuleMetadata for {type_} {{
- const NAME: &'static ::kernel::str::CStr = c\"{name}\";
- }}
-
// Double nested modules, since then nobody can access the public items inside.
mod __module_init {{
mod __module_init {{
diff --git a/samples/rust/rust_driver_auxiliary.rs b/samples/rust/rust_driver_auxiliary.rs
index 7b729687811d..528866b953aa 100644
--- a/samples/rust/rust_driver_auxiliary.rs
+++ b/samples/rust/rust_driver_auxiliary.rs
@@ -18,7 +18,7 @@
use core::any::TypeId;
use pin_init::PinInit;
-const MODULE_NAME: &CStr = <LocalModule as kernel::ModuleMetadata>::NAME;
+const MODULE_NAME: &CStr = THIS_MODULE::name();
const AUXILIARY_NAME: &CStr = c"auxiliary";
struct AuxiliaryDriver;
--
2.43.0
^ permalink raw reply related
* [PATCH RFC v2 11/11] rust: remove old version of ThisModule
From: Kari Argillander @ 2026-01-06 16:11 UTC (permalink / raw)
To: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Alexandre Courbot
Cc: Greg Kroah-Hartman, rust-for-linux, linux-kernel, linux-modules,
Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
Aaron Tomlin, Kari Argillander
In-Reply-To: <20260106-this_module_fix-v2-0-842ac026f00b@gmail.com>
There are now users anymore which use old ThisModule. Also new
ThisModule did have couple quirks which where there only to probide
fucntionality what old ThisModule provided. Those also are not needed
anymore.
Closes: https://github.com/Rust-for-Linux/linux/issues/212
Closes: https://github.com/Rust-for-Linux/linux/issues/1176
Signed-off-by: Kari Argillander <kari.argillander@gmail.com>
---
rust/kernel/lib.rs | 47 -----------------------------------------------
rust/kernel/prelude.rs | 2 +-
2 files changed, 1 insertion(+), 48 deletions(-)
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index dec1d05ebe7b..e709f85ec4b5 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -201,32 +201,6 @@ fn init() -> impl pin_init::PinInit<Self, error::Error> {
}
}
-/// Equivalent to `THIS_MODULE` in the C API.
-///
-/// C header: [`include/linux/init.h`](srctree/include/linux/init.h)
-pub struct ThisModule(*mut bindings::module);
-
-// SAFETY: `THIS_MODULE` may be used from all threads within a module.
-unsafe impl Sync for ThisModule {}
-
-impl ThisModule {
- /// Creates a [`ThisModule`] given the `THIS_MODULE` pointer.
- ///
- /// # Safety
- ///
- /// The pointer must be equal to the right `THIS_MODULE`.
- pub const unsafe fn from_ptr(ptr: *mut bindings::module) -> ThisModule {
- ThisModule(ptr)
- }
-
- /// Access the raw pointer for this module.
- ///
- /// It is up to the user to use it correctly.
- pub const fn as_ptr(&self) -> *mut bindings::module {
- self.0
- }
-}
-
pub mod this_module {
//! Access to the module identity and ownership information.
//!
@@ -360,27 +334,6 @@ impl THIS_MODULE {
pub const fn name() -> &'static ::kernel::str::CStr {
$crate::c_str!($name)
}
-
- // TODO: Temporary to provide functionality old `THIS_MODULE` provided.
- // SAFETY: `__this_module` is constructed by the kernel at load time and
- // will not be freed until the module is unloaded.
- const ThisModule: ::kernel::ThisModule = unsafe {{
- ::kernel::ThisModule::from_ptr(
- <Self as ::kernel::this_module::ThisModule>::OWNER.as_ptr()
- )
- }};
-
- /// Gets a pointer to the underlying `struct module`.
- // TODO: Temporary to provide functionality old `THIS_MODULE` provided.
- pub const fn as_ptr(&self) -> *mut ::kernel::bindings::module {{
- Self::ThisModule.as_ptr()
- }}
-
- /// Gets a reference to the underlying `ThisModule`.
- /// TODO: Temporary to provide functionality old `THIS_MODULE` provided.
- pub const fn as_ref(&self) -> &'static ::kernel::ThisModule {{
- &Self::ThisModule
- }}
}
};
}
diff --git a/rust/kernel/prelude.rs b/rust/kernel/prelude.rs
index 2877e3f7b6d3..66974ec20ef4 100644
--- a/rust/kernel/prelude.rs
+++ b/rust/kernel/prelude.rs
@@ -43,7 +43,7 @@
pub use super::error::{code::*, Error, Result};
-pub use super::{str::CStrExt as _, ThisModule};
+pub use super::str::CStrExt as _;
pub use super::init::InPlaceInit;
--
2.43.0
^ permalink raw reply related
* Re: [PATCH v18 34/42] dept: add module support for struct dept_event_site and dept_event_site_dep
From: Petr Pavlu @ 2026-01-07 12:19 UTC (permalink / raw)
To: Byungchul Park
Cc: kernel_team, torvalds, damien.lemoal, linux-ide, adilger.kernel,
linux-ext4, mingo, peterz, will, tglx, rostedt, joel, sashal,
daniel.vetter, duyuyang, johannes.berg, tj, tytso, willy, david,
amir73il, gregkh, kernel-team, linux-mm, akpm, mhocko, minchan,
hannes, vdavydov.dev, sj, jglisse, dennis, cl, penberg, rientjes,
vbabka, ngupta, linux-block, josef, linux-fsdevel, jack, jlayton,
dan.j.williams, hch, djwong, dri-devel, rodrigosiqueiramelo,
melissa.srw, hamohammed.sa, harry.yoo, chris.p.wilson,
gwan-gyeong.mun, max.byungchul.park, boqun.feng, longman,
yunseong.kim, ysk, yeoreum.yun, netdev, matthew.brost, her0gyugyu,
corbet, catalin.marinas, bp, x86, hpa, luto, sumit.semwal,
gustavo, christian.koenig, andi.shyti, arnd, lorenzo.stoakes,
Liam.Howlett, rppt, surenb, mcgrof, da.gomez, samitolvanen,
paulmck, frederic, neeraj.upadhyay, joelagnelf, josh, urezki,
mathieu.desnoyers, jiangshanlai, qiang.zhang, juri.lelli,
vincent.guittot, dietmar.eggemann, bsegall, mgorman, vschneid,
chuck.lever, neil, okorniev, Dai.Ngo, tom, trondmy, anna, kees,
bigeasy, clrkwllms, mark.rutland, ada.coupriediaz,
kristina.martsenko, wangkefeng.wang, broonie, kevin.brodsky, dwmw,
shakeel.butt, ast, ziy, yuzhao, baolin.wang, usamaarif642,
joel.granados, richard.weiyang, geert+renesas, tim.c.chen, linux,
alexander.shishkin, lillian, chenhuacai, francesco,
guoweikang.kernel, link, jpoimboe, masahiroy, brauner,
thomas.weissschuh, oleg, mjguzik, andrii, wangfushuai, linux-doc,
linux-arm-kernel, linux-media, linaro-mm-sig, linux-i2c,
linux-arch, linux-modules, rcu, linux-nfs, linux-rt-devel,
2407018371, dakr, miguel.ojeda.sandonis, neilb, bagasdotme,
wsa+renesas, dave.hansen, geert, ojeda, alex.gaynor, gary,
bjorn3_gh, lossin, a.hindborg, aliceryhl, tmgross, rust-for-linux,
linux-kernel
In-Reply-To: <20251205071855.72743-35-byungchul@sk.com>
On 12/5/25 8:18 AM, Byungchul Park wrote:
> struct dept_event_site and struct dept_event_site_dep have been
> introduced to track dependencies between multi event sites for a single
> wait, that will be loaded to data segment. Plus, a custom section,
> '.dept.event_sites', also has been introduced to keep pointers to the
> objects to make sure all the event sites defined exist in code.
>
> dept should work with the section and segment of module. Add the
> support to handle the section and segment properly whenever modules are
> loaded and unloaded.
>
> Signed-off-by: Byungchul Park <byungchul@sk.com>
Below are a few comments from the module loader perspective.
> ---
> include/linux/dept.h | 14 +++++++
> include/linux/module.h | 5 +++
> kernel/dependency/dept.c | 79 +++++++++++++++++++++++++++++++++++-----
> kernel/module/main.c | 15 ++++++++
> 4 files changed, 103 insertions(+), 10 deletions(-)
>
> diff --git a/include/linux/dept.h b/include/linux/dept.h
> index 44083e6651ab..c796cdceb04e 100644
> --- a/include/linux/dept.h
> +++ b/include/linux/dept.h
> @@ -166,6 +166,11 @@ struct dept_event_site {
> struct dept_event_site *bfs_parent;
> struct list_head bfs_node;
>
> + /*
> + * for linking all dept_event_site's
> + */
> + struct list_head all_node;
> +
> /*
> * flag indicating the event is not only declared but also
> * actually used in code
> @@ -182,6 +187,11 @@ struct dept_event_site_dep {
> */
> struct list_head dep_node;
> struct list_head dep_rev_node;
> +
> + /*
> + * for linking all dept_event_site_dep's
> + */
> + struct list_head all_node;
> };
>
> #define DEPT_EVENT_SITE_INITIALIZER(es) \
> @@ -193,6 +203,7 @@ struct dept_event_site_dep {
> .bfs_gen = 0, \
> .bfs_parent = NULL, \
> .bfs_node = LIST_HEAD_INIT((es).bfs_node), \
> + .all_node = LIST_HEAD_INIT((es).all_node), \
> .used = false, \
> }
>
> @@ -202,6 +213,7 @@ struct dept_event_site_dep {
> .recover_site = NULL, \
> .dep_node = LIST_HEAD_INIT((esd).dep_node), \
> .dep_rev_node = LIST_HEAD_INIT((esd).dep_rev_node), \
> + .all_node = LIST_HEAD_INIT((esd).all_node), \
> }
>
> struct dept_event_site_init {
> @@ -225,6 +237,7 @@ extern void dept_init(void);
> extern void dept_task_init(struct task_struct *t);
> extern void dept_task_exit(struct task_struct *t);
> extern void dept_free_range(void *start, unsigned int sz);
> +extern void dept_mark_event_site_used(void *start, void *end);
Nit: The coding style recommends not using the extern keyword with
function declarations.
https://www.kernel.org/doc/html/v6.19-rc4/process/coding-style.html#function-prototypes
>
> extern void dept_map_init(struct dept_map *m, struct dept_key *k, int sub_u, const char *n);
> extern void dept_map_reinit(struct dept_map *m, struct dept_key *k, int sub_u, const char *n);
> @@ -288,6 +301,7 @@ struct dept_event_site { };
> #define dept_task_init(t) do { } while (0)
> #define dept_task_exit(t) do { } while (0)
> #define dept_free_range(s, sz) do { } while (0)
> +#define dept_mark_event_site_used(s, e) do { } while (0)
>
> #define dept_map_init(m, k, su, n) do { (void)(n); (void)(k); } while (0)
> #define dept_map_reinit(m, k, su, n) do { (void)(n); (void)(k); } while (0)
> diff --git a/include/linux/module.h b/include/linux/module.h
> index d80c3ea57472..29885ba91951 100644
> --- a/include/linux/module.h
> +++ b/include/linux/module.h
> @@ -29,6 +29,7 @@
> #include <linux/srcu.h>
> #include <linux/static_call_types.h>
> #include <linux/dynamic_debug.h>
> +#include <linux/dept.h>
>
> #include <linux/percpu.h>
> #include <asm/module.h>
> @@ -588,6 +589,10 @@ struct module {
> #ifdef CONFIG_DYNAMIC_DEBUG_CORE
> struct _ddebug_info dyndbg_info;
> #endif
> +#ifdef CONFIG_DEPT
> + struct dept_event_site **dept_event_sites;
> + unsigned int num_dept_event_sites;
> +#endif
> } ____cacheline_aligned __randomize_layout;
> #ifndef MODULE_ARCH_INIT
> #define MODULE_ARCH_INIT {}
My understanding is that entries in the .dept.event_sites section are
added by the dept_event_site_used() macro and they are pointers to the
dept_event_site_init struct, not dept_event_site.
> diff --git a/kernel/dependency/dept.c b/kernel/dependency/dept.c
> index b14400c4f83b..07d883579269 100644
> --- a/kernel/dependency/dept.c
> +++ b/kernel/dependency/dept.c
> @@ -984,6 +984,9 @@ static void bfs(void *root, struct bfs_ops *ops, void *in, void **out)
> * event sites.
> */
>
> +static LIST_HEAD(dept_event_sites);
> +static LIST_HEAD(dept_event_site_deps);
> +
> /*
> * Print all events in the circle.
> */
> @@ -2043,6 +2046,33 @@ static void del_dep_rcu(struct rcu_head *rh)
> preempt_enable();
> }
>
> +/*
> + * NOTE: Must be called with dept_lock held.
> + */
> +static void disconnect_event_site_dep(struct dept_event_site_dep *esd)
> +{
> + list_del_rcu(&esd->dep_node);
> + list_del_rcu(&esd->dep_rev_node);
> +}
> +
> +/*
> + * NOTE: Must be called with dept_lock held.
> + */
> +static void disconnect_event_site(struct dept_event_site *es)
> +{
> + struct dept_event_site_dep *esd, *next_esd;
> +
> + list_for_each_entry_safe(esd, next_esd, &es->dep_head, dep_node) {
> + list_del_rcu(&esd->dep_node);
> + list_del_rcu(&esd->dep_rev_node);
> + }
> +
> + list_for_each_entry_safe(esd, next_esd, &es->dep_rev_head, dep_rev_node) {
> + list_del_rcu(&esd->dep_node);
> + list_del_rcu(&esd->dep_rev_node);
> + }
> +}
> +
> /*
> * NOTE: Must be called with dept_lock held.
> */
> @@ -2384,6 +2414,8 @@ void dept_free_range(void *start, unsigned int sz)
> {
> struct dept_task *dt = dept_task();
> struct dept_class *c, *n;
> + struct dept_event_site_dep *esd, *next_esd;
> + struct dept_event_site *es, *next_es;
> unsigned long flags;
>
> if (unlikely(!dept_working()))
> @@ -2405,6 +2437,24 @@ void dept_free_range(void *start, unsigned int sz)
> while (unlikely(!dept_lock()))
> cpu_relax();
>
> + list_for_each_entry_safe(esd, next_esd, &dept_event_site_deps, all_node) {
> + if (!within((void *)esd, start, sz))
> + continue;
> +
> + disconnect_event_site_dep(esd);
> + list_del(&esd->all_node);
> + }
> +
> + list_for_each_entry_safe(es, next_es, &dept_event_sites, all_node) {
> + if (!within((void *)es, start, sz) &&
> + !within(es->name, start, sz) &&
> + !within(es->func_name, start, sz))
> + continue;
> +
> + disconnect_event_site(es);
> + list_del(&es->all_node);
> + }
> +
> list_for_each_entry_safe(c, n, &dept_classes, all_node) {
> if (!within((void *)c->key, start, sz) &&
> !within(c->name, start, sz))
> @@ -3337,6 +3387,7 @@ void __dept_recover_event(struct dept_event_site_dep *esd,
>
> list_add(&esd->dep_node, &es->dep_head);
> list_add(&esd->dep_rev_node, &rs->dep_rev_head);
> + list_add(&esd->all_node, &dept_event_site_deps);
> check_recover_dl_bfs(esd);
> unlock:
> dept_unlock();
> @@ -3347,6 +3398,23 @@ EXPORT_SYMBOL_GPL(__dept_recover_event);
>
> #define B2KB(B) ((B) / 1024)
>
> +void dept_mark_event_site_used(void *start, void *end)
Nit: I suggest that dept_mark_event_site_used() take pointers to
dept_event_site_init, which would catch the type mismatch with
module::dept_event_sites.
> +{
> + struct dept_event_site_init **evtinitpp;
> +
> + for (evtinitpp = (struct dept_event_site_init **)start;
> + evtinitpp < (struct dept_event_site_init **)end;
> + evtinitpp++) {
> + (*evtinitpp)->evt_site->used = true;
> + (*evtinitpp)->evt_site->func_name = (*evtinitpp)->func_name;
> + list_add(&(*evtinitpp)->evt_site->all_node, &dept_event_sites);
> +
> + pr_info("dept_event_site %s@%s is initialized.\n",
> + (*evtinitpp)->evt_site->name,
> + (*evtinitpp)->evt_site->func_name);
> + }
> +}
> +
> extern char __dept_event_sites_start[], __dept_event_sites_end[];
Related to the above, __dept_event_sites_start and
__dept_event_sites_end can already be properly typed here.
>
> /*
> @@ -3356,20 +3424,11 @@ extern char __dept_event_sites_start[], __dept_event_sites_end[];
> void __init dept_init(void)
> {
> size_t mem_total = 0;
> - struct dept_event_site_init **evtinitpp;
>
> /*
> * dept recover dependency tracking works from now on.
> */
> - for (evtinitpp = (struct dept_event_site_init **)__dept_event_sites_start;
> - evtinitpp < (struct dept_event_site_init **)__dept_event_sites_end;
> - evtinitpp++) {
> - (*evtinitpp)->evt_site->used = true;
> - (*evtinitpp)->evt_site->func_name = (*evtinitpp)->func_name;
> - pr_info("dept_event %s@%s is initialized.\n",
> - (*evtinitpp)->evt_site->name,
> - (*evtinitpp)->evt_site->func_name);
> - }
> + dept_mark_event_site_used(__dept_event_sites_start, __dept_event_sites_end);
> dept_recover_ready = true;
>
> local_irq_disable();
> diff --git a/kernel/module/main.c b/kernel/module/main.c
> index 03ed63f2adf0..82448cdb8ed7 100644
> --- a/kernel/module/main.c
> +++ b/kernel/module/main.c
> @@ -2720,6 +2720,11 @@ static int find_module_sections(struct module *mod, struct load_info *info)
> &mod->dyndbg_info.num_classes);
> #endif
>
> +#ifdef CONFIG_DEPT
> + mod->dept_event_sites = section_objs(info, ".dept.event_sites",
> + sizeof(*mod->dept_event_sites),
> + &mod->num_dept_event_sites);
> +#endif
> return 0;
> }
>
> @@ -3346,6 +3351,14 @@ static int early_mod_check(struct load_info *info, int flags)
> return err;
> }
>
> +static void dept_mark_event_site_used_module(struct module *mod)
> +{
> +#ifdef CONFIG_DEPT
> + dept_mark_event_site_used(mod->dept_event_sites,
> + mod->dept_event_sites + mod->num_dept_event_sites);
> +#endif
> +}
> +
It seems to me that the .dept.event_sites section can be discarded after
the module is initialized. In this case, the section should be prefixed
by ".init" and its address can be obtained at the point of use in
dept_mark_event_site_used_module(), without needing to store it inside
the module struct.
Additionally, what is the reason that the dept_event_site_init data is
not stored in the .dept.event_sites section directly and it requires
a level of indirection?
In general, for my own understanding, I also wonder whether the check to
determine that a dept_event_site is used needs to be done at runtime, or
if it could be done at build time by objtool/modpost.
> /*
> * Allocate and load the module: note that size of section 0 is always
> * zero, and we rely on this for optional sections.
> @@ -3508,6 +3521,8 @@ static int load_module(struct load_info *info, const char __user *uargs,
> /* Done! */
> trace_module_load(mod);
>
> + dept_mark_event_site_used_module(mod);
> +
> return do_init_module(mod);
>
> sysfs_cleanup:
--
Thanks,
Petr
^ permalink raw reply
* [PATCH] module: Remove duplicate freeing of lockdep classes
From: Petr Pavlu @ 2026-01-07 12:22 UTC (permalink / raw)
To: Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen
Cc: Aaron Tomlin, Peter Zijlstra, Ingo Molnar, Will Deacon,
Boqun Feng, Waiman Long, Song Liu, linux-modules, linux-kernel
In the error path of load_module(), under the free_module label, the
code calls lockdep_free_key_range() to release lock classes associated
with the MOD_DATA, MOD_RODATA and MOD_RO_AFTER_INIT module regions, and
subsequently invokes module_deallocate().
Since commit ac3b43283923 ("module: replace module_layout with
module_memory"), the module_deallocate() function calls free_mod_mem(),
which releases the lock classes as well and considers all module
regions.
Attempting to free these classes twice is unnecessary. Remove the
redundant code in load_module().
Signed-off-by: Petr Pavlu <petr.pavlu@suse.com>
---
kernel/module/main.c | 6 ------
1 file changed, 6 deletions(-)
diff --git a/kernel/module/main.c b/kernel/module/main.c
index 710ee30b3bea..bcd259505c8b 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -3544,12 +3544,6 @@ static int load_module(struct load_info *info, const char __user *uargs,
mutex_unlock(&module_mutex);
free_module:
mod_stat_bump_invalid(info, flags);
- /* Free lock-classes; relies on the preceding sync_rcu() */
- for_class_mod_mem_type(type, core_data) {
- lockdep_free_key_range(mod->mem[type].base,
- mod->mem[type].size);
- }
-
module_memory_restore_rox(mod);
module_deallocate(mod, info);
free_copy:
base-commit: 3609fa95fb0f2c1b099e69e56634edb8fc03f87c
--
2.52.0
^ permalink raw reply related
* Re: [PATCH v11 2/8] pkcs7: Allow the signing algo to calculate the digest itself
From: David Howells @ 2026-01-07 13:53 UTC (permalink / raw)
To: Ignat Korchagin
Cc: dhowells, Lukas Wunner, Jarkko Sakkinen, Herbert Xu, Eric Biggers,
Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
Jason A . Donenfeld, Ard Biesheuvel, Stephan Mueller,
linux-crypto, keyrings, linux-modules, linux-kernel
In-Reply-To: <CALrw=nFj9OEsREJ8Kxox3U6N8y=e00ZawxEkCPOb5-6_k=7+nQ@mail.gmail.com>
Ignat Korchagin <ignat@cloudflare.com> wrote:
> > + ret = -ENOMEM;
> > + sig->digest = kmalloc(umax(sinfo->authattrs_len, sig->digest_size),
> > + GFP_KERNEL);
>
> Can we refactor this so we allocate the right size from the start.
The problem is that we don't know the right size until we've tried parsing it.
> Alternatively, should we just unconditionally use this approach
> "overallocating" some times?
In some ways, what I'd rather do is push the hash calculation down into the
crypto/ layer for all public key algos.
Also, we probably don't actually need to copy the authattrs, just retain a
pointer into the source buffer and the length since we don't intend to keep
the digest around beyond the verification procedure. So I might be able to
get away with just a flag saying I don't need to free it.
However, there's an intermediate hash if there are authattrs, so I will need
to store that somewhere - though that could be allocated on demand.
David
^ permalink raw reply
* Re: [PATCH v11 2/8] pkcs7: Allow the signing algo to calculate the digest itself
From: Ignat Korchagin @ 2026-01-07 13:59 UTC (permalink / raw)
To: David Howells
Cc: Lukas Wunner, Jarkko Sakkinen, Herbert Xu, Eric Biggers,
Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
Jason A . Donenfeld, Ard Biesheuvel, Stephan Mueller,
linux-crypto, keyrings, linux-modules, linux-kernel
In-Reply-To: <2366240.1767794004@warthog.procyon.org.uk>
On Wed, Jan 7, 2026 at 1:53 PM David Howells <dhowells@redhat.com> wrote:
>
> Ignat Korchagin <ignat@cloudflare.com> wrote:
>
> > > + ret = -ENOMEM;
> > > + sig->digest = kmalloc(umax(sinfo->authattrs_len, sig->digest_size),
> > > + GFP_KERNEL);
> >
> > Can we refactor this so we allocate the right size from the start.
>
> The problem is that we don't know the right size until we've tried parsing it.
>
> > Alternatively, should we just unconditionally use this approach
> > "overallocating" some times?
>
> In some ways, what I'd rather do is push the hash calculation down into the
> crypto/ layer for all public key algos.
Probably better indeed
> Also, we probably don't actually need to copy the authattrs, just retain a
> pointer into the source buffer and the length since we don't intend to keep
> the digest around beyond the verification procedure. So I might be able to
> get away with just a flag saying I don't need to free it.
>
> However, there's an intermediate hash if there are authattrs, so I will need
> to store that somewhere - though that could be allocated on demand.
>
> David
>
Ignat
^ permalink raw reply
* Re: [PATCH v11 5/8] crypto: Add supplementary info param to asymmetric key signature verification
From: Ignat Korchagin @ 2026-01-07 14:23 UTC (permalink / raw)
To: David Howells
Cc: Lukas Wunner, Jarkko Sakkinen, Herbert Xu, Eric Biggers,
Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
Jason A . Donenfeld, Ard Biesheuvel, Stephan Mueller,
linux-crypto, keyrings, linux-modules, linux-kernel,
David S. Miller
In-Reply-To: <20260105152145.1801972-6-dhowells@redhat.com>
On Mon, Jan 5, 2026 at 3:22 PM David Howells <dhowells@redhat.com> wrote:
>
> Add a supplementary information parameter to the asymmetric key signature
> verification API, in particular crypto_sig_verify() and sig_alg::verify.
> This takes the form of a printable string containing of key=val elements.
>
> This is needed as some algorithms require additional metadata
> (e.g. RSASSA-PSS) and this extra metadata is included in the X.509
> certificates and PKCS#7 messages. Furthermore, keyctl(KEYCTL_PKEY_VERIFY)
> already allows for this to be passed to the kernel, as do the _SIGN,
> _ENCRYPT and _DECRYPT keyctls.
>
> Signed-off-by: David Howells <dhowells@redhat.com>
> cc: Herbert Xu <herbert@gondor.apana.org.au>
> cc: "David S. Miller" <davem@davemloft.net>
> cc: Lukas Wunner <lukas@wunner.de>
> cc: Ignat Korchagin <ignat@cloudflare.com>
> cc: keyrings@vger.kernel.org
> cc: linux-crypto@vger.kernel.org
> ---
> crypto/asymmetric_keys/asymmetric_type.c | 1 +
> crypto/asymmetric_keys/public_key.c | 2 +-
> crypto/asymmetric_keys/signature.c | 1 +
> crypto/ecdsa-p1363.c | 5 +++--
> crypto/ecdsa-x962.c | 5 +++--
> crypto/ecdsa.c | 3 ++-
> crypto/ecrdsa.c | 3 ++-
> crypto/mldsa.c | 3 ++-
> crypto/rsassa-pkcs1.c | 3 ++-
> crypto/sig.c | 3 ++-
> include/crypto/public_key.h | 1 +
> include/crypto/sig.h | 9 ++++++---
> 12 files changed, 26 insertions(+), 13 deletions(-)
>
> diff --git a/crypto/asymmetric_keys/asymmetric_type.c b/crypto/asymmetric_keys/asymmetric_type.c
> index 348966ea2175..dad4f0edfa25 100644
> --- a/crypto/asymmetric_keys/asymmetric_type.c
> +++ b/crypto/asymmetric_keys/asymmetric_type.c
> @@ -596,6 +596,7 @@ static int asymmetric_key_verify_signature(struct kernel_pkey_params *params,
> .digest_size = params->in_len,
> .encoding = params->encoding,
> .hash_algo = params->hash_algo,
> + .info = params->info,
> .digest = (void *)in,
> .s = (void *)in2,
> };
> diff --git a/crypto/asymmetric_keys/public_key.c b/crypto/asymmetric_keys/public_key.c
> index ed6b4b5ae4ef..61dc4f626620 100644
> --- a/crypto/asymmetric_keys/public_key.c
> +++ b/crypto/asymmetric_keys/public_key.c
> @@ -433,7 +433,7 @@ int public_key_verify_signature(const struct public_key *pkey,
> goto error_free_key;
>
> ret = crypto_sig_verify(tfm, sig->s, sig->s_size,
> - sig->digest, sig->digest_size);
> + sig->digest, sig->digest_size, sig->info);
>
> error_free_key:
> kfree_sensitive(key);
> diff --git a/crypto/asymmetric_keys/signature.c b/crypto/asymmetric_keys/signature.c
> index 041d04b5c953..26c0c0112ac4 100644
> --- a/crypto/asymmetric_keys/signature.c
> +++ b/crypto/asymmetric_keys/signature.c
> @@ -29,6 +29,7 @@ void public_key_signature_free(struct public_key_signature *sig)
> kfree(sig->auth_ids[i]);
> kfree(sig->s);
> kfree(sig->digest);
> + kfree(sig->info);
> kfree(sig);
> }
> }
> diff --git a/crypto/ecdsa-p1363.c b/crypto/ecdsa-p1363.c
> index e0c55c64711c..fa987dba1213 100644
> --- a/crypto/ecdsa-p1363.c
> +++ b/crypto/ecdsa-p1363.c
> @@ -18,7 +18,8 @@ struct ecdsa_p1363_ctx {
>
> static int ecdsa_p1363_verify(struct crypto_sig *tfm,
> const void *src, unsigned int slen,
> - const void *digest, unsigned int dlen)
> + const void *digest, unsigned int dlen,
> + const char *info)
> {
> struct ecdsa_p1363_ctx *ctx = crypto_sig_ctx(tfm);
> unsigned int keylen = DIV_ROUND_UP_POW2(crypto_sig_keysize(ctx->child),
> @@ -32,7 +33,7 @@ static int ecdsa_p1363_verify(struct crypto_sig *tfm,
> ecc_digits_from_bytes(src, keylen, sig.r, ndigits);
> ecc_digits_from_bytes(src + keylen, keylen, sig.s, ndigits);
>
> - return crypto_sig_verify(ctx->child, &sig, sizeof(sig), digest, dlen);
> + return crypto_sig_verify(ctx->child, &sig, sizeof(sig), digest, dlen, info);
> }
>
> static unsigned int ecdsa_p1363_key_size(struct crypto_sig *tfm)
> diff --git a/crypto/ecdsa-x962.c b/crypto/ecdsa-x962.c
> index ee71594d10a0..5d7f1078989c 100644
> --- a/crypto/ecdsa-x962.c
> +++ b/crypto/ecdsa-x962.c
> @@ -75,7 +75,8 @@ int ecdsa_get_signature_s(void *context, size_t hdrlen, unsigned char tag,
>
> static int ecdsa_x962_verify(struct crypto_sig *tfm,
> const void *src, unsigned int slen,
> - const void *digest, unsigned int dlen)
> + const void *digest, unsigned int dlen,
> + const char *info)
> {
> struct ecdsa_x962_ctx *ctx = crypto_sig_ctx(tfm);
> struct ecdsa_x962_signature_ctx sig_ctx;
> @@ -89,7 +90,7 @@ static int ecdsa_x962_verify(struct crypto_sig *tfm,
> return err;
>
> return crypto_sig_verify(ctx->child, &sig_ctx.sig, sizeof(sig_ctx.sig),
> - digest, dlen);
> + digest, dlen, info);
> }
>
> static unsigned int ecdsa_x962_key_size(struct crypto_sig *tfm)
> diff --git a/crypto/ecdsa.c b/crypto/ecdsa.c
> index ce8e4364842f..144fd6b9168b 100644
> --- a/crypto/ecdsa.c
> +++ b/crypto/ecdsa.c
> @@ -65,7 +65,8 @@ static int _ecdsa_verify(struct ecc_ctx *ctx, const u64 *hash, const u64 *r, con
> */
> static int ecdsa_verify(struct crypto_sig *tfm,
> const void *src, unsigned int slen,
> - const void *digest, unsigned int dlen)
> + const void *digest, unsigned int dlen,
> + const char *info)
> {
> struct ecc_ctx *ctx = crypto_sig_ctx(tfm);
> size_t bufsize = ctx->curve->g.ndigits * sizeof(u64);
> diff --git a/crypto/ecrdsa.c b/crypto/ecrdsa.c
> index 2c0602f0cd40..59f2d5bb3be4 100644
> --- a/crypto/ecrdsa.c
> +++ b/crypto/ecrdsa.c
> @@ -69,7 +69,8 @@ static const struct ecc_curve *get_curve_by_oid(enum OID oid)
>
> static int ecrdsa_verify(struct crypto_sig *tfm,
> const void *src, unsigned int slen,
> - const void *digest, unsigned int dlen)
> + const void *digest, unsigned int dlen,
> + const char *info)
> {
> struct ecrdsa_ctx *ctx = crypto_sig_ctx(tfm);
> unsigned int ndigits = dlen / sizeof(u64);
> diff --git a/crypto/mldsa.c b/crypto/mldsa.c
> index 2146c774b5ca..ba071d030ab0 100644
> --- a/crypto/mldsa.c
> +++ b/crypto/mldsa.c
> @@ -25,7 +25,8 @@ static int crypto_mldsa_sign(struct crypto_sig *tfm,
>
> static int crypto_mldsa_verify(struct crypto_sig *tfm,
> const void *sig, unsigned int sig_len,
> - const void *msg, unsigned int msg_len)
> + const void *msg, unsigned int msg_len,
> + const char *info)
> {
> const struct crypto_mldsa_ctx *ctx = crypto_sig_ctx(tfm);
>
> diff --git a/crypto/rsassa-pkcs1.c b/crypto/rsassa-pkcs1.c
> index 94fa5e9600e7..6283050e609a 100644
> --- a/crypto/rsassa-pkcs1.c
> +++ b/crypto/rsassa-pkcs1.c
> @@ -215,7 +215,8 @@ static int rsassa_pkcs1_sign(struct crypto_sig *tfm,
>
> static int rsassa_pkcs1_verify(struct crypto_sig *tfm,
> const void *src, unsigned int slen,
> - const void *digest, unsigned int dlen)
> + const void *digest, unsigned int dlen,
> + const char *info)
> {
> struct sig_instance *inst = sig_alg_instance(tfm);
> struct rsassa_pkcs1_inst_ctx *ictx = sig_instance_ctx(inst);
> diff --git a/crypto/sig.c b/crypto/sig.c
> index beba745b6405..c56fea3a53ae 100644
> --- a/crypto/sig.c
> +++ b/crypto/sig.c
> @@ -92,7 +92,8 @@ static int sig_default_sign(struct crypto_sig *tfm,
>
> static int sig_default_verify(struct crypto_sig *tfm,
> const void *src, unsigned int slen,
> - const void *dst, unsigned int dlen)
> + const void *dst, unsigned int dlen,
> + const char *info)
> {
> return -ENOSYS;
> }
> diff --git a/include/crypto/public_key.h b/include/crypto/public_key.h
> index e4ec8003a3a4..1e9a1e4e9916 100644
> --- a/include/crypto/public_key.h
> +++ b/include/crypto/public_key.h
> @@ -47,6 +47,7 @@ struct public_key_signature {
> u32 s_size; /* Number of bytes in signature */
> u32 digest_size; /* Number of bytes in digest */
> bool algo_does_hash; /* Public key algo does its own hashing */
> + char *info; /* Supplementary parameters */
> const char *pkey_algo;
> const char *hash_algo;
> const char *encoding;
> diff --git a/include/crypto/sig.h b/include/crypto/sig.h
> index fa6dafafab3f..885fa6487780 100644
> --- a/include/crypto/sig.h
> +++ b/include/crypto/sig.h
> @@ -56,7 +56,8 @@ struct sig_alg {
> void *dst, unsigned int dlen);
> int (*verify)(struct crypto_sig *tfm,
> const void *src, unsigned int slen,
> - const void *digest, unsigned int dlen);
> + const void *digest, unsigned int dlen,
> + const char *info);
> int (*set_pub_key)(struct crypto_sig *tfm,
> const void *key, unsigned int keylen);
> int (*set_priv_key)(struct crypto_sig *tfm,
> @@ -209,16 +210,18 @@ static inline int crypto_sig_sign(struct crypto_sig *tfm,
> * @slen: source length
> * @digest: digest
> * @dlen: digest length
> + * @info: Additional parameters as a set of k=v
> *
> * Return: zero on verification success; error code in case of error.
> */
> static inline int crypto_sig_verify(struct crypto_sig *tfm,
> const void *src, unsigned int slen,
> - const void *digest, unsigned int dlen)
> + const void *digest, unsigned int dlen,
> + const char *info)
> {
> struct sig_alg *alg = crypto_sig_alg(tfm);
>
> - return alg->verify(tfm, src, slen, digest, dlen);
> + return alg->verify(tfm, src, slen, digest, dlen, info);
> }
>
> /**
>
Reviewed-by: Ignat Korchagin <ignat@cloudflare.com>
^ permalink raw reply
* Re: [PATCH v11 6/8] crypto: Add RSASSA-PSS support
From: Ignat Korchagin @ 2026-01-07 16:24 UTC (permalink / raw)
To: David Howells
Cc: Lukas Wunner, Jarkko Sakkinen, Herbert Xu, Eric Biggers,
Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
Jason A . Donenfeld, Ard Biesheuvel, Stephan Mueller,
linux-crypto, keyrings, linux-modules, linux-kernel,
Tadeusz Struk, David S. Miller
In-Reply-To: <20260105152145.1801972-7-dhowells@redhat.com>
Hi David,
On Mon, Jan 5, 2026 at 3:23 PM David Howells <dhowells@redhat.com> wrote:
>
> Add support for RSASSA-PSS [RFC8017 sec 8.1] signature verification support
> to the RSA driver in crypto/. Note that signing support is not provided.
>
> The verification function requires an info string formatted as a
> space-separated list of key=value pairs. The following parameters need to
> be provided:
>
> (1) sighash=<algo>
>
> The hash algorithm to be used to digest the data.
>
> (2) pss_mask=<type>,...
>
> The mask generation function (MGF) and its parameters.
>
> (3) pss_salt=<len>
>
> The length of the salt used.
>
> The only MGF currently supported is "mgf1". This takes an additional
> parameter indicating the mask-generating hash (which need not be the same
> as the data hash). E.g.:
>
> "sighash=sha256 pss_mask=mgf1,sha256 pss_salt=32"
>
> Signed-off-by: David Howells <dhowells@redhat.com>
> cc: Tadeusz Struk <tadeusz.struk@intel.com>
> cc: Herbert Xu <herbert@gondor.apana.org.au>
> cc: David S. Miller <davem@davemloft.net>
> cc: Lukas Wunner <lukas@wunner.de>
> cc: Ignat Korchagin <ignat@cloudflare.com>
> cc: keyrings@vger.kernel.org
> cc: linux-crypto@vger.kernel.org
> ---
> crypto/Makefile | 1 +
> crypto/rsa.c | 8 +
> crypto/rsassa-pss.c | 397 ++++++++++++++++++++++++++++++++++
> include/crypto/internal/rsa.h | 2 +
> 4 files changed, 408 insertions(+)
> create mode 100644 crypto/rsassa-pss.c
>
> diff --git a/crypto/Makefile b/crypto/Makefile
> index 267d5403045b..5c91440d1751 100644
> --- a/crypto/Makefile
> +++ b/crypto/Makefile
> @@ -50,6 +50,7 @@ rsa_generic-y += rsa.o
> rsa_generic-y += rsa_helper.o
> rsa_generic-y += rsa-pkcs1pad.o
> rsa_generic-y += rsassa-pkcs1.o
> +rsa_generic-y += rsassa-pss.o
> obj-$(CONFIG_CRYPTO_RSA) += rsa_generic.o
>
> $(obj)/ecdsasignature.asn1.o: $(obj)/ecdsasignature.asn1.c $(obj)/ecdsasignature.asn1.h
> diff --git a/crypto/rsa.c b/crypto/rsa.c
> index 6c7734083c98..189a09d54c16 100644
> --- a/crypto/rsa.c
> +++ b/crypto/rsa.c
> @@ -10,6 +10,7 @@
> #include <linux/mpi.h>
> #include <crypto/internal/rsa.h>
> #include <crypto/internal/akcipher.h>
> +#include <crypto/internal/sig.h>
> #include <crypto/akcipher.h>
> #include <crypto/algapi.h>
>
> @@ -414,8 +415,14 @@ static int __init rsa_init(void)
> if (err)
> goto err_unregister_rsa_pkcs1pad;
>
> + err = crypto_register_sig(&rsassa_pss_alg);
> + if (err)
> + goto err_rsassa_pss;
> +
> return 0;
>
> +err_rsassa_pss:
> + crypto_unregister_template(&rsassa_pkcs1_tmpl);
> err_unregister_rsa_pkcs1pad:
> crypto_unregister_template(&rsa_pkcs1pad_tmpl);
> err_unregister_rsa:
> @@ -425,6 +432,7 @@ static int __init rsa_init(void)
>
> static void __exit rsa_exit(void)
> {
> + crypto_unregister_sig(&rsassa_pss_alg);
> crypto_unregister_template(&rsassa_pkcs1_tmpl);
> crypto_unregister_template(&rsa_pkcs1pad_tmpl);
> crypto_unregister_akcipher(&rsa);
> diff --git a/crypto/rsassa-pss.c b/crypto/rsassa-pss.c
> new file mode 100644
> index 000000000000..7f27e8fa6fa7
> --- /dev/null
> +++ b/crypto/rsassa-pss.c
> @@ -0,0 +1,397 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * RSA Signature Scheme combined with EMSA-PSS encoding (RFC 8017 sec 8.2)
> + *
> + * https://www.rfc-editor.org/rfc/rfc8017#section-8.1
> + *
> + * Copyright (c) 2025 Red Hat
> + */
> +
> +#define pr_fmt(fmt) "RSAPSS: "fmt
> +#include <linux/ctype.h>
> +#include <linux/module.h>
> +#include <linux/oid_registry.h>
> +#include <linux/parser.h>
> +#include <linux/scatterlist.h>
> +#include <crypto/akcipher.h>
> +#include <crypto/algapi.h>
> +#include <crypto/hash.h>
> +#include <crypto/sig.h>
> +#include <crypto/internal/akcipher.h>
> +#include <crypto/internal/rsa.h>
> +#include <crypto/internal/sig.h>
> +
> +struct rsassa_pss_ctx {
> + struct crypto_akcipher *rsa;
> + unsigned int key_size;
> + unsigned int salt_len;
> + char *pss_hash;
> + char *mgf1_hash;
> +};
> +
> +enum {
> + rsassa_pss_verify_hash_algo,
> + rsassa_pss_verify_pss_mask,
> + rsassa_pss_verify_pss_salt,
> +};
> +
> +static const match_table_t rsassa_pss_verify_params = {
> + { rsassa_pss_verify_hash_algo, "sighash=%s" },
> + { rsassa_pss_verify_pss_mask, "pss_mask=%s" },
> + { rsassa_pss_verify_pss_salt, "pss_salt=%u" },
> + {}
> +};
> +
> +/*
> + * Parse the signature parameters out of the info string.
> + */
> +static int rsassa_pss_vinfo_parse(struct rsassa_pss_ctx *ctx,
> + char *info)
> +{
> + substring_t args[MAX_OPT_ARGS];
> + char *p, *q;
> +
> + ctx->pss_hash = NULL;
> + ctx->mgf1_hash = NULL;
> + ctx->salt_len = 0;
> +
> + for (p = info; p && *p;) {
> + if (isspace(*p)) {
> + p++;
> + continue;
> + }
> + q = p++;
> + while (*p && !isspace(*p))
> + p++;
> +
> + if (!*p)
> + p = NULL;
> + else
> + *p++ = 0;
A lot of pointers and arithmetic here. Wouldn't it be easier to do
something like in [1]?
> + switch (match_token(q, rsassa_pss_verify_params, args)) {
> + case rsassa_pss_verify_hash_algo:
> + *args[0].to = 0;
> + ctx->pss_hash = args[0].from;
> + break;
> + case rsassa_pss_verify_pss_mask:
> + if (memcmp(args[0].from, "mgf1", 4) != 0)
> + return -ENOPKG;
> + if (args[0].from[4] != ',')
> + return -EINVAL;
> + args[0].from += 5;
> + if (args[0].from >= args[0].to)
> + return -EINVAL;
> + *args[0].to = 0;
> + ctx->mgf1_hash = args[0].from;
> + break;
> + case rsassa_pss_verify_pss_salt:
> + if (match_uint(&args[0], &ctx->salt_len) < 0)
> + return -EINVAL;
> + break;
> + default:
> + pr_debug("Unknown info param\n");
> + return -EINVAL; /* Ignoring it might be better. */
> + }
> + }
> +
> + if (!ctx->pss_hash ||
> + !ctx->mgf1_hash ||
> + !ctx->salt_len)
> + return -EINVAL;
> + return 0;
> +}
> +
> +DEFINE_FREE(crypto_free_shash, struct crypto_shash*,
> + if (!IS_ERR_OR_NULL(_T)) { crypto_free_shash(_T); });
Is this useful enough to go into some commonly used header for shash?
> +/*
> + * Perform mask = MGF1(mgfSeed, masklen) - RFC8017 appendix B.2.1.
> + */
> +static int MGF1(struct rsassa_pss_ctx *ctx,
> + const u8 *mgfSeed, unsigned int mgfSeed_len,
> + u8 *mask, unsigned int maskLen)
> +{
> + struct crypto_shash *hash_tfm __free(crypto_free_shash) = NULL;
> + struct shash_desc *Hash __free(kfree) = NULL;
So even though x509/pkcs7 code now has a counterexample (partially due
to my fault) seems the consensus [2] is to declare and initialise the
variable with the __free attribute at the same time meaning it is OK
to declare the variables later and not follow the "declaration at the
top" rule.
> + unsigned int counter, count_to, hLen, T_len;
> + __be32 *C;
> + int err;
> + u8 *T, *t, *to_hash;
> +
> + hash_tfm = crypto_alloc_shash(ctx->mgf1_hash, 0, 0);
> + if (IS_ERR(hash_tfm))
> + return PTR_ERR(hash_tfm);
> +
> + hLen = crypto_shash_digestsize(hash_tfm);
> + count_to = DIV_ROUND_UP(maskLen, hLen);
> + T_len = hLen * count_to;
> +
> + Hash = kmalloc(roundup(sizeof(struct shash_desc) +
> + crypto_shash_descsize(hash_tfm), 64) +
> + roundup(T_len, 64) + /* T */
> + roundup(mgfSeed_len + 4, 64), /* mgfSeed||C */
> + GFP_KERNEL);
> + if (!Hash)
> + return -ENOMEM;
> +
> + Hash->tfm = hash_tfm;
> +
> + /* 2: Let T be the empty octet string. */
> + T = (void *)Hash +
> + roundup(sizeof(struct shash_desc) +
> + crypto_shash_descsize(hash_tfm), 64);
> +
> + /* 3: Generate the mask. */
> + to_hash = T + roundup(T_len, 64);
> + memcpy(to_hash, mgfSeed, mgfSeed_len);
> + C = (__be32 *)(to_hash + mgfSeed_len);
> +
> + t = T;
> + for (counter = 0; counter < count_to; counter++) {
> + /* 3A: C = I2OSP(counter, 4). */
> + put_unaligned_be32(counter, C);
> +
> + /* 3B: T = T || Hash(mgfSeed || C). */
> + err = crypto_shash_digest(Hash, to_hash, mgfSeed_len + 4, t);
> + if (err < 0)
> + return err;
> +
> + t += hLen;
> + }
> +
> + /* 4: Output T to mask */
> + memcpy(mask, T, maskLen);
> + return 0;
> +}
> +
> +/*
> + * Perform EMSA-PSS-VERIFY(M, EM, emBits) - RFC8017 sec 9.1.2.
> + */
> +static int emsa_pss_verify(struct rsassa_pss_ctx *ctx,
> + const u8 *M, unsigned int M_len,
> + const u8 *EM, unsigned int emLen)
> +{
> + struct crypto_shash *hash_tfm __free(crypto_free_shash);
> + struct shash_desc *Hash __free(kfree) = NULL;
Same here: declare when initialised
> + unsigned int emBits, hLen, sLen, DB_len;
> + const u8 *maskedDB, *H;
> + u8 *mHash, *dbMask, *DB, *salt, *Mprime, *Hprime;
> + int err, i;
> +
> + emBits = 8 - fls(EM[0]);
> + emBits = emLen * 8 - emBits;
> +
> + hash_tfm = crypto_alloc_shash(ctx->pss_hash, 0, 0);
> + if (IS_ERR(hash_tfm))
> + return PTR_ERR(hash_tfm);
> +
> + hLen = crypto_shash_digestsize(hash_tfm);
> + sLen = ctx->salt_len;
> +
> + if (sLen > 65536 ||
> + emBits < 8 * (hLen + sLen) + 9)
> + return -EBADMSG;
> +
> + DB_len = emLen - hLen - 1;
> +
> + Hash = kmalloc(roundup(sizeof(struct shash_desc) +
> + crypto_shash_descsize(hash_tfm), 64) +
> + roundup(hLen, 64) + /* mHash */
> + roundup(DB_len, 64) + /* DB and dbMask */
> + roundup(8 + hLen + sLen, 64) + /* M' */
> + roundup(hLen, 64), /* H' */
> + GFP_KERNEL);
> + if (!Hash)
> + return -ENOMEM;
> +
> + Hash->tfm = hash_tfm;
> +
> + mHash = (void *)Hash +
> + roundup(sizeof(struct shash_desc) +
> + crypto_shash_descsize(hash_tfm), 64);
> + DB = dbMask = mHash + roundup(hLen, 64);
> + Mprime = dbMask + roundup(DB_len, 64);
> + Hprime = Mprime + roundup(8 + hLen + sLen, 64);
> +
> + /* 1. Check len M against hash input limitation. */
> + /* The standard says ~2EiB for SHA1, so I think we can ignore this. */
> +
> + /* 2. mHash = Hash(M).
> + * In theory, we would do:
> + * err = crypto_shash_digest(Hash, M, M_len, mHash);
> + * but the caller is assumed to already have done that for us.
> + */
> + if (M_len != hLen)
> + return -EINVAL;
> + memcpy(mHash, M, hLen);
> +
> + /* 3. Check emLen against hLen + sLen + 2. */
> + if (emLen < hLen + sLen + 2)
> + return -EBADMSG;
> +
> + /* 4. Validate EM. */
> + if (EM[emLen - 1] != 0xbc)
> + return -EKEYREJECTED;
> +
> + /* 5. Pick maskedDB and H. */
> + maskedDB = EM;
> + H = EM + DB_len;
> +
> + /* 6. Check leftmost 8emLen-emBits bits of maskedDB are 0. */
> + /* Can only find emBits by counting the zeros on the Left. */
> +
> + /* 7. Let dbMask = MGF(H, emLen - hLen - 1). */
> + err = MGF1(ctx, H, hLen, dbMask, DB_len);
> + if (err < 0)
> + return err;
> +
> + /* 8. Let DB = maskedDB XOR dbMask. */
> + for (i = 0; i < DB_len; i++)
> + DB[i] = maskedDB[i] ^ dbMask[i];
> +
> + /* 9. Set leftmost bits in DB to zero. */
> + int z = 8 * emLen - emBits;
> + if (z > 0) {
> + if (z >= 8) {
> + DB[0] = 0;
> + } else {
> + z = 8 - z;
> + DB[0] &= (1 << z) - 1;
> + }
> + }
> +
> + /* 10. Check the left part of DB is {0,0,...,1}. */
> + for (i = 0; i < emLen - hLen - sLen - 2; i++)
> + if (DB[i] != 0)
> + return -EKEYREJECTED;
> + if (DB[i] != 0x01)
> + return -EKEYREJECTED;
> +
> + /* 11. Let salt be the last sLen octets of DB. */
> + salt = DB + DB_len - sLen;
> +
> + /* 12. Let M' be 00 00 00 00 00 00 00 00 || mHash || salt. */
> + memset(Mprime, 0, 8);
> + memcpy(Mprime + 8, mHash, hLen);
> + memcpy(Mprime + 8 + hLen, salt, sLen);
> +
> + /* 13. Let H' = Hash(M'). */
> + err = crypto_shash_digest(Hash, Mprime, 8 + hLen + sLen, Hprime);
> + if (err < 0)
> + return err;
> +
> + /* 14. Check H = H'. */
> + if (memcmp(H, Hprime, hLen) != 0)
> + return -EKEYREJECTED;
> + return 0;
> +}
> +
> +/*
> + * Perform RSASSA-PSS-VERIFY((n,e),M,S) - RFC8017 sec 8.1.2.
> + */
> +static int rsassa_pss_verify(struct crypto_sig *tfm,
> + const void *src, unsigned int slen,
> + const void *digest, unsigned int dlen,
> + const char *info)
> +{
> + struct akcipher_request *rsa_req __free(kfree) = NULL;
And here: declare at the time of init.
> + struct rsassa_pss_ctx *ctx = crypto_sig_ctx(tfm);
> + struct crypto_wait cwait;
> + struct scatterlist sg;
> + unsigned int rsa_reqsize = crypto_akcipher_reqsize(ctx->rsa);
> + char *str __free(kfree) = NULL;
Declare at the time of init.
> + u8 *EM;
> + int err;
> +
> + if (!info)
> + return -EINVAL;
> +
> + str = kstrdup(info, GFP_KERNEL);
> + if (!str)
> + return -ENOMEM;
> +
> + err = rsassa_pss_vinfo_parse(ctx, str);
> + if (err < 0)
> + return err;
> +
> + /* RFC8017 sec 8.1.2 step 1 - length checking */
> + if (!ctx->key_size || slen != ctx->key_size)
> + return -EINVAL;
> +
> + /* RFC8017 sec 8.1.2 step 2 - RSA verification */
> + rsa_req = kmalloc(sizeof(*rsa_req) + rsa_reqsize + ctx->key_size,
> + GFP_KERNEL);
> + if (!rsa_req)
> + return -ENOMEM;
> +
> + EM = (u8 *)(rsa_req + 1) + rsa_reqsize;
> + memcpy(EM, src, slen);
> +
> + crypto_init_wait(&cwait);
> + sg_init_one(&sg, EM, slen);
> + akcipher_request_set_tfm(rsa_req, ctx->rsa);
> + akcipher_request_set_crypt(rsa_req, &sg, &sg, slen, slen);
> + akcipher_request_set_callback(rsa_req, CRYPTO_TFM_REQ_MAY_SLEEP,
> + crypto_req_done, &cwait);
> +
> + err = crypto_akcipher_encrypt(rsa_req);
> + err = crypto_wait_req(err, &cwait);
> + if (err)
> + return err;
> +
> + /* RFC 8017 sec 8.1.2 step 3 - EMSA-PSS(M, EM, modbits-1) */
> + return emsa_pss_verify(ctx, digest, dlen, EM, slen);
> +}
> +
> +static unsigned int rsassa_pss_key_size(struct crypto_sig *tfm)
> +{
> + struct rsassa_pss_ctx *ctx = crypto_sig_ctx(tfm);
> +
> + return ctx->key_size * BITS_PER_BYTE;
> +}
> +
> +static int rsassa_pss_set_pub_key(struct crypto_sig *tfm,
> + const void *key, unsigned int keylen)
> +{
> + struct rsassa_pss_ctx *ctx = crypto_sig_ctx(tfm);
> +
> + return rsa_set_key(ctx->rsa, &ctx->key_size, RSA_PUB, key, keylen);
> +}
> +
> +static int rsassa_pss_init_tfm(struct crypto_sig *tfm)
> +{
> + struct crypto_akcipher *rsa;
> + struct rsassa_pss_ctx *ctx = crypto_sig_ctx(tfm);
> +
> + rsa = crypto_alloc_akcipher("rsa", 0, 0);
> + if (IS_ERR(rsa))
> + return PTR_ERR(rsa);
> +
> + ctx->rsa = rsa;
> + return 0;
> +}
> +
> +static void rsassa_pss_exit_tfm(struct crypto_sig *tfm)
> +{
> + struct rsassa_pss_ctx *ctx = crypto_sig_ctx(tfm);
> +
> + crypto_free_akcipher(ctx->rsa);
> +}
> +
> +struct sig_alg rsassa_pss_alg = {
> + .verify = rsassa_pss_verify,
> + .set_pub_key = rsassa_pss_set_pub_key,
> + .key_size = rsassa_pss_key_size,
> + .init = rsassa_pss_init_tfm,
> + .exit = rsassa_pss_exit_tfm,
> + .base = {
> + .cra_name = "rsassa-pss",
> + .cra_driver_name = "rsassa-pss-generic",
> + .cra_priority = 100,
> + .cra_module = THIS_MODULE,
> + .cra_ctxsize = sizeof(struct rsassa_pss_ctx),
> + },
> +};
> +
> +MODULE_ALIAS_CRYPTO("rsassa-pss");
> diff --git a/include/crypto/internal/rsa.h b/include/crypto/internal/rsa.h
> index 071a1951b992..d7f38a273949 100644
> --- a/include/crypto/internal/rsa.h
> +++ b/include/crypto/internal/rsa.h
> @@ -83,4 +83,6 @@ static inline int rsa_set_key(struct crypto_akcipher *child,
>
> extern struct crypto_template rsa_pkcs1pad_tmpl;
> extern struct crypto_template rsassa_pkcs1_tmpl;
> +extern struct sig_alg rsassa_pss_alg;
> +
> #endif
>
Ignat
[1]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/security/keys/trusted-keys/trusted_tpm1.c?id=720a485d12c590750f40f4ffbe41e36725f43f3d#n718
[2]: https://lore.kernel.org/lkml/58fd478f408a34b578ee8d949c5c4b4da4d4f41d.camel@HansenPartnership.com/T/
^ permalink raw reply
* [PATCH v2 02/11] rust: macros: use `quote!` from vendored crate
From: Gary Guo @ 2026-01-07 16:15 UTC (permalink / raw)
To: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Brendan Higgins, David Gow, Rae Moar,
Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
Aaron Tomlin, Tamir Duberstein, Igor Korotin,
José Expósito, Greg Kroah-Hartman
Cc: rust-for-linux, Guilherme Giacomo Simoes, linux-kernel,
linux-kselftest, kunit-dev, linux-modules
In-Reply-To: <20260107161729.3855851-1-gary@kernel.org>
From: Gary Guo <gary@garyguo.net>
With `quote` crate now vendored in the kernel, we can remove our custom
`quote!` macro implementation and just rely on that crate instead.
The `quote` crate uses types from the `proc-macro2` library so we also
update to use that, and perform conversion in the top-level lib.rs.
Clippy complains about unnecessary `.to_string()` as `proc-macro2`
provides additional `PartialEq` impl, so they are removed.
Reviewed-by: Tamir Duberstein <tamird@gmail.com>
Reviewed-by: Benno Lossin <lossin@kernel.org>
Signed-off-by: Gary Guo <gary@garyguo.net>
---
rust/macros/concat_idents.rs | 2 +-
rust/macros/export.rs | 4 +-
rust/macros/fmt.rs | 4 +-
rust/macros/helpers.rs | 4 +-
rust/macros/kunit.rs | 5 +-
rust/macros/lib.rs | 21 ++--
rust/macros/module.rs | 6 +-
rust/macros/paste.rs | 2 +-
rust/macros/quote.rs | 182 -----------------------------------
rust/macros/vtable.rs | 7 +-
10 files changed, 32 insertions(+), 205 deletions(-)
delete mode 100644 rust/macros/quote.rs
diff --git a/rust/macros/concat_idents.rs b/rust/macros/concat_idents.rs
index 7e4b450f3a507..12cb231c3d715 100644
--- a/rust/macros/concat_idents.rs
+++ b/rust/macros/concat_idents.rs
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: GPL-2.0
-use proc_macro::{token_stream, Ident, TokenStream, TokenTree};
+use proc_macro2::{token_stream, Ident, TokenStream, TokenTree};
use crate::helpers::expect_punct;
diff --git a/rust/macros/export.rs b/rust/macros/export.rs
index a08f6337d5c8d..92d9b30971929 100644
--- a/rust/macros/export.rs
+++ b/rust/macros/export.rs
@@ -1,7 +1,9 @@
// SPDX-License-Identifier: GPL-2.0
+use proc_macro2::TokenStream;
+use quote::quote;
+
use crate::helpers::function_name;
-use proc_macro::TokenStream;
/// Please see [`crate::export`] for documentation.
pub(crate) fn export(_attr: TokenStream, ts: TokenStream) -> TokenStream {
diff --git a/rust/macros/fmt.rs b/rust/macros/fmt.rs
index 2f4b9f6e22110..19f709262552b 100644
--- a/rust/macros/fmt.rs
+++ b/rust/macros/fmt.rs
@@ -1,8 +1,10 @@
// SPDX-License-Identifier: GPL-2.0
-use proc_macro::{Ident, TokenStream, TokenTree};
use std::collections::BTreeSet;
+use proc_macro2::{Ident, TokenStream, TokenTree};
+use quote::quote_spanned;
+
/// Please see [`crate::fmt`] for documentation.
pub(crate) fn fmt(input: TokenStream) -> TokenStream {
let mut input = input.into_iter();
diff --git a/rust/macros/helpers.rs b/rust/macros/helpers.rs
index 365d7eb499c08..13fafaba12261 100644
--- a/rust/macros/helpers.rs
+++ b/rust/macros/helpers.rs
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: GPL-2.0
-use proc_macro::{token_stream, Group, Ident, TokenStream, TokenTree};
+use proc_macro2::{token_stream, Group, Ident, TokenStream, TokenTree};
pub(crate) fn try_ident(it: &mut token_stream::IntoIter) -> Option<String> {
if let Some(TokenTree::Ident(ident)) = it.next() {
@@ -86,7 +86,7 @@ pub(crate) fn function_name(input: TokenStream) -> Option<Ident> {
let mut input = input.into_iter();
while let Some(token) = input.next() {
match token {
- TokenTree::Ident(i) if i.to_string() == "fn" => {
+ TokenTree::Ident(i) if i == "fn" => {
if let Some(TokenTree::Ident(i)) = input.next() {
return Some(i);
}
diff --git a/rust/macros/kunit.rs b/rust/macros/kunit.rs
index b395bb0536959..5cd6aa5eef07d 100644
--- a/rust/macros/kunit.rs
+++ b/rust/macros/kunit.rs
@@ -4,10 +4,11 @@
//!
//! Copyright (c) 2023 José Expósito <jose.exposito89@gmail.com>
-use proc_macro::{Delimiter, Group, TokenStream, TokenTree};
use std::collections::HashMap;
use std::fmt::Write;
+use proc_macro2::{Delimiter, Group, TokenStream, TokenTree};
+
pub(crate) fn kunit_tests(attr: TokenStream, ts: TokenStream) -> TokenStream {
let attr = attr.to_string();
@@ -59,7 +60,7 @@ pub(crate) fn kunit_tests(attr: TokenStream, ts: TokenStream) -> TokenStream {
}
_ => (),
},
- TokenTree::Ident(i) if i.to_string() == "fn" && attributes.contains_key("test") => {
+ TokenTree::Ident(i) if i == "fn" && attributes.contains_key("test") => {
if let Some(TokenTree::Ident(test_name)) = body_it.next() {
tests.push((test_name, attributes.remove("cfg").unwrap_or_default()))
}
diff --git a/rust/macros/lib.rs b/rust/macros/lib.rs
index b38002151871a..945982c21f703 100644
--- a/rust/macros/lib.rs
+++ b/rust/macros/lib.rs
@@ -11,8 +11,6 @@
// to avoid depending on the full `proc_macro_span` on Rust >= 1.88.0.
#![cfg_attr(not(CONFIG_RUSTC_HAS_SPAN_FILE), feature(proc_macro_span))]
-#[macro_use]
-mod quote;
mod concat_idents;
mod export;
mod fmt;
@@ -132,7 +130,7 @@
/// the kernel module.
#[proc_macro]
pub fn module(ts: TokenStream) -> TokenStream {
- module::module(ts)
+ module::module(ts.into()).into()
}
/// Declares or implements a vtable trait.
@@ -207,7 +205,7 @@ pub fn module(ts: TokenStream) -> TokenStream {
/// [`kernel::error::VTABLE_DEFAULT_ERROR`]: ../kernel/error/constant.VTABLE_DEFAULT_ERROR.html
#[proc_macro_attribute]
pub fn vtable(attr: TokenStream, ts: TokenStream) -> TokenStream {
- vtable::vtable(attr, ts)
+ vtable::vtable(attr.into(), ts.into()).into()
}
/// Export a function so that C code can call it via a header file.
@@ -230,7 +228,7 @@ pub fn vtable(attr: TokenStream, ts: TokenStream) -> TokenStream {
/// automatically exported with `EXPORT_SYMBOL_GPL`.
#[proc_macro_attribute]
pub fn export(attr: TokenStream, ts: TokenStream) -> TokenStream {
- export::export(attr, ts)
+ export::export(attr.into(), ts.into()).into()
}
/// Like [`core::format_args!`], but automatically wraps arguments in [`kernel::fmt::Adapter`].
@@ -248,7 +246,7 @@ pub fn export(attr: TokenStream, ts: TokenStream) -> TokenStream {
/// [`pr_info!`]: ../kernel/macro.pr_info.html
#[proc_macro]
pub fn fmt(input: TokenStream) -> TokenStream {
- fmt::fmt(input)
+ fmt::fmt(input.into()).into()
}
/// Concatenate two identifiers.
@@ -306,7 +304,7 @@ pub fn fmt(input: TokenStream) -> TokenStream {
/// ```
#[proc_macro]
pub fn concat_idents(ts: TokenStream) -> TokenStream {
- concat_idents::concat_idents(ts)
+ concat_idents::concat_idents(ts.into()).into()
}
/// Paste identifiers together.
@@ -444,9 +442,12 @@ pub fn concat_idents(ts: TokenStream) -> TokenStream {
/// [`paste`]: https://docs.rs/paste/
#[proc_macro]
pub fn paste(input: TokenStream) -> TokenStream {
- let mut tokens = input.into_iter().collect();
+ let mut tokens = proc_macro2::TokenStream::from(input).into_iter().collect();
paste::expand(&mut tokens);
- tokens.into_iter().collect()
+ tokens
+ .into_iter()
+ .collect::<proc_macro2::TokenStream>()
+ .into()
}
/// Registers a KUnit test suite and its test cases using a user-space like syntax.
@@ -473,5 +474,5 @@ pub fn paste(input: TokenStream) -> TokenStream {
/// ```
#[proc_macro_attribute]
pub fn kunit_tests(attr: TokenStream, ts: TokenStream) -> TokenStream {
- kunit::kunit_tests(attr, ts)
+ kunit::kunit_tests(attr.into(), ts.into()).into()
}
diff --git a/rust/macros/module.rs b/rust/macros/module.rs
index 80cb9b16f5aaf..b855a2b586e18 100644
--- a/rust/macros/module.rs
+++ b/rust/macros/module.rs
@@ -1,9 +1,11 @@
// SPDX-License-Identifier: GPL-2.0
-use crate::helpers::*;
-use proc_macro::{token_stream, Delimiter, Literal, TokenStream, TokenTree};
use std::fmt::Write;
+use proc_macro2::{token_stream, Delimiter, Literal, TokenStream, TokenTree};
+
+use crate::helpers::*;
+
fn expect_string_array(it: &mut token_stream::IntoIter) -> Vec<String> {
let group = expect_group(it);
assert_eq!(group.delimiter(), Delimiter::Bracket);
diff --git a/rust/macros/paste.rs b/rust/macros/paste.rs
index cce712d19855b..2181e312a7d32 100644
--- a/rust/macros/paste.rs
+++ b/rust/macros/paste.rs
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: GPL-2.0
-use proc_macro::{Delimiter, Group, Ident, Spacing, Span, TokenTree};
+use proc_macro2::{Delimiter, Group, Ident, Spacing, Span, TokenTree};
fn concat_helper(tokens: &[TokenTree]) -> Vec<(String, Span)> {
let mut tokens = tokens.iter();
diff --git a/rust/macros/quote.rs b/rust/macros/quote.rs
deleted file mode 100644
index ddfc21577539c..0000000000000
--- a/rust/macros/quote.rs
+++ /dev/null
@@ -1,182 +0,0 @@
-// SPDX-License-Identifier: Apache-2.0 OR MIT
-
-use proc_macro::{TokenStream, TokenTree};
-
-pub(crate) trait ToTokens {
- fn to_tokens(&self, tokens: &mut TokenStream);
-}
-
-impl<T: ToTokens> ToTokens for Option<T> {
- fn to_tokens(&self, tokens: &mut TokenStream) {
- if let Some(v) = self {
- v.to_tokens(tokens);
- }
- }
-}
-
-impl ToTokens for proc_macro::Group {
- fn to_tokens(&self, tokens: &mut TokenStream) {
- tokens.extend([TokenTree::from(self.clone())]);
- }
-}
-
-impl ToTokens for proc_macro::Ident {
- fn to_tokens(&self, tokens: &mut TokenStream) {
- tokens.extend([TokenTree::from(self.clone())]);
- }
-}
-
-impl ToTokens for TokenTree {
- fn to_tokens(&self, tokens: &mut TokenStream) {
- tokens.extend([self.clone()]);
- }
-}
-
-impl ToTokens for TokenStream {
- fn to_tokens(&self, tokens: &mut TokenStream) {
- tokens.extend(self.clone());
- }
-}
-
-/// Converts tokens into [`proc_macro::TokenStream`] and performs variable interpolations with
-/// the given span.
-///
-/// This is a similar to the
-/// [`quote_spanned!`](https://docs.rs/quote/latest/quote/macro.quote_spanned.html) macro from the
-/// `quote` crate but provides only just enough functionality needed by the current `macros` crate.
-macro_rules! quote_spanned {
- ($span:expr => $($tt:tt)*) => {{
- let mut tokens = ::proc_macro::TokenStream::new();
- {
- #[allow(unused_variables)]
- let span = $span;
- quote_spanned!(@proc tokens span $($tt)*);
- }
- tokens
- }};
- (@proc $v:ident $span:ident) => {};
- (@proc $v:ident $span:ident #$id:ident $($tt:tt)*) => {
- $crate::quote::ToTokens::to_tokens(&$id, &mut $v);
- quote_spanned!(@proc $v $span $($tt)*);
- };
- (@proc $v:ident $span:ident #(#$id:ident)* $($tt:tt)*) => {
- for token in $id {
- $crate::quote::ToTokens::to_tokens(&token, &mut $v);
- }
- quote_spanned!(@proc $v $span $($tt)*);
- };
- (@proc $v:ident $span:ident ( $($inner:tt)* ) $($tt:tt)*) => {
- #[allow(unused_mut)]
- let mut tokens = ::proc_macro::TokenStream::new();
- quote_spanned!(@proc tokens $span $($inner)*);
- $v.extend([::proc_macro::TokenTree::Group(::proc_macro::Group::new(
- ::proc_macro::Delimiter::Parenthesis,
- tokens,
- ))]);
- quote_spanned!(@proc $v $span $($tt)*);
- };
- (@proc $v:ident $span:ident [ $($inner:tt)* ] $($tt:tt)*) => {
- let mut tokens = ::proc_macro::TokenStream::new();
- quote_spanned!(@proc tokens $span $($inner)*);
- $v.extend([::proc_macro::TokenTree::Group(::proc_macro::Group::new(
- ::proc_macro::Delimiter::Bracket,
- tokens,
- ))]);
- quote_spanned!(@proc $v $span $($tt)*);
- };
- (@proc $v:ident $span:ident { $($inner:tt)* } $($tt:tt)*) => {
- let mut tokens = ::proc_macro::TokenStream::new();
- quote_spanned!(@proc tokens $span $($inner)*);
- $v.extend([::proc_macro::TokenTree::Group(::proc_macro::Group::new(
- ::proc_macro::Delimiter::Brace,
- tokens,
- ))]);
- quote_spanned!(@proc $v $span $($tt)*);
- };
- (@proc $v:ident $span:ident :: $($tt:tt)*) => {
- $v.extend([::proc_macro::Spacing::Joint, ::proc_macro::Spacing::Alone].map(|spacing| {
- ::proc_macro::TokenTree::Punct(::proc_macro::Punct::new(':', spacing))
- }));
- quote_spanned!(@proc $v $span $($tt)*);
- };
- (@proc $v:ident $span:ident : $($tt:tt)*) => {
- $v.extend([::proc_macro::TokenTree::Punct(
- ::proc_macro::Punct::new(':', ::proc_macro::Spacing::Alone),
- )]);
- quote_spanned!(@proc $v $span $($tt)*);
- };
- (@proc $v:ident $span:ident , $($tt:tt)*) => {
- $v.extend([::proc_macro::TokenTree::Punct(
- ::proc_macro::Punct::new(',', ::proc_macro::Spacing::Alone),
- )]);
- quote_spanned!(@proc $v $span $($tt)*);
- };
- (@proc $v:ident $span:ident @ $($tt:tt)*) => {
- $v.extend([::proc_macro::TokenTree::Punct(
- ::proc_macro::Punct::new('@', ::proc_macro::Spacing::Alone),
- )]);
- quote_spanned!(@proc $v $span $($tt)*);
- };
- (@proc $v:ident $span:ident ! $($tt:tt)*) => {
- $v.extend([::proc_macro::TokenTree::Punct(
- ::proc_macro::Punct::new('!', ::proc_macro::Spacing::Alone),
- )]);
- quote_spanned!(@proc $v $span $($tt)*);
- };
- (@proc $v:ident $span:ident ; $($tt:tt)*) => {
- $v.extend([::proc_macro::TokenTree::Punct(
- ::proc_macro::Punct::new(';', ::proc_macro::Spacing::Alone),
- )]);
- quote_spanned!(@proc $v $span $($tt)*);
- };
- (@proc $v:ident $span:ident + $($tt:tt)*) => {
- $v.extend([::proc_macro::TokenTree::Punct(
- ::proc_macro::Punct::new('+', ::proc_macro::Spacing::Alone),
- )]);
- quote_spanned!(@proc $v $span $($tt)*);
- };
- (@proc $v:ident $span:ident = $($tt:tt)*) => {
- $v.extend([::proc_macro::TokenTree::Punct(
- ::proc_macro::Punct::new('=', ::proc_macro::Spacing::Alone),
- )]);
- quote_spanned!(@proc $v $span $($tt)*);
- };
- (@proc $v:ident $span:ident # $($tt:tt)*) => {
- $v.extend([::proc_macro::TokenTree::Punct(
- ::proc_macro::Punct::new('#', ::proc_macro::Spacing::Alone),
- )]);
- quote_spanned!(@proc $v $span $($tt)*);
- };
- (@proc $v:ident $span:ident & $($tt:tt)*) => {
- $v.extend([::proc_macro::TokenTree::Punct(
- ::proc_macro::Punct::new('&', ::proc_macro::Spacing::Alone),
- )]);
- quote_spanned!(@proc $v $span $($tt)*);
- };
- (@proc $v:ident $span:ident _ $($tt:tt)*) => {
- $v.extend([::proc_macro::TokenTree::Ident(
- ::proc_macro::Ident::new("_", $span),
- )]);
- quote_spanned!(@proc $v $span $($tt)*);
- };
- (@proc $v:ident $span:ident $id:ident $($tt:tt)*) => {
- $v.extend([::proc_macro::TokenTree::Ident(
- ::proc_macro::Ident::new(stringify!($id), $span),
- )]);
- quote_spanned!(@proc $v $span $($tt)*);
- };
-}
-
-/// Converts tokens into [`proc_macro::TokenStream`] and performs variable interpolations with
-/// mixed site span ([`Span::mixed_site()`]).
-///
-/// This is a similar to the [`quote!`](https://docs.rs/quote/latest/quote/macro.quote.html) macro
-/// from the `quote` crate but provides only just enough functionality needed by the current
-/// `macros` crate.
-///
-/// [`Span::mixed_site()`]: https://doc.rust-lang.org/proc_macro/struct.Span.html#method.mixed_site
-macro_rules! quote {
- ($($tt:tt)*) => {
- quote_spanned!(::proc_macro::Span::mixed_site() => $($tt)*)
- }
-}
diff --git a/rust/macros/vtable.rs b/rust/macros/vtable.rs
index ee06044fcd4f3..a67d1cc81a2d3 100644
--- a/rust/macros/vtable.rs
+++ b/rust/macros/vtable.rs
@@ -1,9 +1,10 @@
// SPDX-License-Identifier: GPL-2.0
-use proc_macro::{Delimiter, Group, TokenStream, TokenTree};
use std::collections::HashSet;
use std::fmt::Write;
+use proc_macro2::{Delimiter, Group, TokenStream, TokenTree};
+
pub(crate) fn vtable(_attr: TokenStream, ts: TokenStream) -> TokenStream {
let mut tokens: Vec<_> = ts.into_iter().collect();
@@ -31,7 +32,7 @@ pub(crate) fn vtable(_attr: TokenStream, ts: TokenStream) -> TokenStream {
let mut consts = HashSet::new();
while let Some(token) = body_it.next() {
match token {
- TokenTree::Ident(ident) if ident.to_string() == "fn" => {
+ TokenTree::Ident(ident) if ident == "fn" => {
let fn_name = match body_it.next() {
Some(TokenTree::Ident(ident)) => ident.to_string(),
// Possibly we've encountered a fn pointer type instead.
@@ -39,7 +40,7 @@ pub(crate) fn vtable(_attr: TokenStream, ts: TokenStream) -> TokenStream {
};
functions.push(fn_name);
}
- TokenTree::Ident(ident) if ident.to_string() == "const" => {
+ TokenTree::Ident(ident) if ident == "const" => {
let const_name = match body_it.next() {
Some(TokenTree::Ident(ident)) => ident.to_string(),
// Possibly we've encountered an inline const block instead.
--
2.51.2
^ permalink raw reply related
* [PATCH v2 04/11] rust: macros: use `syn` to parse `module!` macro
From: Gary Guo @ 2026-01-07 16:15 UTC (permalink / raw)
To: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Luis Chamberlain, Petr Pavlu, Daniel Gomez,
Sami Tolvanen, Aaron Tomlin, Tamir Duberstein,
José Expósito
Cc: rust-for-linux, Patrick Miller, linux-kernel, linux-modules
In-Reply-To: <20260107161729.3855851-1-gary@kernel.org>
From: Gary Guo <gary@garyguo.net>
With `syn` being available in the kernel, use it to parse the complex
custom `module!` macro to replace existing helpers. Only parsing is
changed in this commit, the code generation is untouched.
This has the benefit of better error message when the macro is used
incorrectly, as it can point to a concrete span on what's going wrong.
For example, if a field is specified twice, previously it reads:
error: proc macro panicked
--> samples/rust/rust_minimal.rs:7:1
|
7 | / module! {
8 | | type: RustMinimal,
9 | | name: "rust_minimal",
10 | | author: "Rust for Linux Contributors",
11 | | description: "Rust minimal sample",
12 | | license: "GPL",
13 | | license: "GPL",
14 | | }
| |_^
|
= help: message: Duplicated key "license". Keys can only be specified once.
now it reads:
error: duplicated key "license". Keys can only be specified once.
--> samples/rust/rust_minimal.rs:13:5
|
13 | license: "GPL",
| ^^^^^^^
Signed-off-by: Gary Guo <gary@garyguo.net>
---
rust/macros/helpers.rs | 109 ++++--------
rust/macros/lib.rs | 6 +-
rust/macros/module.rs | 389 +++++++++++++++++++++++++----------------
3 files changed, 277 insertions(+), 227 deletions(-)
diff --git a/rust/macros/helpers.rs b/rust/macros/helpers.rs
index 13fafaba12261..fa66ef6eb0f3d 100644
--- a/rust/macros/helpers.rs
+++ b/rust/macros/helpers.rs
@@ -1,53 +1,21 @@
// SPDX-License-Identifier: GPL-2.0
-use proc_macro2::{token_stream, Group, Ident, TokenStream, TokenTree};
-
-pub(crate) fn try_ident(it: &mut token_stream::IntoIter) -> Option<String> {
- if let Some(TokenTree::Ident(ident)) = it.next() {
- Some(ident.to_string())
- } else {
- None
- }
-}
-
-pub(crate) fn try_sign(it: &mut token_stream::IntoIter) -> Option<char> {
- let peek = it.clone().next();
- match peek {
- Some(TokenTree::Punct(punct)) if punct.as_char() == '-' => {
- let _ = it.next();
- Some(punct.as_char())
- }
- _ => None,
- }
-}
-
-pub(crate) fn try_literal(it: &mut token_stream::IntoIter) -> Option<String> {
- if let Some(TokenTree::Literal(literal)) = it.next() {
- Some(literal.to_string())
- } else {
- None
- }
-}
-
-pub(crate) fn try_string(it: &mut token_stream::IntoIter) -> Option<String> {
- try_literal(it).and_then(|string| {
- if string.starts_with('\"') && string.ends_with('\"') {
- let content = &string[1..string.len() - 1];
- if content.contains('\\') {
- panic!("Escape sequences in string literals not yet handled");
- }
- Some(content.to_string())
- } else if string.starts_with("r\"") {
- panic!("Raw string literals are not yet handled");
- } else {
- None
- }
- })
-}
-
-pub(crate) fn expect_ident(it: &mut token_stream::IntoIter) -> String {
- try_ident(it).expect("Expected Ident")
-}
+use proc_macro2::{
+ token_stream,
+ Ident,
+ TokenStream,
+ TokenTree, //
+};
+use quote::ToTokens;
+use syn::{
+ parse::{
+ Parse,
+ ParseStream, //
+ },
+ Error,
+ LitStr,
+ Result, //
+};
pub(crate) fn expect_punct(it: &mut token_stream::IntoIter) -> char {
if let TokenTree::Punct(punct) = it.next().expect("Reached end of token stream for Punct") {
@@ -57,27 +25,28 @@ pub(crate) fn expect_punct(it: &mut token_stream::IntoIter) -> char {
}
}
-pub(crate) fn expect_string(it: &mut token_stream::IntoIter) -> String {
- try_string(it).expect("Expected string")
-}
+/// A string literal that is required to have ASCII value only.
+pub(crate) struct AsciiLitStr(LitStr);
-pub(crate) fn expect_string_ascii(it: &mut token_stream::IntoIter) -> String {
- let string = try_string(it).expect("Expected string");
- assert!(string.is_ascii(), "Expected ASCII string");
- string
+impl Parse for AsciiLitStr {
+ fn parse(input: ParseStream<'_>) -> Result<Self> {
+ let s: LitStr = input.parse()?;
+ if !s.value().is_ascii() {
+ return Err(Error::new_spanned(s, "expected ASCII-only string literal"));
+ }
+ Ok(Self(s))
+ }
}
-pub(crate) fn expect_group(it: &mut token_stream::IntoIter) -> Group {
- if let TokenTree::Group(group) = it.next().expect("Reached end of token stream for Group") {
- group
- } else {
- panic!("Expected Group");
+impl ToTokens for AsciiLitStr {
+ fn to_tokens(&self, ts: &mut TokenStream) {
+ self.0.to_tokens(ts);
}
}
-pub(crate) fn expect_end(it: &mut token_stream::IntoIter) {
- if it.next().is_some() {
- panic!("Expected end");
+impl AsciiLitStr {
+ pub(crate) fn value(&self) -> String {
+ self.0.value()
}
}
@@ -114,17 +83,3 @@ pub(crate) fn file() -> String {
proc_macro::Span::call_site().file()
}
}
-
-/// Parse a token stream of the form `expected_name: "value",` and return the
-/// string in the position of "value".
-///
-/// # Panics
-///
-/// - On parse error.
-pub(crate) fn expect_string_field(it: &mut token_stream::IntoIter, expected_name: &str) -> String {
- assert_eq!(expect_ident(it), expected_name);
- assert_eq!(expect_punct(it), ':');
- let string = expect_string(it);
- assert_eq!(expect_punct(it), ',');
- string
-}
diff --git a/rust/macros/lib.rs b/rust/macros/lib.rs
index 9955c04dbaae3..c5347127a3a51 100644
--- a/rust/macros/lib.rs
+++ b/rust/macros/lib.rs
@@ -131,8 +131,10 @@
/// - `firmware`: array of ASCII string literals of the firmware files of
/// the kernel module.
#[proc_macro]
-pub fn module(ts: TokenStream) -> TokenStream {
- module::module(ts.into()).into()
+pub fn module(input: TokenStream) -> TokenStream {
+ module::module(parse_macro_input!(input))
+ .unwrap_or_else(|e| e.into_compile_error())
+ .into()
}
/// Declares or implements a vtable trait.
diff --git a/rust/macros/module.rs b/rust/macros/module.rs
index b855a2b586e18..6ad7b411ccde4 100644
--- a/rust/macros/module.rs
+++ b/rust/macros/module.rs
@@ -2,28 +2,30 @@
use std::fmt::Write;
-use proc_macro2::{token_stream, Delimiter, Literal, TokenStream, TokenTree};
+use proc_macro2::{
+ Literal,
+ TokenStream, //
+};
+use quote::ToTokens;
+use syn::{
+ braced,
+ bracketed,
+ ext::IdentExt,
+ parse::{
+ Parse,
+ ParseStream, //
+ },
+ punctuated::Punctuated,
+ Error,
+ Expr,
+ Ident,
+ LitStr,
+ Result,
+ Token, //
+};
use crate::helpers::*;
-fn expect_string_array(it: &mut token_stream::IntoIter) -> Vec<String> {
- let group = expect_group(it);
- assert_eq!(group.delimiter(), Delimiter::Bracket);
- let mut values = Vec::new();
- let mut it = group.stream().into_iter();
-
- while let Some(val) = try_string(&mut it) {
- assert!(val.is_ascii(), "Expected ASCII string");
- values.push(val);
- match it.next() {
- Some(TokenTree::Punct(punct)) => assert_eq!(punct.as_char(), ','),
- None => break,
- _ => panic!("Expected ',' or end of array"),
- }
- }
- values
-}
-
struct ModInfoBuilder<'a> {
module: &'a str,
counter: usize,
@@ -113,12 +115,16 @@ fn emit_params(&mut self, info: &ModuleInfo) {
};
for param in params {
- let ops = param_ops_path(¶m.ptype);
+ let param_name = param.name.to_string();
+ let param_type = param.ptype.to_string();
+ let param_default = param.default.to_token_stream().to_string();
+
+ let ops = param_ops_path(¶m_type);
// Note: The spelling of these fields is dictated by the user space
// tool `modinfo`.
- self.emit_param("parmtype", ¶m.name, ¶m.ptype);
- self.emit_param("parm", ¶m.name, ¶m.description);
+ self.emit_param("parmtype", ¶m_name, ¶m_type);
+ self.emit_param("parm", ¶m_name, ¶m.description.value());
write!(
self.param_buffer,
@@ -160,10 +166,7 @@ fn emit_params(&mut self, info: &ModuleInfo) {
);
}};
",
- module_name = info.name,
- param_type = param.ptype,
- param_default = param.default,
- param_name = param.name,
+ module_name = info.name.value(),
ops = ops,
)
.unwrap();
@@ -187,127 +190,82 @@ fn param_ops_path(param_type: &str) -> &'static str {
}
}
-fn expect_param_default(param_it: &mut token_stream::IntoIter) -> String {
- assert_eq!(expect_ident(param_it), "default");
- assert_eq!(expect_punct(param_it), ':');
- let sign = try_sign(param_it);
- let default = try_literal(param_it).expect("Expected default param value");
- assert_eq!(expect_punct(param_it), ',');
- let mut value = sign.map(String::from).unwrap_or_default();
- value.push_str(&default);
- value
-}
-
-#[derive(Debug, Default)]
-struct ModuleInfo {
- type_: String,
- license: String,
- name: String,
- authors: Option<Vec<String>>,
- description: Option<String>,
- alias: Option<Vec<String>>,
- firmware: Option<Vec<String>>,
- imports_ns: Option<Vec<String>>,
- params: Option<Vec<Parameter>>,
-}
-
-#[derive(Debug)]
-struct Parameter {
- name: String,
- ptype: String,
- default: String,
- description: String,
-}
-
-fn expect_params(it: &mut token_stream::IntoIter) -> Vec<Parameter> {
- let params = expect_group(it);
- assert_eq!(params.delimiter(), Delimiter::Brace);
- let mut it = params.stream().into_iter();
- let mut parsed = Vec::new();
-
- loop {
- let param_name = match it.next() {
- Some(TokenTree::Ident(ident)) => ident.to_string(),
- Some(_) => panic!("Expected Ident or end"),
- None => break,
- };
-
- assert_eq!(expect_punct(&mut it), ':');
- let param_type = expect_ident(&mut it);
- let group = expect_group(&mut it);
- assert_eq!(group.delimiter(), Delimiter::Brace);
- assert_eq!(expect_punct(&mut it), ',');
-
- let mut param_it = group.stream().into_iter();
- let param_default = expect_param_default(&mut param_it);
- let param_description = expect_string_field(&mut param_it, "description");
- expect_end(&mut param_it);
-
- parsed.push(Parameter {
- name: param_name,
- ptype: param_type,
- default: param_default,
- description: param_description,
- })
- }
-
- parsed
-}
-
-impl ModuleInfo {
- fn parse(it: &mut token_stream::IntoIter) -> Self {
- let mut info = ModuleInfo::default();
-
- const EXPECTED_KEYS: &[&str] = &[
- "type",
- "name",
- "authors",
- "description",
- "license",
- "alias",
- "firmware",
- "imports_ns",
- "params",
- ];
- const REQUIRED_KEYS: &[&str] = &["type", "name", "license"];
+/// Parse fields that are required to use a specific order.
+///
+/// As fields must follow a specific order, we *could* just parse fields one by one by peeking.
+/// However the error message generated when implementing that way is not very friendly.
+///
+/// So instead we parse fields in an arbitrary order, but only enforce the ordering after parsing,
+/// and if the wrong order is used, the proper order is communicated to the user with error message.
+///
+/// Usage looks like this:
+/// ```ignore
+/// parse_ordered_fields! {
+/// from input;
+///
+/// // This will extract "foo: <field>" into a variable named "foo".
+/// // The variable will have type `Option<_>`.
+/// foo => <expression that parses the field>,
+///
+/// // If you need the variable name to be different than the key name.
+/// // This extracts "baz: <field>" into a variable named "bar".
+/// // You might want this if "baz" is a keyword.
+/// baz as bar => <expression that parse the field>,
+///
+/// // You can mark a key as required, and the variable will no longer be `Option`.
+/// // foobar will be of type `Expr` instead of `Option<Expr>`.
+/// foobar [required] => input.parse::<Expr>()?,
+/// }
+/// ```
+macro_rules! parse_ordered_fields {
+ (@gen
+ [$input:expr]
+ [$([$name:ident; $key:ident; $parser:expr])*]
+ [$([$req_name:ident; $req_key:ident])*]
+ ) => {
+ $(let mut $name = None;)*
+
+ const EXPECTED_KEYS: &[&str] = &[$(stringify!($key),)*];
+ const REQUIRED_KEYS: &[&str] = &[$(stringify!($req_key),)*];
+
+ let span = $input.span();
let mut seen_keys = Vec::new();
loop {
- let key = match it.next() {
- Some(TokenTree::Ident(ident)) => ident.to_string(),
- Some(_) => panic!("Expected Ident or end"),
- None => break,
- };
+ if $input.is_empty() {
+ break;
+ }
+
+ let key = $input.call(Ident::parse_any)?;
if seen_keys.contains(&key) {
- panic!("Duplicated key \"{key}\". Keys can only be specified once.");
+ Err(Error::new_spanned(
+ &key,
+ format!(r#"duplicated key "{key}". Keys can only be specified once."#),
+ ))?
}
- assert_eq!(expect_punct(it), ':');
-
- match key.as_str() {
- "type" => info.type_ = expect_ident(it),
- "name" => info.name = expect_string_ascii(it),
- "authors" => info.authors = Some(expect_string_array(it)),
- "description" => info.description = Some(expect_string(it)),
- "license" => info.license = expect_string_ascii(it),
- "alias" => info.alias = Some(expect_string_array(it)),
- "firmware" => info.firmware = Some(expect_string_array(it)),
- "imports_ns" => info.imports_ns = Some(expect_string_array(it)),
- "params" => info.params = Some(expect_params(it)),
- _ => panic!("Unknown key \"{key}\". Valid keys are: {EXPECTED_KEYS:?}."),
+ $input.parse::<Token![:]>()?;
+
+ match &*key.to_string() {
+ $(
+ stringify!($key) => $name = Some($parser),
+ )*
+ _ => {
+ Err(Error::new_spanned(
+ &key,
+ format!(r#"unknown key "{key}". Valid keys are: {EXPECTED_KEYS:?}."#),
+ ))?
+ }
}
- assert_eq!(expect_punct(it), ',');
-
+ $input.parse::<Token![,]>()?;
seen_keys.push(key);
}
- expect_end(it);
-
for key in REQUIRED_KEYS {
if !seen_keys.iter().any(|e| e == key) {
- panic!("Missing required key \"{key}\".");
+ Err(Error::new(span, format!(r#"missing required key "{key}""#)))?
}
}
@@ -319,43 +277,178 @@ fn parse(it: &mut token_stream::IntoIter) -> Self {
}
if seen_keys != ordered_keys {
- panic!("Keys are not ordered as expected. Order them like: {ordered_keys:?}.");
+ Err(Error::new(
+ span,
+ format!(r#"keys are not ordered as expected. Order them like: {ordered_keys:?}."#),
+ ))?
+ }
+
+ $(let $req_name = $req_name.expect("required field");)*
+ };
+
+ // Handle required fields.
+ (@gen
+ [$input:expr] [$($tok:tt)*] [$($req:tt)*]
+ $key:ident as $name:ident [required] => $parser:expr,
+ $($rest:tt)*
+ ) => {
+ parse_ordered_fields!(
+ @gen [$input] [$($tok)* [$name; $key; $parser]] [$($req)* [$name; $key]] $($rest)*
+ )
+ };
+ (@gen
+ [$input:expr] [$($tok:tt)*] [$($req:tt)*]
+ $name:ident [required] => $parser:expr,
+ $($rest:tt)*
+ ) => {
+ parse_ordered_fields!(
+ @gen [$input] [$($tok)* [$name; $name; $parser]] [$($req)* [$name; $name]] $($rest)*
+ )
+ };
+
+ // Handle optional fields.
+ (@gen
+ [$input:expr] [$($tok:tt)*] [$($req:tt)*]
+ $key:ident as $name:ident => $parser:expr,
+ $($rest:tt)*
+ ) => {
+ parse_ordered_fields!(
+ @gen [$input] [$($tok)* [$name; $key; $parser]] [$($req)*] $($rest)*
+ )
+ };
+ (@gen
+ [$input:expr] [$($tok:tt)*] [$($req:tt)*]
+ $name:ident => $parser:expr,
+ $($rest:tt)*
+ ) => {
+ parse_ordered_fields!(
+ @gen [$input] [$($tok)* [$name; $name; $parser]] [$($req)*] $($rest)*
+ )
+ };
+
+ (from $input:expr; $($tok:tt)*) => {
+ parse_ordered_fields!(@gen [$input] [] [] $($tok)*)
+ }
+}
+
+struct Parameter {
+ name: Ident,
+ ptype: Ident,
+ default: Expr,
+ description: LitStr,
+}
+
+impl Parse for Parameter {
+ fn parse(input: ParseStream<'_>) -> Result<Self> {
+ let name = input.parse()?;
+ input.parse::<Token![:]>()?;
+ let ptype = input.parse()?;
+
+ let fields;
+ braced!(fields in input);
+
+ parse_ordered_fields! {
+ from fields;
+ default [required] => fields.parse()?,
+ description [required] => fields.parse()?,
}
- info
+ Ok(Self {
+ name,
+ ptype,
+ default,
+ description,
+ })
}
}
-pub(crate) fn module(ts: TokenStream) -> TokenStream {
- let mut it = ts.into_iter();
+pub(crate) struct ModuleInfo {
+ type_: Ident,
+ license: AsciiLitStr,
+ name: AsciiLitStr,
+ authors: Option<Punctuated<AsciiLitStr, Token![,]>>,
+ description: Option<LitStr>,
+ alias: Option<Punctuated<AsciiLitStr, Token![,]>>,
+ firmware: Option<Punctuated<AsciiLitStr, Token![,]>>,
+ imports_ns: Option<Punctuated<AsciiLitStr, Token![,]>>,
+ params: Option<Punctuated<Parameter, Token![,]>>,
+}
- let info = ModuleInfo::parse(&mut it);
+impl Parse for ModuleInfo {
+ fn parse(input: ParseStream<'_>) -> Result<Self> {
+ parse_ordered_fields!(
+ from input;
+ type as type_ [required] => input.parse()?,
+ name [required] => input.parse()?,
+ authors => {
+ let list;
+ bracketed!(list in input);
+ Punctuated::parse_terminated(&list)?
+ },
+ description => input.parse()?,
+ license [required] => input.parse()?,
+ alias => {
+ let list;
+ bracketed!(list in input);
+ Punctuated::parse_terminated(&list)?
+ },
+ firmware => {
+ let list;
+ bracketed!(list in input);
+ Punctuated::parse_terminated(&list)?
+ },
+ imports_ns => {
+ let list;
+ bracketed!(list in input);
+ Punctuated::parse_terminated(&list)?
+ },
+ params => {
+ let list;
+ braced!(list in input);
+ Punctuated::parse_terminated(&list)?
+ },
+ );
+
+ Ok(ModuleInfo {
+ type_,
+ license,
+ name,
+ authors,
+ description,
+ alias,
+ firmware,
+ imports_ns,
+ params,
+ })
+ }
+}
+pub(crate) fn module(info: ModuleInfo) -> Result<TokenStream> {
// Rust does not allow hyphens in identifiers, use underscore instead.
- let ident = info.name.replace('-', "_");
+ let ident = info.name.value().replace('-', "_");
let mut modinfo = ModInfoBuilder::new(ident.as_ref());
if let Some(authors) = &info.authors {
for author in authors {
- modinfo.emit("author", author);
+ modinfo.emit("author", &author.value());
}
}
if let Some(description) = &info.description {
- modinfo.emit("description", description);
+ modinfo.emit("description", &description.value());
}
- modinfo.emit("license", &info.license);
+ modinfo.emit("license", &info.license.value());
if let Some(aliases) = &info.alias {
for alias in aliases {
- modinfo.emit("alias", alias);
+ modinfo.emit("alias", &alias.value());
}
}
if let Some(firmware) = &info.firmware {
for fw in firmware {
- modinfo.emit("firmware", fw);
+ modinfo.emit("firmware", &fw.value());
}
}
if let Some(imports) = &info.imports_ns {
for ns in imports {
- modinfo.emit("import_ns", ns);
+ modinfo.emit("import_ns", &ns.value());
}
}
@@ -366,7 +459,7 @@ pub(crate) fn module(ts: TokenStream) -> TokenStream {
modinfo.emit_params(&info);
- format!(
+ Ok(format!(
"
/// The module name.
///
@@ -536,12 +629,12 @@ mod module_parameters {{
}}
",
type_ = info.type_,
- name = info.name,
+ name = info.name.value(),
ident = ident,
modinfo = modinfo.buffer,
params = modinfo.param_buffer,
initcall_section = ".initcall6.init"
)
.parse()
- .expect("Error parsing formatted string into token stream.")
+ .expect("Error parsing formatted string into token stream."))
}
--
2.51.2
^ permalink raw reply related
* [PATCH v2 05/11] rust: macros: use `quote!` for `module!` macro
From: Gary Guo @ 2026-01-07 16:15 UTC (permalink / raw)
To: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Luis Chamberlain, Petr Pavlu, Daniel Gomez,
Sami Tolvanen, Aaron Tomlin
Cc: rust-for-linux, linux-modules, linux-kernel
In-Reply-To: <20260107161729.3855851-1-gary@kernel.org>
From: Gary Guo <gary@garyguo.net>
This has no behavioural change, but is good for maintainability. With
`quote!`, we're no longer using string templates, so we don't need to
quote " and {} inside the template anymore. Further more, editors can
now highlight the code template.
This also improves the robustness as it eliminates the need for string
quoting and escaping.
Co-developed-by: Benno Lossin <lossin@kernel.org>
Signed-off-by: Benno Lossin <lossin@kernel.org>
Signed-off-by: Gary Guo <gary@garyguo.net>
---
rust/macros/module.rs | 558 ++++++++++++++++++++++--------------------
1 file changed, 295 insertions(+), 263 deletions(-)
diff --git a/rust/macros/module.rs b/rust/macros/module.rs
index 6ad7b411ccde4..ba345d672839e 100644
--- a/rust/macros/module.rs
+++ b/rust/macros/module.rs
@@ -1,12 +1,15 @@
// SPDX-License-Identifier: GPL-2.0
-use std::fmt::Write;
+use std::ffi::CString;
use proc_macro2::{
Literal,
TokenStream, //
};
-use quote::ToTokens;
+use quote::{
+ format_ident,
+ quote, //
+};
use syn::{
braced,
bracketed,
@@ -15,11 +18,13 @@
Parse,
ParseStream, //
},
+ parse_quote,
punctuated::Punctuated,
Error,
Expr,
Ident,
LitStr,
+ Path,
Result,
Token, //
};
@@ -29,8 +34,8 @@
struct ModInfoBuilder<'a> {
module: &'a str,
counter: usize,
- buffer: String,
- param_buffer: String,
+ ts: TokenStream,
+ param_ts: TokenStream,
}
impl<'a> ModInfoBuilder<'a> {
@@ -38,8 +43,8 @@ fn new(module: &'a str) -> Self {
ModInfoBuilder {
module,
counter: 0,
- buffer: String::new(),
- param_buffer: String::new(),
+ ts: TokenStream::new(),
+ param_ts: TokenStream::new(),
}
}
@@ -56,33 +61,32 @@ fn emit_base(&mut self, field: &str, content: &str, builtin: bool, param: bool)
// Loadable modules' modinfo strings go as-is.
format!("{field}={content}\0")
};
-
- let buffer = if param {
- &mut self.param_buffer
+ let length = string.len();
+ let string = Literal::byte_string(string.as_bytes());
+ let cfg = if builtin {
+ quote!(#[cfg(not(MODULE))])
} else {
- &mut self.buffer
+ quote!(#[cfg(MODULE)])
};
- write!(
- buffer,
- "
- {cfg}
- #[doc(hidden)]
- #[cfg_attr(not(target_os = \"macos\"), link_section = \".modinfo\")]
- #[used(compiler)]
- pub static __{module}_{counter}: [u8; {length}] = *{string};
- ",
- cfg = if builtin {
- "#[cfg(not(MODULE))]"
- } else {
- "#[cfg(MODULE)]"
- },
+ let counter = format_ident!(
+ "__{module}_{counter}",
module = self.module.to_uppercase(),
- counter = self.counter,
- length = string.len(),
- string = Literal::byte_string(string.as_bytes()),
- )
- .unwrap();
+ counter = self.counter
+ );
+ let item = quote! {
+ #cfg
+ #[doc(hidden)]
+ #[cfg_attr(not(target_os = "macos"), link_section = ".modinfo")]
+ #[used(compiler)]
+ pub static #counter: [u8; #length] = *#string;
+ };
+
+ if param {
+ self.param_ts.extend(item);
+ } else {
+ self.ts.extend(item);
+ }
self.counter += 1;
}
@@ -115,77 +119,86 @@ fn emit_params(&mut self, info: &ModuleInfo) {
};
for param in params {
- let param_name = param.name.to_string();
- let param_type = param.ptype.to_string();
- let param_default = param.default.to_token_stream().to_string();
+ let param_name_str = param.name.to_string();
+ let param_type_str = param.ptype.to_string();
- let ops = param_ops_path(¶m_type);
+ let ops = param_ops_path(¶m_type_str);
// Note: The spelling of these fields is dictated by the user space
// tool `modinfo`.
- self.emit_param("parmtype", ¶m_name, ¶m_type);
- self.emit_param("parm", ¶m_name, ¶m.description.value());
-
- write!(
- self.param_buffer,
- "
- pub(crate) static {param_name}:
- ::kernel::module_param::ModuleParamAccess<{param_type}> =
- ::kernel::module_param::ModuleParamAccess::new({param_default});
-
- const _: () = {{
- #[link_section = \"__param\"]
- #[used]
- static __{module_name}_{param_name}_struct:
+ self.emit_param("parmtype", ¶m_name_str, ¶m_type_str);
+ self.emit_param("parm", ¶m_name_str, ¶m.description.value());
+
+ let static_name = format_ident!("__{}_{}_struct", self.module, param.name);
+ let param_name_cstr = Literal::c_string(
+ &CString::new(param_name_str).expect("name contains NUL-terminator"),
+ );
+ let param_name_cstr_with_module = Literal::c_string(
+ &CString::new(format!("{}.{}", self.module, param.name))
+ .expect("name contains NUL-terminator"),
+ );
+
+ let param_name = ¶m.name;
+ let param_type = ¶m.ptype;
+ let param_default = ¶m.default;
+
+ self.param_ts.extend(quote!{
+ #[allow(non_upper_case_globals)]
+ pub(crate) static #param_name:
+ ::kernel::module_param::ModuleParamAccess<#param_type> =
+ ::kernel::module_param::ModuleParamAccess::new(#param_default);
+
+ const _: () = {
+ #[allow(non_upper_case_globals)]
+ #[link_section = "__param"]
+ #[used(compiler)]
+ static #static_name:
::kernel::module_param::KernelParam =
::kernel::module_param::KernelParam::new(
- ::kernel::bindings::kernel_param {{
- name: if ::core::cfg!(MODULE) {{
- ::kernel::c_str!(\"{param_name}\").to_bytes_with_nul()
- }} else {{
- ::kernel::c_str!(\"{module_name}.{param_name}\")
- .to_bytes_with_nul()
- }}.as_ptr(),
+ ::kernel::bindings::kernel_param {
+ name: kernel::str::as_char_ptr_in_const_context(
+ if ::core::cfg!(MODULE) {
+ #param_name_cstr
+ } else {
+ #param_name_cstr_with_module
+ }
+ ),
// SAFETY: `__this_module` is constructed by the kernel at load
// time and will not be freed until the module is unloaded.
#[cfg(MODULE)]
- mod_: unsafe {{
+ mod_: unsafe {
core::ptr::from_ref(&::kernel::bindings::__this_module)
.cast_mut()
- }},
+ },
#[cfg(not(MODULE))]
mod_: ::core::ptr::null_mut(),
- ops: core::ptr::from_ref(&{ops}),
+ ops: core::ptr::from_ref(&#ops),
perm: 0, // Will not appear in sysfs
level: -1,
flags: 0,
- __bindgen_anon_1: ::kernel::bindings::kernel_param__bindgen_ty_1 {{
- arg: {param_name}.as_void_ptr()
- }},
- }}
+ __bindgen_anon_1: ::kernel::bindings::kernel_param__bindgen_ty_1 {
+ arg: #param_name.as_void_ptr()
+ },
+ }
);
- }};
- ",
- module_name = info.name.value(),
- ops = ops,
- )
- .unwrap();
+ };
+ });
}
}
}
-fn param_ops_path(param_type: &str) -> &'static str {
+fn param_ops_path(param_type: &str) -> Path {
match param_type {
- "i8" => "::kernel::module_param::PARAM_OPS_I8",
- "u8" => "::kernel::module_param::PARAM_OPS_U8",
- "i16" => "::kernel::module_param::PARAM_OPS_I16",
- "u16" => "::kernel::module_param::PARAM_OPS_U16",
- "i32" => "::kernel::module_param::PARAM_OPS_I32",
- "u32" => "::kernel::module_param::PARAM_OPS_U32",
- "i64" => "::kernel::module_param::PARAM_OPS_I64",
- "u64" => "::kernel::module_param::PARAM_OPS_U64",
- "isize" => "::kernel::module_param::PARAM_OPS_ISIZE",
- "usize" => "::kernel::module_param::PARAM_OPS_USIZE",
+ "i8" => parse_quote!(::kernel::module_param::PARAM_OPS_I8),
+ "u8" => parse_quote!(::kernel::module_param::PARAM_OPS_U8),
+ "i16" => parse_quote!(::kernel::module_param::PARAM_OPS_I16),
+ "u16" => parse_quote!(::kernel::module_param::PARAM_OPS_U16),
+ "i32" => parse_quote!(::kernel::module_param::PARAM_OPS_I32),
+ "u32" => parse_quote!(::kernel::module_param::PARAM_OPS_U32),
+ "i64" => parse_quote!(::kernel::module_param::PARAM_OPS_I64),
+ "u64" => parse_quote!(::kernel::module_param::PARAM_OPS_U64),
+ "isize" => parse_quote!(::kernel::module_param::PARAM_OPS_ISIZE),
+ "usize" => parse_quote!(::kernel::module_param::PARAM_OPS_USIZE),
t => panic!("Unsupported parameter type {}", t),
}
}
@@ -424,29 +437,41 @@ fn parse(input: ParseStream<'_>) -> Result<Self> {
}
pub(crate) fn module(info: ModuleInfo) -> Result<TokenStream> {
+ let ModuleInfo {
+ type_,
+ license,
+ name,
+ authors,
+ description,
+ alias,
+ firmware,
+ imports_ns,
+ params: _,
+ } = &info;
+
// Rust does not allow hyphens in identifiers, use underscore instead.
- let ident = info.name.value().replace('-', "_");
+ let ident = name.value().replace('-', "_");
let mut modinfo = ModInfoBuilder::new(ident.as_ref());
- if let Some(authors) = &info.authors {
+ if let Some(authors) = authors {
for author in authors {
modinfo.emit("author", &author.value());
}
}
- if let Some(description) = &info.description {
+ if let Some(description) = description {
modinfo.emit("description", &description.value());
}
- modinfo.emit("license", &info.license.value());
- if let Some(aliases) = &info.alias {
+ modinfo.emit("license", &license.value());
+ if let Some(aliases) = alias {
for alias in aliases {
modinfo.emit("alias", &alias.value());
}
}
- if let Some(firmware) = &info.firmware {
+ if let Some(firmware) = firmware {
for fw in firmware {
modinfo.emit("firmware", &fw.value());
}
}
- if let Some(imports) = &info.imports_ns {
+ if let Some(imports) = imports_ns {
for ns in imports {
modinfo.emit("import_ns", &ns.value());
}
@@ -459,182 +484,189 @@ pub(crate) fn module(info: ModuleInfo) -> Result<TokenStream> {
modinfo.emit_params(&info);
- Ok(format!(
- "
- /// The module name.
- ///
- /// Used by the printing macros, e.g. [`info!`].
- const __LOG_PREFIX: &[u8] = b\"{name}\\0\";
-
- // SAFETY: `__this_module` is constructed by the kernel at load time and will not be
- // freed until the module is unloaded.
- #[cfg(MODULE)]
- static THIS_MODULE: ::kernel::ThisModule = unsafe {{
- extern \"C\" {{
- static __this_module: ::kernel::types::Opaque<::kernel::bindings::module>;
- }}
-
- ::kernel::ThisModule::from_ptr(__this_module.get())
- }};
- #[cfg(not(MODULE))]
- static THIS_MODULE: ::kernel::ThisModule = unsafe {{
- ::kernel::ThisModule::from_ptr(::core::ptr::null_mut())
- }};
-
- /// The `LocalModule` type is the type of the module created by `module!`,
- /// `module_pci_driver!`, `module_platform_driver!`, etc.
- type LocalModule = {type_};
-
- impl ::kernel::ModuleMetadata for {type_} {{
- const NAME: &'static ::kernel::str::CStr = c\"{name}\";
- }}
-
- // Double nested modules, since then nobody can access the public items inside.
- mod __module_init {{
- mod __module_init {{
- use super::super::{type_};
- use pin_init::PinInit;
-
- /// The \"Rust loadable module\" mark.
- //
- // This may be best done another way later on, e.g. as a new modinfo
- // key or a new section. For the moment, keep it simple.
- #[cfg(MODULE)]
- #[doc(hidden)]
- #[used(compiler)]
- static __IS_RUST_MODULE: () = ();
-
- static mut __MOD: ::core::mem::MaybeUninit<{type_}> =
- ::core::mem::MaybeUninit::uninit();
-
- // Loadable modules need to export the `{{init,cleanup}}_module` identifiers.
- /// # Safety
- ///
- /// This function must not be called after module initialization, because it may be
- /// freed after that completes.
- #[cfg(MODULE)]
- #[doc(hidden)]
- #[no_mangle]
- #[link_section = \".init.text\"]
- pub unsafe extern \"C\" fn init_module() -> ::kernel::ffi::c_int {{
- // SAFETY: This function is inaccessible to the outside due to the double
- // module wrapping it. It is called exactly once by the C side via its
- // unique name.
- unsafe {{ __init() }}
- }}
-
- #[cfg(MODULE)]
- #[doc(hidden)]
- #[used(compiler)]
- #[link_section = \".init.data\"]
- static __UNIQUE_ID___addressable_init_module: unsafe extern \"C\" fn() -> i32 = init_module;
-
- #[cfg(MODULE)]
- #[doc(hidden)]
- #[no_mangle]
- #[link_section = \".exit.text\"]
- pub extern \"C\" fn cleanup_module() {{
- // SAFETY:
- // - This function is inaccessible to the outside due to the double
- // module wrapping it. It is called exactly once by the C side via its
- // unique name,
- // - furthermore it is only called after `init_module` has returned `0`
- // (which delegates to `__init`).
- unsafe {{ __exit() }}
- }}
-
- #[cfg(MODULE)]
- #[doc(hidden)]
- #[used(compiler)]
- #[link_section = \".exit.data\"]
- static __UNIQUE_ID___addressable_cleanup_module: extern \"C\" fn() = cleanup_module;
-
- // Built-in modules are initialized through an initcall pointer
- // and the identifiers need to be unique.
- #[cfg(not(MODULE))]
- #[cfg(not(CONFIG_HAVE_ARCH_PREL32_RELOCATIONS))]
- #[doc(hidden)]
- #[link_section = \"{initcall_section}\"]
- #[used(compiler)]
- pub static __{ident}_initcall: extern \"C\" fn() ->
- ::kernel::ffi::c_int = __{ident}_init;
-
- #[cfg(not(MODULE))]
- #[cfg(CONFIG_HAVE_ARCH_PREL32_RELOCATIONS)]
- ::core::arch::global_asm!(
- r#\".section \"{initcall_section}\", \"a\"
- __{ident}_initcall:
- .long __{ident}_init - .
- .previous
- \"#
- );
-
- #[cfg(not(MODULE))]
- #[doc(hidden)]
- #[no_mangle]
- pub extern \"C\" fn __{ident}_init() -> ::kernel::ffi::c_int {{
- // SAFETY: This function is inaccessible to the outside due to the double
- // module wrapping it. It is called exactly once by the C side via its
- // placement above in the initcall section.
- unsafe {{ __init() }}
- }}
-
- #[cfg(not(MODULE))]
- #[doc(hidden)]
- #[no_mangle]
- pub extern \"C\" fn __{ident}_exit() {{
- // SAFETY:
- // - This function is inaccessible to the outside due to the double
- // module wrapping it. It is called exactly once by the C side via its
- // unique name,
- // - furthermore it is only called after `__{ident}_init` has
- // returned `0` (which delegates to `__init`).
- unsafe {{ __exit() }}
- }}
-
- /// # Safety
- ///
- /// This function must only be called once.
- unsafe fn __init() -> ::kernel::ffi::c_int {{
- let initer =
- <{type_} as ::kernel::InPlaceModule>::init(&super::super::THIS_MODULE);
- // SAFETY: No data race, since `__MOD` can only be accessed by this module
- // and there only `__init` and `__exit` access it. These functions are only
- // called once and `__exit` cannot be called before or during `__init`.
- match unsafe {{ initer.__pinned_init(__MOD.as_mut_ptr()) }} {{
- Ok(m) => 0,
- Err(e) => e.to_errno(),
- }}
- }}
-
- /// # Safety
- ///
- /// This function must
- /// - only be called once,
- /// - be called after `__init` has been called and returned `0`.
- unsafe fn __exit() {{
- // SAFETY: No data race, since `__MOD` can only be accessed by this module
- // and there only `__init` and `__exit` access it. These functions are only
- // called once and `__init` was already called.
- unsafe {{
- // Invokes `drop()` on `__MOD`, which should be used for cleanup.
- __MOD.assume_init_drop();
- }}
- }}
- {modinfo}
- }}
- }}
- mod module_parameters {{
- {params}
- }}
- ",
- type_ = info.type_,
- name = info.name.value(),
- ident = ident,
- modinfo = modinfo.buffer,
- params = modinfo.param_buffer,
- initcall_section = ".initcall6.init"
- )
- .parse()
- .expect("Error parsing formatted string into token stream."))
+ let modinfo_ts = modinfo.ts;
+ let params_ts = modinfo.param_ts;
+
+ let ident_init = format_ident!("__{ident}_init");
+ let ident_exit = format_ident!("__{ident}_exit");
+ let ident_initcall = format_ident!("__{ident}_initcall");
+ let initcall_section = ".initcall6.init";
+
+ let global_asm = format!(
+ r#".section "{initcall_section}", "a"
+ __{ident}_initcall:
+ .long __{ident}_init - .
+ .previous
+ "#
+ );
+
+ let name_cstr =
+ Literal::c_string(&CString::new(name.value()).expect("name contains NUL-terminator"));
+
+ Ok(quote! {
+ /// The module name.
+ ///
+ /// Used by the printing macros, e.g. [`info!`].
+ const __LOG_PREFIX: &[u8] = #name_cstr.to_bytes_with_nul();
+
+ // SAFETY: `__this_module` is constructed by the kernel at load time and will not be
+ // freed until the module is unloaded.
+ #[cfg(MODULE)]
+ static THIS_MODULE: ::kernel::ThisModule = unsafe {
+ extern "C" {
+ static __this_module: ::kernel::types::Opaque<::kernel::bindings::module>;
+ };
+
+ ::kernel::ThisModule::from_ptr(__this_module.get())
+ };
+
+ #[cfg(not(MODULE))]
+ static THIS_MODULE: ::kernel::ThisModule = unsafe {
+ ::kernel::ThisModule::from_ptr(::core::ptr::null_mut())
+ };
+
+ /// The `LocalModule` type is the type of the module created by `module!`,
+ /// `module_pci_driver!`, `module_platform_driver!`, etc.
+ type LocalModule = #type_;
+
+ impl ::kernel::ModuleMetadata for #type_ {
+ const NAME: &'static ::kernel::str::CStr = #name_cstr;
+ }
+
+ // Double nested modules, since then nobody can access the public items inside.
+ mod __module_init {
+ mod __module_init {
+ use super::super::#type_;
+ use pin_init::PinInit;
+
+ /// The "Rust loadable module" mark.
+ //
+ // This may be best done another way later on, e.g. as a new modinfo
+ // key or a new section. For the moment, keep it simple.
+ #[cfg(MODULE)]
+ #[doc(hidden)]
+ #[used(compiler)]
+ static __IS_RUST_MODULE: () = ();
+
+ static mut __MOD: ::core::mem::MaybeUninit<#type_> =
+ ::core::mem::MaybeUninit::uninit();
+
+ // Loadable modules need to export the `{init,cleanup}_module` identifiers.
+ /// # Safety
+ ///
+ /// This function must not be called after module initialization, because it may be
+ /// freed after that completes.
+ #[cfg(MODULE)]
+ #[doc(hidden)]
+ #[no_mangle]
+ #[link_section = ".init.text"]
+ pub unsafe extern "C" fn init_module() -> ::kernel::ffi::c_int {
+ // SAFETY: This function is inaccessible to the outside due to the double
+ // module wrapping it. It is called exactly once by the C side via its
+ // unique name.
+ unsafe { __init() }
+ }
+
+ #[cfg(MODULE)]
+ #[doc(hidden)]
+ #[used(compiler)]
+ #[link_section = ".init.data"]
+ static __UNIQUE_ID___addressable_init_module: unsafe extern "C" fn() -> i32 =
+ init_module;
+
+ #[cfg(MODULE)]
+ #[doc(hidden)]
+ #[no_mangle]
+ #[link_section = ".exit.text"]
+ pub extern "C" fn cleanup_module() {
+ // SAFETY:
+ // - This function is inaccessible to the outside due to the double
+ // module wrapping it. It is called exactly once by the C side via its
+ // unique name,
+ // - furthermore it is only called after `init_module` has returned `0`
+ // (which delegates to `__init`).
+ unsafe { __exit() }
+ }
+
+ #[cfg(MODULE)]
+ #[doc(hidden)]
+ #[used(compiler)]
+ #[link_section = ".exit.data"]
+ static __UNIQUE_ID___addressable_cleanup_module: extern "C" fn() = cleanup_module;
+
+ // Built-in modules are initialized through an initcall pointer
+ // and the identifiers need to be unique.
+ #[cfg(not(MODULE))]
+ #[cfg(not(CONFIG_HAVE_ARCH_PREL32_RELOCATIONS))]
+ #[doc(hidden)]
+ #[link_section = #initcall_section]
+ #[used(compiler)]
+ pub static #ident_initcall: extern "C" fn() ->
+ ::kernel::ffi::c_int = #ident_initcall;
+
+ #[cfg(not(MODULE))]
+ #[cfg(CONFIG_HAVE_ARCH_PREL32_RELOCATIONS)]
+ ::core::arch::global_asm!(#global_asm);
+
+ #[cfg(not(MODULE))]
+ #[doc(hidden)]
+ #[no_mangle]
+ pub extern "C" fn #ident_init() -> ::kernel::ffi::c_int {
+ // SAFETY: This function is inaccessible to the outside due to the double
+ // module wrapping it. It is called exactly once by the C side via its
+ // placement above in the initcall section.
+ unsafe { __init() }
+ }
+
+ #[cfg(not(MODULE))]
+ #[doc(hidden)]
+ #[no_mangle]
+ pub extern "C" fn #ident_exit() {
+ // SAFETY:
+ // - This function is inaccessible to the outside due to the double
+ // module wrapping it. It is called exactly once by the C side via its
+ // unique name,
+ // - furthermore it is only called after `#ident_init` has
+ // returned `0` (which delegates to `__init`).
+ unsafe { __exit() }
+ }
+
+ /// # Safety
+ ///
+ /// This function must only be called once.
+ unsafe fn __init() -> ::kernel::ffi::c_int {
+ let initer =
+ <#type_ as ::kernel::InPlaceModule>::init(&super::super::THIS_MODULE);
+ // SAFETY: No data race, since `__MOD` can only be accessed by this module
+ // and there only `__init` and `__exit` access it. These functions are only
+ // called once and `__exit` cannot be called before or during `__init`.
+ match unsafe { initer.__pinned_init(__MOD.as_mut_ptr()) } {
+ Ok(m) => 0,
+ Err(e) => e.to_errno(),
+ }
+ }
+
+ /// # Safety
+ ///
+ /// This function must
+ /// - only be called once,
+ /// - be called after `__init` has been called and returned `0`.
+ unsafe fn __exit() {
+ // SAFETY: No data race, since `__MOD` can only be accessed by this module
+ // and there only `__init` and `__exit` access it. These functions are only
+ // called once and `__init` was already called.
+ unsafe {
+ // Invokes `drop()` on `__MOD`, which should be used for cleanup.
+ __MOD.assume_init_drop();
+ }
+ }
+
+ #modinfo_ts
+ }
+ }
+
+ mod module_parameters {
+ #params_ts
+ }
+ })
}
--
2.51.2
^ permalink raw reply related
* [PATCH v2 09/11] rust: macros: allow arbitrary types to be used in `module!` macro
From: Gary Guo @ 2026-01-07 16:15 UTC (permalink / raw)
To: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Luis Chamberlain, Petr Pavlu, Daniel Gomez,
Sami Tolvanen, Aaron Tomlin
Cc: rust-for-linux, linux-modules, linux-kernel
In-Reply-To: <20260107161729.3855851-1-gary@kernel.org>
From: Gary Guo <gary@garyguo.net>
Previously this only accepts an identifier, but now with `syn` it is
easy to make it accepts any type.
Reviewed-by: Benno Lossin <lossin@kernel.org>
Signed-off-by: Gary Guo <gary@garyguo.net>
---
rust/macros/module.rs | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/rust/macros/module.rs b/rust/macros/module.rs
index ba345d672839e..31d39764c6926 100644
--- a/rust/macros/module.rs
+++ b/rust/macros/module.rs
@@ -26,7 +26,8 @@
LitStr,
Path,
Result,
- Token, //
+ Token,
+ Type, //
};
use crate::helpers::*;
@@ -376,7 +377,7 @@ fn parse(input: ParseStream<'_>) -> Result<Self> {
}
pub(crate) struct ModuleInfo {
- type_: Ident,
+ type_: Type,
license: AsciiLitStr,
name: AsciiLitStr,
authors: Option<Punctuated<AsciiLitStr, Token![,]>>,
@@ -536,7 +537,6 @@ impl ::kernel::ModuleMetadata for #type_ {
// Double nested modules, since then nobody can access the public items inside.
mod __module_init {
mod __module_init {
- use super::super::#type_;
use pin_init::PinInit;
/// The "Rust loadable module" mark.
@@ -548,7 +548,7 @@ mod __module_init {
#[used(compiler)]
static __IS_RUST_MODULE: () = ();
- static mut __MOD: ::core::mem::MaybeUninit<#type_> =
+ static mut __MOD: ::core::mem::MaybeUninit<super::super::LocalModule> =
::core::mem::MaybeUninit::uninit();
// Loadable modules need to export the `{init,cleanup}_module` identifiers.
@@ -635,8 +635,9 @@ pub extern "C" fn #ident_exit() {
///
/// This function must only be called once.
unsafe fn __init() -> ::kernel::ffi::c_int {
- let initer =
- <#type_ as ::kernel::InPlaceModule>::init(&super::super::THIS_MODULE);
+ let initer = <super::super::LocalModule as ::kernel::InPlaceModule>::init(
+ &super::super::THIS_MODULE
+ );
// SAFETY: No data race, since `__MOD` can only be accessed by this module
// and there only `__init` and `__exit` access it. These functions are only
// called once and `__exit` cannot be called before or during `__init`.
--
2.51.2
^ permalink raw reply related
* [PATCH v2 10/11] rust: macros: rearrange `#[doc(hidden)]` in `module!` macro
From: Gary Guo @ 2026-01-07 16:15 UTC (permalink / raw)
To: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Luis Chamberlain, Petr Pavlu, Daniel Gomez,
Sami Tolvanen, Aaron Tomlin
Cc: rust-for-linux, Tamir Duberstein, linux-modules, linux-kernel
In-Reply-To: <20260107161729.3855851-1-gary@kernel.org>
From: Gary Guo <gary@garyguo.net>
This `#[doc(hidden)]` can just be applied on a module to hide anything
within.
Reviewed-by: Tamir Duberstein <tamird@gmail.com>
Reviewed-by: Benno Lossin <lossin@kernel.org>
Signed-off-by: Gary Guo <gary@garyguo.net>
---
rust/macros/module.rs | 10 +---------
1 file changed, 1 insertion(+), 9 deletions(-)
diff --git a/rust/macros/module.rs b/rust/macros/module.rs
index 31d39764c6926..c86cced5e0e8f 100644
--- a/rust/macros/module.rs
+++ b/rust/macros/module.rs
@@ -77,7 +77,6 @@ fn emit_base(&mut self, field: &str, content: &str, builtin: bool, param: bool)
);
let item = quote! {
#cfg
- #[doc(hidden)]
#[cfg_attr(not(target_os = "macos"), link_section = ".modinfo")]
#[used(compiler)]
pub static #counter: [u8; #length] = *#string;
@@ -535,6 +534,7 @@ impl ::kernel::ModuleMetadata for #type_ {
}
// Double nested modules, since then nobody can access the public items inside.
+ #[doc(hidden)]
mod __module_init {
mod __module_init {
use pin_init::PinInit;
@@ -544,7 +544,6 @@ mod __module_init {
// This may be best done another way later on, e.g. as a new modinfo
// key or a new section. For the moment, keep it simple.
#[cfg(MODULE)]
- #[doc(hidden)]
#[used(compiler)]
static __IS_RUST_MODULE: () = ();
@@ -557,7 +556,6 @@ mod __module_init {
/// This function must not be called after module initialization, because it may be
/// freed after that completes.
#[cfg(MODULE)]
- #[doc(hidden)]
#[no_mangle]
#[link_section = ".init.text"]
pub unsafe extern "C" fn init_module() -> ::kernel::ffi::c_int {
@@ -568,14 +566,12 @@ mod __module_init {
}
#[cfg(MODULE)]
- #[doc(hidden)]
#[used(compiler)]
#[link_section = ".init.data"]
static __UNIQUE_ID___addressable_init_module: unsafe extern "C" fn() -> i32 =
init_module;
#[cfg(MODULE)]
- #[doc(hidden)]
#[no_mangle]
#[link_section = ".exit.text"]
pub extern "C" fn cleanup_module() {
@@ -589,7 +585,6 @@ pub extern "C" fn cleanup_module() {
}
#[cfg(MODULE)]
- #[doc(hidden)]
#[used(compiler)]
#[link_section = ".exit.data"]
static __UNIQUE_ID___addressable_cleanup_module: extern "C" fn() = cleanup_module;
@@ -598,7 +593,6 @@ pub extern "C" fn cleanup_module() {
// and the identifiers need to be unique.
#[cfg(not(MODULE))]
#[cfg(not(CONFIG_HAVE_ARCH_PREL32_RELOCATIONS))]
- #[doc(hidden)]
#[link_section = #initcall_section]
#[used(compiler)]
pub static #ident_initcall: extern "C" fn() ->
@@ -609,7 +603,6 @@ pub extern "C" fn cleanup_module() {
::core::arch::global_asm!(#global_asm);
#[cfg(not(MODULE))]
- #[doc(hidden)]
#[no_mangle]
pub extern "C" fn #ident_init() -> ::kernel::ffi::c_int {
// SAFETY: This function is inaccessible to the outside due to the double
@@ -619,7 +612,6 @@ pub extern "C" fn #ident_init() -> ::kernel::ffi::c_int {
}
#[cfg(not(MODULE))]
- #[doc(hidden)]
#[no_mangle]
pub extern "C" fn #ident_exit() {
// SAFETY:
--
2.51.2
^ permalink raw reply related
* Re: [PATCH] module: Remove duplicate freeing of lockdep classes
From: Aaron Tomlin @ 2026-01-07 16:31 UTC (permalink / raw)
To: Petr Pavlu
Cc: Luis Chamberlain, Daniel Gomez, Sami Tolvanen, Peter Zijlstra,
Ingo Molnar, Will Deacon, Boqun Feng, Waiman Long, Song Liu,
linux-modules, linux-kernel
In-Reply-To: <20260107122329.1324707-1-petr.pavlu@suse.com>
[-- Attachment #1: Type: text/plain, Size: 1606 bytes --]
On Wed, Jan 07, 2026 at 01:22:57PM +0100, Petr Pavlu wrote:
> In the error path of load_module(), under the free_module label, the
> code calls lockdep_free_key_range() to release lock classes associated
> with the MOD_DATA, MOD_RODATA and MOD_RO_AFTER_INIT module regions, and
> subsequently invokes module_deallocate().
>
> Since commit ac3b43283923 ("module: replace module_layout with
> module_memory"), the module_deallocate() function calls free_mod_mem(),
> which releases the lock classes as well and considers all module
> regions.
>
> Attempting to free these classes twice is unnecessary. Remove the
> redundant code in load_module().
>
> Signed-off-by: Petr Pavlu <petr.pavlu@suse.com>
> ---
> kernel/module/main.c | 6 ------
> 1 file changed, 6 deletions(-)
>
> diff --git a/kernel/module/main.c b/kernel/module/main.c
> index 710ee30b3bea..bcd259505c8b 100644
> --- a/kernel/module/main.c
> +++ b/kernel/module/main.c
> @@ -3544,12 +3544,6 @@ static int load_module(struct load_info *info, const char __user *uargs,
> mutex_unlock(&module_mutex);
> free_module:
> mod_stat_bump_invalid(info, flags);
> - /* Free lock-classes; relies on the preceding sync_rcu() */
> - for_class_mod_mem_type(type, core_data) {
> - lockdep_free_key_range(mod->mem[type].base,
> - mod->mem[type].size);
> - }
> -
> module_memory_restore_rox(mod);
> module_deallocate(mod, info);
> free_copy:
>
> base-commit: 3609fa95fb0f2c1b099e69e56634edb8fc03f87c
> --
> 2.52.0
>
Reviewed-by: Aaron Tomlin <atomlin@atomlin.com>
--
Aaron Tomlin
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox