* [PATCH RFC v3 05/15] rust: drm: fix missing owner in file_operations
From: Kari Argillander @ 2026-01-10 15:08 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: <20260110-this_module_fix-v3-0-97a3d9c14e8b@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 d49a9ba02635..705afea65ff6 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<TM: ThisModule>() -> bindings::file_operations {
let mut fops: bindings::file_operations = pin_init::zeroed();
- fops.owner = core::ptr::null_mut();
+ fops.owner = TM::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 v3 04/15] rust: block: fix missing owner field in block_device_operations
From: Kari Argillander @ 2026-01-10 15:08 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: <20260110-this_module_fix-v3-0-97a3d9c14e8b@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 v3 03/15] rust: miscdevice: fix use after free because missing .owner
From: Kari Argillander @ 2026-01-10 15:08 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: <20260110-this_module_fix-v3-0-97a3d9c14e8b@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 v3 02/15] rust: add new ThisModule trait and THIS_MODULE impl
From: Kari Argillander @ 2026-01-10 15:08 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: <20260110-this_module_fix-v3-0-97a3d9c14e8b@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 docs examples will need THIS_MODULE so also define that in docs.
Signed-off-by: Kari Argillander <kari.argillander@gmail.com>
---
rust/kernel/configfs.rs | 6 +-
rust/kernel/lib.rs | 159 ++++++++++++++++++++++++++++++++++++++++++++
rust/macros/module.rs | 16 +----
scripts/rustdoc_test_gen.rs | 2 +
4 files changed, 166 insertions(+), 17 deletions(-)
diff --git a/rust/kernel/configfs.rs b/rust/kernel/configfs.rs
index 466fb7f40762..fe80439ab21f 100644
--- a/rust/kernel/configfs.rs
+++ b/rust/kernel/configfs.rs
@@ -876,7 +876,7 @@ fn as_ptr(&self) -> *const bindings::config_item_type {
/// configfs::Subsystem<Configuration>,
/// Configuration
/// >::new_with_child_ctor::<N,Child>(
-/// &THIS_MODULE,
+/// THIS_MODULE.as_ref(),
/// &CONFIGURATION_ATTRS
/// );
///
@@ -1020,7 +1020,7 @@ macro_rules! configfs_attrs {
static [< $data:upper _TPE >] : $crate::configfs::ItemType<$container, $data> =
$crate::configfs::ItemType::<$container, $data>::new::<N>(
- &THIS_MODULE, &[<$ data:upper _ATTRS >]
+ THIS_MODULE.as_ref(), &[<$ data:upper _ATTRS >]
);
)?
@@ -1029,7 +1029,7 @@ macro_rules! configfs_attrs {
$crate::configfs::ItemType<$container, $data> =
$crate::configfs::ItemType::<$container, $data>::
new_with_child_ctor::<N, $child>(
- &THIS_MODULE, &[<$ data:upper _ATTRS >]
+ THIS_MODULE.as_ref(), &[<$ data:upper _ATTRS >]
);
)?
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 510d4bfc7c2b..4b899f75e56d 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 v3 01/15] rust: enable const_refs_to_static feature
From: Kari Argillander @ 2026-01-10 15:07 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: <20260110-this_module_fix-v3-0-97a3d9c14e8b@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 v3 00/15] rust: Reimplement ThisModule to fix ownership problems
From: Kari Argillander @ 2026-01-10 15:07 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. I would like to get feed back is this
resonable seperation and how we will land this. I have tried my best so
that it can be applied in multiple staged if needed. I have not receive
any feedback on this series and that is little bit worrying.
Introduce new ThisModule trait and THIS_MODULE impl.
So currently we have problem that we are not always filling .owner field
example 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 crate cannot use it. Currently we have
THIS_MODULE, LocalModule and
module: &' static ThisModule
on init functions. As we anyway need THIS_MODULE just use that for all
of these things.
Patches 1-2 introduce THIS_MODULE and ThisModule trait.
Patches 3-12 can be applied any order after 1-2.
Patch 13 depends on patches 11-12.
Patches 14-15 are clean up patches and depends 1-13.
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 v3:
- Edit cover.
- Seperate module name changes to seperate patches.
- Rebase top of next 20260109
- Seperate configfs changes to own commit.
- Fix one place still used M over TM for ThisModule.
- Link to v2: https://lore.kernel.org/r/20260106-this_module_fix-v2-0-842ac026f00b@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 (15):
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: configfs: use new THIS_MODULE
rust: binder: use new THIS_MODULE
rust: firmware: use THIS_MODULE over LocalModule for name
gpu: nova-core: use THIS_MODULE over LocalModule for name
samples: rust: auxiliary: use THIS_MODULE over LocalModule for name
rust: driver: make RegistrationOps::register() to use new ThisModule
rust: phy: make Registration::register() use new ThisModule
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: f417b7ffcbef7d76b0d8860518f50dae0e7e5eda
change-id: 20251230-this_module_fix-a390bff24897
Best regards,
--
Kari Argillander <kari.argillander@gmail.com>
^ permalink raw reply
* Re: [PATCH v11 6/8] crypto: Add RSASSA-PSS support
From: David Howells @ 2026-01-08 14:39 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: dhowells, Lukas Wunner, Ignat Korchagin, 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: <aV-t3m-ZxAcEqZmq@kernel.org>
Jarkko Sakkinen <jarkko@kernel.org> wrote:
> > +struct rsassa_pss_ctx {
> > + struct crypto_akcipher *rsa;
> > + unsigned int key_size;
> > + unsigned int salt_len;
> > + char *pss_hash;
> > + char *mgf1_hash;
> > +};
>
> Just a nit but I would not align these fields as it does not serve any
> purpose here.
It makes them easier to read.
David
^ permalink raw reply
* Re: [PATCH v11 3/8] pkcs7, x509: Add ML-DSA support
From: David Howells @ 2026-01-08 14:38 UTC (permalink / raw)
To: Eric Biggers, Herbert Xu
Cc: dhowells, Lukas Wunner, Ignat Korchagin, Jarkko Sakkinen,
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: <20260106080251.GD2630@sol>
Eric Biggers <ebiggers@kernel.org> wrote:
>
> 1.) If the CMS object doesn't include signed attributes, then it's a
> digest of the real message the caller provided.
Yeah - that needs fixing, but I need to be able to test it.
openssl-4.0 (at least that's what appears to be on the master branch) will
have a fix for ML-DSA CMS_NOATTR support (it was committed in November), but
it's not available yet unless you want to build your own.
sign-file would would normally use CMS_NOATTR, and this is worked round by
patch 4 in this series by using signed attributes for ML_DSA.
David
^ permalink raw reply
* Re: [PATCH] Documentation/kbuild: Document gendwarfksyms build dependencies
From: Miguel Ojeda @ 2026-01-08 14:10 UTC (permalink / raw)
To: linjh22s
Cc: Nathan Chancellor, Nicolas Schier, Jonathan Corbet, Miguel Ojeda,
Boqun Feng, Sami Tolvanen, Masahiro Yamada, Petr Pavlu,
linux-kbuild, linux-doc, linux-kernel, linux-modules
In-Reply-To: <20260108-documents_gendwarfksyms-v1-1-52b1f9c38c70@gmail.com>
On Thu, Jan 8, 2026 at 9:45 AM Jihan LIN via B4 Relay
<devnull+linjh22s.gmail.com@kernel.org> wrote:
>
> Fixes: f28568841ae0 ("tools: Add gendwarfksyms")
Not sure if this is meant to be a fix or not; but if it is, then Cc:
stable should be considered since that commit is in an LTS and a
Stable kernel.
> +Dependencies
> +-----
Shouldn't this be the full length?
> + sudo pacman --needed -S zlib libelf
I think these are supposed to be a tab (at least this file uses that)..
Also, Cc'ing linux-modules@vger.kernel.org as per the "GENDWARFKSYMS"
entry in `MAINTAINERS`.
I hope that helps!
Cheers,
Miguel
^ permalink raw reply
* Re: [PATCH v11 6/8] crypto: Add RSASSA-PSS support
From: Jarkko Sakkinen @ 2026-01-08 13:15 UTC (permalink / raw)
To: David Howells
Cc: Lukas Wunner, Ignat Korchagin, 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>
On Mon, Jan 05, 2026 at 03:21:31PM +0000, David Howells 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;
> +};
Just a nit but I would not align these fields as it does not serve any
purpose here.
> +
> +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;
> +
> + 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); });
> +
> +/*
> + * 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;
> + 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;
> + 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;
> + 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;
> + 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
>
BR, Jarkko
^ permalink raw reply
* Re: [PATCH v11 2/8] pkcs7: Allow the signing algo to calculate the digest itself
From: David Howells @ 2026-01-08 12:59 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: <2366240.1767794004@warthog.procyon.org.uk>
David Howells <dhowells@redhat.com> wrote:
> 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.
Actually, we probably do need to copy it. The problem is that we have to
modify the tag on the authenticatedAttributes (PKCS#7)/signedAttrs (CMS) blob
before we digest it, e.g. in pkcs7_digest():
memcpy(sig->digest, sinfo->authattrs, sinfo->authattrs_len);
((u8 *)sig->digest)[0] = ASN1_CONS_BIT | ASN1_SET;
as specified in RFC9882 and other places:
3.2. Signature Generation and Verification
...
When signed attributes are included, ML-DSA (pure mode) signatures are
computed over the complete DER encoding of the SignedAttrs value
contained in the SignerInfo's signedAttrs field. As described in
Section 5.4 of [RFC5652], this encoding includes the tag and length
octets, but an EXPLICIT SET OF tag is used rather than the IMPLICIT
[0] tag that appears in the final message. ...
We might be able to get away with modifying it in place - but I don't know
that that's true for all users.
David
^ permalink raw reply
* Re: [PATCH] software node: replace -EEXIST with -EBUSY
From: Greg Kroah-Hartman @ 2026-01-08 11:58 UTC (permalink / raw)
To: Daniel Gomez
Cc: Andy Shevchenko, Daniel Scally, Heikki Krogerus, Sakari Ailus,
Rafael J. Wysocki, Danilo Krummrich, Luis Chamberlain, Petr Pavlu,
Sami Tolvanen, Aaron Tomlin, Lucas De Marchi, linux-acpi,
linux-modules, linux-kernel, Daniel Gomez
In-Reply-To: <320328d3-6714-4c28-a19c-c9113b25c2af@kernel.org>
On Thu, Jan 08, 2026 at 12:51:46PM +0100, Daniel Gomez wrote:
>
>
> On 22/12/2025 12.56, Greg Kroah-Hartman wrote:
> > On Mon, Dec 22, 2025 at 09:48:54AM +0100, Daniel Gomez wrote:
> >> On 22/12/2025 09.19, Greg Kroah-Hartman wrote:
> >>> On Sat, Dec 20, 2025 at 04:55:00AM +0100, Daniel Gomez 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. 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:
> >>>> * meraki_mx100 pcengines_apuv2
> >>>>
> >>>> Signed-off-by: Daniel Gomez <da.gomez@samsung.com>
> >>>> ---
> >>>> The error code -EEXIST is reserved by the kernel module loader to
> >>>> indicate that a module with the same name is already loaded. When a
> >>>> module's init function returns -EEXIST, kmod interprets this as "module
> >>>> already loaded" and reports success instead of failure [1].
> >>>>
> >>>> The kernel module loader will include a safety net that provides -EEXIST
> >>>> to -EBUSY with a warning [2], and a documentation patch has been sent to
> >>>> prevent future occurrences [3].
> >>>>
> >>>> These affected code paths were identified using a static analysis tool
> >>>> [4] that traces -EEXIST returns to module_init(). The tool was developed
> >>>> with AI assistance and all findings were manually validated.
> >>>>
> >>>> Link: https://lore.kernel.org/all/aKEVQhJpRdiZSliu@orbyte.nwl.cc/ [1]
> >>>> Link: https://lore.kernel.org/all/20251013-module-warn-ret-v1-0-ab65b41af01f@intel.com/ [2]
> >>>> Link: https://lore.kernel.org/all/20251218-dev-module-init-eexists-modules-docs-v1-0-361569aa782a@samsung.com/ [3]
> >>>> Link: https://gitlab.com/-/snippets/4913469 [4]
> >>>> ---
> >>>> drivers/base/swnode.c | 2 +-
> >>>> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
> >>>> index 16a8301c25d6..083593d99a18 100644
> >>>> --- a/drivers/base/swnode.c
> >>>> +++ b/drivers/base/swnode.c
> >>>> @@ -919,7 +919,7 @@ int software_node_register(const struct software_node *node)
> >>>> struct swnode *parent = software_node_to_swnode(node->parent);
> >>>>
> >>>> if (software_node_to_swnode(node))
> >>>> - return -EEXIST;
> >>>> + return -EBUSY;
> >>>
> >>> While I understand the want for the module loader to be returning
> >>> -EBUSY, that doesn't really make sense down here in this layer of the
> >>> kernel.
> >>>
> >>> So why doesn't the module loader turn -EEXIST return values into -EBUSY
> >>> if it wishes to pass that value on to userspace? Otherwise you are
> >>
> >> Indeed, we are planning to do that as well with "[PATCH 0/2] module: Tweak
> >> return and warning":
> >>
> >> https://lore.kernel.org/all/20251013-module-warn-ret-v1-0-ab65b41af01f@intel.com/#t
> >>
> >> However, we don't consider that as the right fix.
> >>
> >>> going to be constantly playing "whack-a-mole" here and have really
> >>> set things up so that NO api can ever return EEXIST as an error value in
> >>> the future.
> >>
> >> 100%.
> >>
> >> For that reason, on top of the series from Lucas, we are documenting this to
> >> make it clear:
> >>
> >> https://lore.kernel.org/linux-modules/20251218-dev-module-init-eexists-modules-docs-v1-0-361569aa782a@samsung.com/T/#m2ed6fbffb3f78b9bff53840f6492a198c389cb50
> >
> > Wait, no, that's not what I mean at all :)
> >
> >> And sending patches where we see modules need fixing. I have already sent 6 out
> >> of 20-ish series (that include a total of 40+ fixes):
> >>
> >> https://lore.kernel.org/all/20251220-dev-module-init-eexists-linux-scsi-v1-0-5379db749d54@samsung.com
> >> https://lore.kernel.org/all/20251219-dev-module-init-eexists-netfilter-v1-1-efd3f62412dc@samsung.com
> >> https://lore.kernel.org/all/20251220-dev-module-init-eexists-bpf-v1-1-7f186663dbe7@samsung.com
> >> https://lore.kernel.org/all/20251220-dev-module-init-eexists-keyring-v1-1-a2f23248c300@samsung.com
> >> https://lore.kernel.org/all/20251220-dev-module-init-eexists-dm-devel-v1-1-90ed00444ea0@samsung.com
> >
> > Please no, let us keep using -EEXIST in the kernel source, and if your
>
> This is not just random places in the kernel. It's only errors in the module
> initialization path.
The "module initialization path" is deep. really really deep. Look at
the "probe" path for many drivers, that can be quite intensive and deep,
so attempting to audit all EEXIST errors for all of those paths is going
to be rough, if impossible.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH] software node: replace -EEXIST with -EBUSY
From: Daniel Gomez @ 2026-01-08 11:55 UTC (permalink / raw)
To: Lucas De Marchi, Greg Kroah-Hartman
Cc: Andy Shevchenko, Daniel Scally, Heikki Krogerus, Sakari Ailus,
Rafael J. Wysocki, Danilo Krummrich, Luis Chamberlain, Petr Pavlu,
Sami Tolvanen, Aaron Tomlin, linux-acpi, linux-modules,
linux-kernel, Daniel Gomez
In-Reply-To: <4zatcu4izel7yijmkinxy6wq6owktwsyxkazb5ufc4vua54ojx@3vq4dgtydgia>
On 06/01/2026 15.24, Lucas De Marchi wrote:
> On Mon, Dec 22, 2025 at 12:56:38PM +0100, Greg Kroah-Hartman wrote:
>> On Mon, Dec 22, 2025 at 09:48:54AM +0100, Daniel Gomez wrote:
>>> On 22/12/2025 09.19, Greg Kroah-Hartman wrote:
>>> > On Sat, Dec 20, 2025 at 04:55:00AM +0100, Daniel Gomez 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. 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:
>>> >> * meraki_mx100 pcengines_apuv2
>>> >>
>>> >> Signed-off-by: Daniel Gomez <da.gomez@samsung.com>
>>> >> ---
>>> >> The error code -EEXIST is reserved by the kernel module loader to
>>> >> indicate that a module with the same name is already loaded. When a
>>> >> module's init function returns -EEXIST, kmod interprets this as "module
>>> >> already loaded" and reports success instead of failure [1].
>>> >>
>>> >> The kernel module loader will include a safety net that provides -EEXIST
>>> >> to -EBUSY with a warning [2], and a documentation patch has been sent to
>>> >> prevent future occurrences [3].
>>> >>
>>> >> These affected code paths were identified using a static analysis tool
>>> >> [4] that traces -EEXIST returns to module_init(). The tool was developed
>>> >> with AI assistance and all findings were manually validated.
>>> >>
>>> >> Link: https://lore.kernel.org/all/aKEVQhJpRdiZSliu@orbyte.nwl.cc/ [1]
>>> >> Link: https://lore.kernel.org/all/20251013-module-warn-ret-v1-0-ab65b41af01f@intel.com/ [2]
>>> >> Link: https://lore.kernel.org/all/20251218-dev-module-init-eexists-modules-docs-v1-0-361569aa782a@samsung.com/ [3]
>>> >> Link: https://gitlab.com/-/snippets/4913469 [4]
>>> >> ---
>>> >> drivers/base/swnode.c | 2 +-
>>> >> 1 file changed, 1 insertion(+), 1 deletion(-)
>>> >>
>>> >> diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
>>> >> index 16a8301c25d6..083593d99a18 100644
>>> >> --- a/drivers/base/swnode.c
>>> >> +++ b/drivers/base/swnode.c
>>> >> @@ -919,7 +919,7 @@ int software_node_register(const struct software_node *node)
>>> >> struct swnode *parent = software_node_to_swnode(node->parent);
>>> >>
>>> >> if (software_node_to_swnode(node))
>>> >> - return -EEXIST;
>>> >> + return -EBUSY;
>>> >
>>> > While I understand the want for the module loader to be returning
>>> > -EBUSY, that doesn't really make sense down here in this layer of the
>>> > kernel.
>>> >
>>> > So why doesn't the module loader turn -EEXIST return values into -EBUSY
>>> > if it wishes to pass that value on to userspace? Otherwise you are
>>>
>>> Indeed, we are planning to do that as well with "[PATCH 0/2] module: Tweak
>>> return and warning":
>>>
>>> https://lore.kernel.org/all/20251013-module-warn-ret-v1-0-ab65b41af01f@intel.com/#t
>>>
>>> However, we don't consider that as the right fix.
>>>
>>> > going to be constantly playing "whack-a-mole" here and have really
>>> > set things up so that NO api can ever return EEXIST as an error value in
>>> > the future.
>>>
>>> 100%.
>>>
>>> For that reason, on top of the series from Lucas, we are documenting this to
>>> make it clear:
>>>
>>> https://lore.kernel.org/linux-modules/20251218-dev-module-init-eexists-modules-docs-v1-0-361569aa782a@samsung.com/T/#m2ed6fbffb3f78b9bff53840f6492a198c389cb50
>>
>> Wait, no, that's not what I mean at all :)
>>
>>> And sending patches where we see modules need fixing. I have already sent 6 out
>>> of 20-ish series (that include a total of 40+ fixes):
>>>
>>> https://lore.kernel.org/all/20251220-dev-module-init-eexists-linux-scsi-v1-0-5379db749d54@samsung.com
>>> https://lore.kernel.org/all/20251219-dev-module-init-eexists-netfilter-v1-1-efd3f62412dc@samsung.com
>>> https://lore.kernel.org/all/20251220-dev-module-init-eexists-bpf-v1-1-7f186663dbe7@samsung.com
>>> https://lore.kernel.org/all/20251220-dev-module-init-eexists-keyring-v1-1-a2f23248c300@samsung.com
>>> https://lore.kernel.org/all/20251220-dev-module-init-eexists-dm-devel-v1-1-90ed00444ea0@samsung.com
>>
>> Please no, let us keep using -EEXIST in the kernel source, and if your
>> usage is going to map this to userspace somehow, do the translation
>> there, in the module code, as your original patch above said.
>>
>> Otherwise, again, this is never going to work, let the subsystems use
>> this error code how ever they feel they need to.
>
> Ok. When I added the warning I was more following what the other error
> handling was doing for positive values. Happy to change that to simply
> map the error code before returning from do_init_module().
>
> Daniel, do you want me to resend that with the warning removed?
Yes please, I think we should do that and explain the agreement in this thread
in the commit message so others can understand the why.
^ permalink raw reply
* Re: [PATCH v11 7/8] pkcs7, x509: Add RSASSA-PSS support
From: David Howells @ 2026-01-08 11: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=nHMm0Br9iaMyCQwbujb-vus3nsA-__1Nw=UG-an=B_jtg@mail.gmail.com>
Ignat Korchagin <ignat@cloudflare.com> wrote:
> > + case OID_id_rsassa_pss:
> > + goto rsassa_pss;
> ...
> > +rsassa_pss:
> > + if (!ctx->algo_params || !ctx->algo_params_size) {
> > + pr_debug("RSASSA-PSS sig algo without parameters\n");
> > + return -EBADMSG;
> > + }
> > +
> > + err = rsassa_parse_sig_params(sig, ctx->algo_params, ctx->algo_params_size);
> > + if (err < 0)
> > + return err;
> > +
> > + sig->pkey_algo = "rsa";
> > + sig->encoding = "emsa-pss";
> > + goto out;
> > }
>
> I really don't like this. Is it possible to factor this out to a
> separate function and just call here? Should the factored function
> even be part of the implementation somehow?
I'll move the check into rsassa_parse_sig_params() and then move the remaining
code back into the switch case. That also means that x509_note_sig_algo()
doesn't need the check either. It does change the pr_fmt value seen by the
pr_debug(), but that's probably fine.
> > ctx->last_oid = look_up_OID(value, vlen);
> > if (ctx->last_oid == OID__NR) {
> > - char buffer[50];
> > + char buffer[56];
> > sprint_oid(value, vlen, buffer, sizeof(buffer));
>
> I've seen this elsewhere in the crypto code (namely in ECC) but is it
> generally a good idea to declare long buffers on the stack?
It's not all that long (7 words on a 64-bit machine - similarish to a function
call), and the output of sprint_oid() is limited to it.
David
^ permalink raw reply
* Re: [PATCH] software node: replace -EEXIST with -EBUSY
From: Daniel Gomez @ 2026-01-08 11:51 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Andy Shevchenko, Daniel Scally, Heikki Krogerus, Sakari Ailus,
Rafael J. Wysocki, Danilo Krummrich, Luis Chamberlain, Petr Pavlu,
Sami Tolvanen, Aaron Tomlin, Lucas De Marchi, linux-acpi,
linux-modules, linux-kernel, Daniel Gomez
In-Reply-To: <2025122212-fiction-setback-ede5@gregkh>
On 22/12/2025 12.56, Greg Kroah-Hartman wrote:
> On Mon, Dec 22, 2025 at 09:48:54AM +0100, Daniel Gomez wrote:
>> On 22/12/2025 09.19, Greg Kroah-Hartman wrote:
>>> On Sat, Dec 20, 2025 at 04:55:00AM +0100, Daniel Gomez 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. 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:
>>>> * meraki_mx100 pcengines_apuv2
>>>>
>>>> Signed-off-by: Daniel Gomez <da.gomez@samsung.com>
>>>> ---
>>>> The error code -EEXIST is reserved by the kernel module loader to
>>>> indicate that a module with the same name is already loaded. When a
>>>> module's init function returns -EEXIST, kmod interprets this as "module
>>>> already loaded" and reports success instead of failure [1].
>>>>
>>>> The kernel module loader will include a safety net that provides -EEXIST
>>>> to -EBUSY with a warning [2], and a documentation patch has been sent to
>>>> prevent future occurrences [3].
>>>>
>>>> These affected code paths were identified using a static analysis tool
>>>> [4] that traces -EEXIST returns to module_init(). The tool was developed
>>>> with AI assistance and all findings were manually validated.
>>>>
>>>> Link: https://lore.kernel.org/all/aKEVQhJpRdiZSliu@orbyte.nwl.cc/ [1]
>>>> Link: https://lore.kernel.org/all/20251013-module-warn-ret-v1-0-ab65b41af01f@intel.com/ [2]
>>>> Link: https://lore.kernel.org/all/20251218-dev-module-init-eexists-modules-docs-v1-0-361569aa782a@samsung.com/ [3]
>>>> Link: https://gitlab.com/-/snippets/4913469 [4]
>>>> ---
>>>> drivers/base/swnode.c | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
>>>> index 16a8301c25d6..083593d99a18 100644
>>>> --- a/drivers/base/swnode.c
>>>> +++ b/drivers/base/swnode.c
>>>> @@ -919,7 +919,7 @@ int software_node_register(const struct software_node *node)
>>>> struct swnode *parent = software_node_to_swnode(node->parent);
>>>>
>>>> if (software_node_to_swnode(node))
>>>> - return -EEXIST;
>>>> + return -EBUSY;
>>>
>>> While I understand the want for the module loader to be returning
>>> -EBUSY, that doesn't really make sense down here in this layer of the
>>> kernel.
>>>
>>> So why doesn't the module loader turn -EEXIST return values into -EBUSY
>>> if it wishes to pass that value on to userspace? Otherwise you are
>>
>> Indeed, we are planning to do that as well with "[PATCH 0/2] module: Tweak
>> return and warning":
>>
>> https://lore.kernel.org/all/20251013-module-warn-ret-v1-0-ab65b41af01f@intel.com/#t
>>
>> However, we don't consider that as the right fix.
>>
>>> going to be constantly playing "whack-a-mole" here and have really
>>> set things up so that NO api can ever return EEXIST as an error value in
>>> the future.
>>
>> 100%.
>>
>> For that reason, on top of the series from Lucas, we are documenting this to
>> make it clear:
>>
>> https://lore.kernel.org/linux-modules/20251218-dev-module-init-eexists-modules-docs-v1-0-361569aa782a@samsung.com/T/#m2ed6fbffb3f78b9bff53840f6492a198c389cb50
>
> Wait, no, that's not what I mean at all :)
>
>> And sending patches where we see modules need fixing. I have already sent 6 out
>> of 20-ish series (that include a total of 40+ fixes):
>>
>> https://lore.kernel.org/all/20251220-dev-module-init-eexists-linux-scsi-v1-0-5379db749d54@samsung.com
>> https://lore.kernel.org/all/20251219-dev-module-init-eexists-netfilter-v1-1-efd3f62412dc@samsung.com
>> https://lore.kernel.org/all/20251220-dev-module-init-eexists-bpf-v1-1-7f186663dbe7@samsung.com
>> https://lore.kernel.org/all/20251220-dev-module-init-eexists-keyring-v1-1-a2f23248c300@samsung.com
>> https://lore.kernel.org/all/20251220-dev-module-init-eexists-dm-devel-v1-1-90ed00444ea0@samsung.com
>
> Please no, let us keep using -EEXIST in the kernel source, and if your
This is not just random places in the kernel. It's only errors in the module
initialization path.
> usage is going to map this to userspace somehow, do the translation
> there, in the module code, as your original patch above said.
> > Otherwise, again, this is never going to work, let the subsystems use
> this error code how ever they feel they need to.
I considered module_init() (somehow) to be part of the module code, and
replacing the error at the module loading layer felt like a hack to me. My
concern is that a module_init() user expecting -EEXIST to propagate to userspace
won't get it as it will be silently replaced. But without a concrete use case
where that matters, I'll go with the consensus. Note that I'm hinting at we
should remove the warning from Lucas' original patch [1] before merging it.
Link: https://lore.kernel.org/all/20251013-module-warn-ret-v1-1-ab65b41af01f@intel.com/ [1]
^ permalink raw reply
* Re: [PATCH v11 6/8] crypto: Add RSASSA-PSS support
From: David Howells @ 2026-01-08 11:29 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,
Tadeusz Struk, David S. Miller
In-Reply-To: <CALrw=nHhs61yqkLkK9F4UGU_ujnniMzrkbhjRDc+Aa69XTFTvg@mail.gmail.com>
Ignat Korchagin <ignat@cloudflare.com> wrote:
> A lot of pointers and arithmetic here. Wouldn't it be easier to do
> something like in [1]?
Fair point.
> > +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?
Maybe - I guess there's no actual cost to doing so as it generates an inline
function.
> > + 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.
Ok, I'll move the decls.
David
^ permalink raw reply
* Re: [PATCH 3/3] module: Add compile-time check for embedded NUL characters
From: Andy Shevchenko @ 2026-01-08 8:49 UTC (permalink / raw)
To: Luck, Tony
Cc: Dan Carpenter, Al Viro, Daniel Gomez, Sami Tolvanen, Chris Li,
Eric Biggers, Kees Cook, Luis Chamberlain, Rusty Russell,
Petr Pavlu, linux-modules@vger.kernel.org, Malcolm Priestley,
Mauro Carvalho Chehab, Hans Verkuil, Uwe Kleine-König,
linux-kernel@vger.kernel.org, linux-media@vger.kernel.org,
linux-hardening@vger.kernel.org
In-Reply-To: <aUV7kyjxlijuy5sC@agluck-desk3>
On Fri, Dec 19, 2025 at 08:21:39AM -0800, Luck, Tony wrote:
> On Fri, Dec 19, 2025 at 03:45:42PM +0300, Dan Carpenter wrote:
> > On Fri, Dec 12, 2025 at 02:30:48AM +0900, Daniel Gomez wrote:
> > > Maybe the flag fix just needs to be applied to the evaluation? Other op
> > > structs do the same. But Dan's patch did not implement evaluate. E.g.:
> > >
> > > static struct symbol_op constant_p_op = {
> > > .evaluate = evaluate_to_int_const_expr,
> > > .expand = expand_constant_p
> > > };
> >
> > I was waiting for you to send this as a patch. I can do it if you
> > need me to.
>
> Al Viro thought this was wrong. His alternative patch is here:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/viro/sparse.git/commit/?id=2634e39bf02697a18fece057208150362c985992
Sparse still is PITA as of today, can we get some fix (Al's or alternative)
ASAP to be applied and sparse tagged as 0.6.5 so the distros will pack the
new version, please?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* [PATCH 2/2] docs: symbol-namespaces: mention sysfs attribute
From: Nicholas Sielicki @ 2026-01-08 4:47 UTC (permalink / raw)
To: Luis Chamberlain, Petr Pavlu, Daniel Gomez
Cc: Sami Tolvanen, Aaron Tomlin, Matthias Maennich, Peter Zijlstra,
Jonathan Corbet, Randy Dunlap, linux-modules, linux-kernel,
Nicholas Sielicki
In-Reply-To: <20260108044710.33036-1-linux@opensource.nslick.com>
Reference the new /sys/module/*/import_ns sysfs attribute in docs as an
alternative to modinfo for inspecting imported namespaces of loaded
modules.
Signed-off-by: Nicholas Sielicki <linux@opensource.nslick.com>
---
Documentation/core-api/symbol-namespaces.rst | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/Documentation/core-api/symbol-namespaces.rst b/Documentation/core-api/symbol-namespaces.rst
index 034898e81ba2..2304d5bffcce 100644
--- a/Documentation/core-api/symbol-namespaces.rst
+++ b/Documentation/core-api/symbol-namespaces.rst
@@ -114,6 +114,11 @@ inspected with modinfo::
import_ns: USB_STORAGE
[...]
+For modules that are currently loaded, imported namespaces are also available
+via sysfs::
+
+ $ cat /sys/module/ums_karma/import_ns
+ USB_STORAGE
It is advisable to add the MODULE_IMPORT_NS() statement close to other module
metadata definitions like MODULE_AUTHOR() or MODULE_LICENSE().
--
2.52.0
^ permalink raw reply related
* [PATCH 1/2] module: expose imported namespaces via sysfs
From: Nicholas Sielicki @ 2026-01-08 4:47 UTC (permalink / raw)
To: Luis Chamberlain, Petr Pavlu, Daniel Gomez
Cc: Sami Tolvanen, Aaron Tomlin, Matthias Maennich, Peter Zijlstra,
Jonathan Corbet, Randy Dunlap, linux-modules, linux-kernel,
Nicholas Sielicki
Currently, the only way for userspace to determine which symbol
namespaces a module imports is to locate the .ko file on disk (which may
not match the loaded module), then either parsing the binary manually
and handling any potential compression, or shelling-out to modinfo.
This is painful in cases where userspace wants to distinguish between
module variants that share the same name but import different
namespaces. For example, the nvidia-uvm module exists in both open and
closed source variants; the open source version imports the DMA_BUF
namespace while the closed source version does not, and networking
middleware may want to initialize itself differently depending on that.
Add /sys/module/*/import_ns to expose imported namespaces for loaded
modules. The file contains one namespace per line and only exists for
modules that import at least one namespace.
Signed-off-by: Nicholas Sielicki <linux@opensource.nslick.com>
---
Documentation/ABI/testing/sysfs-module | 9 ++++
include/linux/module.h | 1 +
kernel/module/main.c | 62 +++++++++++++++++++++++++-
3 files changed, 71 insertions(+), 1 deletion(-)
diff --git a/Documentation/ABI/testing/sysfs-module b/Documentation/ABI/testing/sysfs-module
index 6bc9af6229f0..d8e2a33fd514 100644
--- a/Documentation/ABI/testing/sysfs-module
+++ b/Documentation/ABI/testing/sysfs-module
@@ -48,6 +48,15 @@ Contact: Kay Sievers <kay.sievers@vrfy.org>
Description: Show the initialization state(live, coming, going) of
the module.
+What: /sys/module/*/import_ns
+Date: January 2026
+KernelVersion: 6.20
+Contact: linux-modules@vger.kernel.org
+Description: List of symbol namespaces imported by this module via
+ MODULE_IMPORT_NS(). Each namespace appears on a separate line.
+ This file only exists for modules that import at least one
+ namespace.
+
What: /sys/module/*/taint
Date: Jan 2012
KernelVersion: 3.3
diff --git a/include/linux/module.h b/include/linux/module.h
index d80c3ea57472..f1bcca03f90b 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -419,6 +419,7 @@ struct module {
struct module_attribute *modinfo_attrs;
const char *version;
const char *srcversion;
+ const char *imported_namespaces;
struct kobject *holders_dir;
/* Exported symbols */
diff --git a/kernel/module/main.c b/kernel/module/main.c
index 710ee30b3bea..6c41934f1135 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -607,6 +607,23 @@ static const struct module_attribute modinfo_##field = { \
MODINFO_ATTR(version);
MODINFO_ATTR(srcversion);
+static ssize_t show_modinfo_import_ns(const struct module_attribute *mattr,
+ struct module_kobject *mk, char *buffer)
+{
+ return sysfs_emit(buffer, "%s\n", mk->mod->imported_namespaces);
+}
+
+static int modinfo_import_ns_exists(struct module *mod)
+{
+ return mod->imported_namespaces != NULL;
+}
+
+static const struct module_attribute modinfo_import_ns = {
+ .attr = { .name = "import_ns", .mode = 0444 },
+ .show = show_modinfo_import_ns,
+ .test = modinfo_import_ns_exists,
+};
+
static struct {
char name[MODULE_NAME_LEN];
char taints[MODULE_FLAGS_BUF_SIZE];
@@ -1058,6 +1075,7 @@ const struct module_attribute *const modinfo_attrs[] = {
&module_uevent,
&modinfo_version,
&modinfo_srcversion,
+ &modinfo_import_ns,
&modinfo_initstate,
&modinfo_coresize,
#ifdef CONFIG_ARCH_WANTS_MODULES_DATA_IN_VMALLOC
@@ -1753,11 +1771,48 @@ static void module_license_taint_check(struct module *mod, const char *license)
}
}
+static int copy_modinfo_import_ns(struct module *mod, struct load_info *info)
+{
+ char *ns;
+ size_t len, total_len = 0;
+ char *buf, *p;
+
+ for_each_modinfo_entry(ns, info, "import_ns")
+ total_len += strlen(ns) + 1;
+
+ if (!total_len) {
+ mod->imported_namespaces = NULL;
+ return 0;
+ }
+
+ buf = kmalloc(total_len, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ p = buf;
+ for_each_modinfo_entry(ns, info, "import_ns") {
+ len = strlen(ns);
+ memcpy(p, ns, len);
+ p += len;
+ *p++ = '\n';
+ }
+ /* Replace trailing newline with null terminator. */
+ *(p - 1) = '\0';
+
+ mod->imported_namespaces = buf;
+ return 0;
+}
+
+static void free_modinfo_import_ns(struct module *mod)
+{
+ kfree(mod->imported_namespaces);
+}
+
static int setup_modinfo(struct module *mod, struct load_info *info)
{
const struct module_attribute *attr;
char *imported_namespace;
- int i;
+ int i, err;
for (i = 0; (attr = modinfo_attrs[i]); i++) {
if (attr->setup)
@@ -1776,6 +1831,10 @@ static int setup_modinfo(struct module *mod, struct load_info *info)
}
}
+ err = copy_modinfo_import_ns(mod, info);
+ if (err)
+ return err;
+
return 0;
}
@@ -1788,6 +1847,7 @@ static void free_modinfo(struct module *mod)
if (attr->free)
attr->free(mod);
}
+ free_modinfo_import_ns(mod);
}
bool __weak module_init_section(const char *name)
--
2.52.0
^ permalink raw reply related
* Re: [PATCH v2 05/11] rust: macros: use `quote!` for `module!` macro
From: Gary Guo @ 2026-01-07 17:54 UTC (permalink / raw)
To: Tamir Duberstein
Cc: Miguel Ojeda, Boqun Feng, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
Aaron Tomlin, rust-for-linux, linux-modules, linux-kernel
In-Reply-To: <CAJ-ks9nzn3wgZMaQFXFHBOEf-Fuhr61Tp7Knvu0CWOWYgOEqDw@mail.gmail.com>
On Wed, 7 Jan 2026 12:19:26 -0500
Tamir Duberstein <tamird@gmail.com> wrote:
> On Wed, Jan 7, 2026 at 11:59 AM Gary Guo <gary@kernel.org> wrote:
> >
> > 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>
>
> Are there significant changes here? You didn't pick up my tag.
There're changes related to added `params` which you probably already
noticed.
Best,
Gary
^ permalink raw reply
* Re: [PATCH v2 05/11] rust: macros: use `quote!` for `module!` macro
From: Tamir Duberstein @ 2026-01-07 17:19 UTC (permalink / raw)
To: Gary Guo
Cc: Miguel Ojeda, Boqun Feng, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
Aaron Tomlin, rust-for-linux, linux-modules, linux-kernel
In-Reply-To: <20260107161729.3855851-6-gary@kernel.org>
On Wed, Jan 7, 2026 at 11:59 AM Gary Guo <gary@kernel.org> wrote:
>
> 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>
Are there significant changes here? You didn't pick up my tag.
> ---
> 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();
This renaming is unfortunately; these variables were just introduced
in the previous patch.
>
> - 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
* Re: [PATCH v2 04/11] rust: macros: use `syn` to parse `module!` macro
From: Tamir Duberstein @ 2026-01-07 17:11 UTC (permalink / raw)
To: Gary Guo
Cc: Miguel Ojeda, Boqun Feng, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
Aaron Tomlin, José Expósito, rust-for-linux,
Patrick Miller, linux-kernel, linux-modules
In-Reply-To: <20260107161729.3855851-5-gary@kernel.org>
On Wed, Jan 7, 2026 at 11:30 AM Gary Guo <gary@kernel.org> wrote:
>
> 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>
Reviewed-by: Tamir Duberstein <tamird@gmail.com>
> ---
> 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>()?,
> +/// }
> +/// ```
It's a shame that this macro bails on the first failure rather than
accumulating them all.
> +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;
> + }
`while !$input.is_empty()`?
> +
> + 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)*
> + )
> + };
It would be nice to avoid the combinatorial explosion here, though I'm
not immediately sure how.
> +
> + (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
* Re: [PATCH v11 8/8] modsign: Enable RSASSA-PSS module signing
From: Ignat Korchagin @ 2026-01-07 16:38 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: <20260105152145.1801972-9-dhowells@redhat.com>
On Mon, Jan 5, 2026 at 3:22 PM David Howells <dhowells@redhat.com> wrote:
>
> Add support for RSASSA-PSS signatures (RFC8017) for use with module signing
> and other public key cryptography done by the kernel.
>
> Note that only signature verification is supported by the kernel.
>
> Note further that this alters some of the same code as the MLDSA support,
> so that needs to be applied first to avoid conflicts.
>
> Signed-off-by: David Howells <dhowells@redhat.com>
> cc: Lukas Wunner <lukas@wunner.de>
> cc: Ignat Korchagin <ignat@cloudflare.com>
> cc: Herbert Xu <herbert@gondor.apana.org.au>
> cc: keyrings@vger.kernel.org
> cc: linux-crypto@vger.kernel.org
> ---
> certs/Kconfig | 6 ++++++
> certs/Makefile | 1 +
> scripts/sign-file.c | 39 +++++++++++++++++++++++++++++++++++++--
> 3 files changed, 44 insertions(+), 2 deletions(-)
>
> diff --git a/certs/Kconfig b/certs/Kconfig
> index 94b086684d07..beb8991ad761 100644
> --- a/certs/Kconfig
> +++ b/certs/Kconfig
> @@ -27,6 +27,12 @@ config MODULE_SIG_KEY_TYPE_RSA
> help
> Use an RSA key for module signing.
>
> +config MODULE_SIG_KEY_TYPE_RSASSA_PSS
> + bool "RSASSA-PSS"
> + select CRYPTO_RSA
> + help
> + Use an RSASSA-PSS key for module signing.
> +
> config MODULE_SIG_KEY_TYPE_ECDSA
> bool "ECDSA"
> select CRYPTO_ECDSA
> diff --git a/certs/Makefile b/certs/Makefile
> index 3ee1960f9f4a..3b5a3a303f4c 100644
> --- a/certs/Makefile
> +++ b/certs/Makefile
> @@ -42,6 +42,7 @@ targets += x509_certificate_list
> # boolean option and we unfortunately can't make it depend on !RANDCONFIG.
> ifeq ($(CONFIG_MODULE_SIG_KEY),certs/signing_key.pem)
>
> +keytype-$(CONFIG_MODULE_SIG_KEY_TYPE_RSASSA_PSS) := -newkey rsassa-pss
> keytype-$(CONFIG_MODULE_SIG_KEY_TYPE_ECDSA) := -newkey ec -pkeyopt ec_paramgen_curve:secp384r1
> keytype-$(CONFIG_MODULE_SIG_KEY_TYPE_MLDSA_44) := -newkey ml-dsa-44
> keytype-$(CONFIG_MODULE_SIG_KEY_TYPE_MLDSA_65) := -newkey ml-dsa-65
> diff --git a/scripts/sign-file.c b/scripts/sign-file.c
> index b726581075f9..ca605095194e 100644
> --- a/scripts/sign-file.c
> +++ b/scripts/sign-file.c
> @@ -233,6 +233,7 @@ int main(int argc, char **argv)
> EVP_PKEY *private_key;
> #ifndef USE_PKCS7
> CMS_ContentInfo *cms = NULL;
> + CMS_SignerInfo *signer;
> unsigned int use_keyid = 0;
> #else
> PKCS7 *pkcs7 = NULL;
> @@ -329,13 +330,47 @@ int main(int argc, char **argv)
> !EVP_PKEY_is_a(private_key, "ML-DSA-65") &&
> !EVP_PKEY_is_a(private_key, "ML-DSA-87"))
> flags |= use_signed_attrs;
> + if (EVP_PKEY_is_a(private_key, "RSASSA-PSS"))
> + flags |= CMS_KEY_PARAM;
> + if (EVP_PKEY_is_a(private_key, "RSASSA-PSS")) {
> + EVP_PKEY_CTX *pkctx;
> + char mdname[1024] = {};
> +
> + pkctx = EVP_PKEY_CTX_new(private_key, NULL);
> +
> + ERR(!EVP_PKEY_sign_init(pkctx), "EVP_PKEY_sign_init");
> + ERR(!EVP_PKEY_CTX_set_rsa_padding(pkctx, RSA_PKCS1_PSS_PADDING),
> + "EVP_PKEY_CTX_set_rsa_padding");
> + ERR(!EVP_PKEY_CTX_set_rsa_mgf1_md_name(pkctx, hash_algo, NULL),
> + "EVP_PKEY_CTX_set_rsa_mgf1_md_name");
> +
> + ERR(!EVP_PKEY_CTX_get_rsa_mgf1_md_name(pkctx, mdname, sizeof(mdname)),
> + "EVP_PKEY_CTX_get_rsa_mgf1_md_name");
> + printf("RSASSA-PSS %s\n", mdname);
> + }
>
> /* Load the signature message from the digest buffer. */
> cms = CMS_sign(NULL, NULL, NULL, NULL, flags);
> ERR(!cms, "CMS_sign");
>
> - ERR(!CMS_add1_signer(cms, x509, private_key, digest_algo, flags),
> - "CMS_add1_signer");
> + signer = CMS_add1_signer(cms, x509, private_key, digest_algo, flags);
> + ERR(!signer, "CMS_add1_signer");
> +
> + if (EVP_PKEY_is_a(private_key, "RSASSA-PSS")) {
> + EVP_PKEY_CTX *pkctx;
> + char mdname[1024] = {};
> +
> + pkctx = CMS_SignerInfo_get0_pkey_ctx(signer);
> + ERR(!EVP_PKEY_CTX_set_rsa_padding(pkctx, RSA_PKCS1_PSS_PADDING),
> + "EVP_PKEY_CTX_set_rsa_padding");
> + ERR(!EVP_PKEY_CTX_set_rsa_mgf1_md_name(pkctx, hash_algo, NULL),
> + "EVP_PKEY_CTX_set_rsa_mgf1_md_name");
> +
> + ERR(!EVP_PKEY_CTX_get_rsa_mgf1_md_name(pkctx, mdname, sizeof(mdname)),
> + "EVP_PKEY_CTX_get_rsa_mgf1_md_name");
> + printf("RSASSA-PSS %s\n", mdname);
> + }
> +
> ERR(CMS_final(cms, bm, NULL, flags) != 1,
> "CMS_final");
>
>
Reviewed-by: Ignat Korchagin <ignat@cloudflare.com>
^ permalink raw reply
* Re: [PATCH v11 7/8] pkcs7, x509: Add RSASSA-PSS support
From: Ignat Korchagin @ 2026-01-07 16:36 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: <20260105152145.1801972-8-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 keys and signatures to the PKCS#7 and X.509
> implementations. This requires adding support for algorithm parameters for
> keys and signatures as RSASSA-PSS needs metadata. The ASN.1 encoded data
> is converted into a printable key=value list string and passed to the
> verification code.
>
> Signed-off-by: David Howells <dhowells@redhat.com>
> cc: Lukas Wunner <lukas@wunner.de>
> cc: Ignat Korchagin <ignat@cloudflare.com>
> cc: Herbert Xu <herbert@gondor.apana.org.au>
> cc: keyrings@vger.kernel.org
> cc: linux-crypto@vger.kernel.org
> ---
> crypto/asymmetric_keys/Makefile | 12 +-
> crypto/asymmetric_keys/mgf1_params.asn1 | 12 ++
> crypto/asymmetric_keys/pkcs7.asn1 | 2 +-
> crypto/asymmetric_keys/pkcs7_parser.c | 113 ++++++-----
> crypto/asymmetric_keys/public_key.c | 10 +
> crypto/asymmetric_keys/rsassa_params.asn1 | 25 +++
> crypto/asymmetric_keys/rsassa_parser.c | 233 ++++++++++++++++++++++
> crypto/asymmetric_keys/rsassa_parser.h | 25 +++
> crypto/asymmetric_keys/x509.asn1 | 2 +-
> crypto/asymmetric_keys/x509_cert_parser.c | 96 +++++----
> crypto/asymmetric_keys/x509_parser.h | 33 ++-
> crypto/asymmetric_keys/x509_public_key.c | 28 ++-
> include/linux/oid_registry.h | 2 +
> 13 files changed, 490 insertions(+), 103 deletions(-)
> create mode 100644 crypto/asymmetric_keys/mgf1_params.asn1
> create mode 100644 crypto/asymmetric_keys/rsassa_params.asn1
> create mode 100644 crypto/asymmetric_keys/rsassa_parser.c
> create mode 100644 crypto/asymmetric_keys/rsassa_parser.h
>
> diff --git a/crypto/asymmetric_keys/Makefile b/crypto/asymmetric_keys/Makefile
> index bc65d3b98dcb..c5aed382ee8a 100644
> --- a/crypto/asymmetric_keys/Makefile
> +++ b/crypto/asymmetric_keys/Makefile
> @@ -21,7 +21,11 @@ x509_key_parser-y := \
> x509_akid.asn1.o \
> x509_cert_parser.o \
> x509_loader.o \
> - x509_public_key.o
> + x509_public_key.o \
> + rsassa_params.asn1.o \
> + rsassa_parser.o \
> + mgf1_params.asn1.o
> +
> obj-$(CONFIG_FIPS_SIGNATURE_SELFTEST) += x509_selftest.o
> x509_selftest-y += selftest.o
> x509_selftest-$(CONFIG_FIPS_SIGNATURE_SELFTEST_RSA) += selftest_rsa.o
> @@ -31,8 +35,14 @@ $(obj)/x509_cert_parser.o: \
> $(obj)/x509.asn1.h \
> $(obj)/x509_akid.asn1.h
>
> +$(obj)/rsassa_parser.o: \
> + $(obj)/rsassa_params.asn1.h \
> + $(obj)/mgf1_params.asn1.h
> +
> $(obj)/x509.asn1.o: $(obj)/x509.asn1.c $(obj)/x509.asn1.h
> $(obj)/x509_akid.asn1.o: $(obj)/x509_akid.asn1.c $(obj)/x509_akid.asn1.h
> +$(obj)/rsassa_params.asn1.o: $(obj)/rsassa_params.asn1.c $(obj)/rsassa_params.asn1.h
> +$(obj)/mgf1_params.asn1.o: $(obj)/mgf1_params.asn1.c $(obj)/mgf1_params.asn1.h
>
> #
> # PKCS#8 private key handling
> diff --git a/crypto/asymmetric_keys/mgf1_params.asn1 b/crypto/asymmetric_keys/mgf1_params.asn1
> new file mode 100644
> index 000000000000..c3bc4643e72c
> --- /dev/null
> +++ b/crypto/asymmetric_keys/mgf1_params.asn1
> @@ -0,0 +1,12 @@
> +-- SPDX-License-Identifier: BSD-3-Clause
> +--
> +-- Copyright (C) 2009 IETF Trust and the persons identified as authors
> +-- of the code
> +--
> +--
> +-- https://datatracker.ietf.org/doc/html/rfc4055 Section 6.
> +
> +AlgorithmIdentifier ::= SEQUENCE {
> + algorithm OBJECT IDENTIFIER ({ mgf1_note_OID }),
> + parameters ANY OPTIONAL
> +}
> diff --git a/crypto/asymmetric_keys/pkcs7.asn1 b/crypto/asymmetric_keys/pkcs7.asn1
> index 28e1f4a41c14..03c2248f23bc 100644
> --- a/crypto/asymmetric_keys/pkcs7.asn1
> +++ b/crypto/asymmetric_keys/pkcs7.asn1
> @@ -124,7 +124,7 @@ UnauthenticatedAttribute ::= SEQUENCE {
>
> DigestEncryptionAlgorithmIdentifier ::= SEQUENCE {
> algorithm OBJECT IDENTIFIER ({ pkcs7_note_OID }),
> - parameters ANY OPTIONAL
> + parameters ANY OPTIONAL ({ pkcs7_sig_note_algo_params })
> }
>
> EncryptedDigest ::= OCTET STRING ({ pkcs7_sig_note_signature })
> diff --git a/crypto/asymmetric_keys/pkcs7_parser.c b/crypto/asymmetric_keys/pkcs7_parser.c
> index 90c36fe1b5ed..81996b60c1f1 100644
> --- a/crypto/asymmetric_keys/pkcs7_parser.c
> +++ b/crypto/asymmetric_keys/pkcs7_parser.c
> @@ -14,6 +14,7 @@
> #include <linux/oid_registry.h>
> #include <crypto/public_key.h>
> #include "pkcs7_parser.h"
> +#include "rsassa_parser.h"
> #include "pkcs7.asn1.h"
>
> MODULE_DESCRIPTION("PKCS#7 parser");
> @@ -30,6 +31,8 @@ struct pkcs7_parse_context {
> enum OID last_oid; /* Last OID encountered */
> unsigned x509_index;
> unsigned sinfo_index;
> + unsigned algo_params_size;
> + const void *algo_params;
> const void *raw_serial;
> unsigned raw_serial_size;
> unsigned raw_issuer_size;
> @@ -225,45 +228,29 @@ int pkcs7_sig_note_digest_algo(void *context, size_t hdrlen,
> const void *value, size_t vlen)
> {
> struct pkcs7_parse_context *ctx = context;
> + const char *algo;
>
> - switch (ctx->last_oid) {
> - case OID_sha1:
> - ctx->sinfo->sig->hash_algo = "sha1";
> - break;
> - case OID_sha256:
> - ctx->sinfo->sig->hash_algo = "sha256";
> - break;
> - case OID_sha384:
> - ctx->sinfo->sig->hash_algo = "sha384";
> - break;
> - case OID_sha512:
> - ctx->sinfo->sig->hash_algo = "sha512";
> - break;
> - case OID_sha224:
> - ctx->sinfo->sig->hash_algo = "sha224";
> - break;
> - case OID_sm3:
> - ctx->sinfo->sig->hash_algo = "sm3";
> - break;
> - case OID_gost2012Digest256:
> - ctx->sinfo->sig->hash_algo = "streebog256";
> - break;
> - case OID_gost2012Digest512:
> - ctx->sinfo->sig->hash_algo = "streebog512";
> - break;
> - case OID_sha3_256:
> - ctx->sinfo->sig->hash_algo = "sha3-256";
> - break;
> - case OID_sha3_384:
> - ctx->sinfo->sig->hash_algo = "sha3-384";
> - break;
> - case OID_sha3_512:
> - ctx->sinfo->sig->hash_algo = "sha3-512";
> - break;
> - default:
> - printk("Unsupported digest algo: %u\n", ctx->last_oid);
> + algo = oid_to_hash(ctx->last_oid);
> + if (!algo) {
> + pr_notice("Unsupported digest algo: %u\n", ctx->last_oid);
> return -ENOPKG;
> }
> +
> + ctx->sinfo->sig->hash_algo = algo;
> + return 0;
> +}
> +
> +/*
> + * Note the parameters for the signature.
> + */
> +int pkcs7_sig_note_algo_params(void *context, size_t hdrlen,
> + unsigned char tag,
> + const void *value, size_t vlen)
> +{
> + struct pkcs7_parse_context *ctx = context;
> +
> + ctx->algo_params = value - hdrlen;
> + ctx->algo_params_size = vlen + hdrlen;
> return 0;
> }
>
> @@ -275,12 +262,16 @@ int pkcs7_sig_note_pkey_algo(void *context, size_t hdrlen,
> const void *value, size_t vlen)
> {
> struct pkcs7_parse_context *ctx = context;
> + struct public_key_signature *sig = ctx->sinfo->sig;
> + int err;
>
> switch (ctx->last_oid) {
> case OID_rsaEncryption:
> - ctx->sinfo->sig->pkey_algo = "rsa";
> - ctx->sinfo->sig->encoding = "pkcs1";
> + sig->pkey_algo = "rsa";
> + sig->encoding = "pkcs1";
> break;
> + case OID_id_rsassa_pss:
> + goto rsassa_pss;
I really don't like this. Is it possible to factor this out to a
separate function and just call here? Should the factored function
even be part of the implementation somehow?
> case OID_id_ecdsa_with_sha1:
> case OID_id_ecdsa_with_sha224:
> case OID_id_ecdsa_with_sha256:
> @@ -289,34 +280,52 @@ int pkcs7_sig_note_pkey_algo(void *context, size_t hdrlen,
> case OID_id_ecdsa_with_sha3_256:
> case OID_id_ecdsa_with_sha3_384:
> case OID_id_ecdsa_with_sha3_512:
> - ctx->sinfo->sig->pkey_algo = "ecdsa";
> - ctx->sinfo->sig->encoding = "x962";
> + sig->pkey_algo = "ecdsa";
> + sig->encoding = "x962";
> break;
> case OID_gost2012PKey256:
> case OID_gost2012PKey512:
> - ctx->sinfo->sig->pkey_algo = "ecrdsa";
> - ctx->sinfo->sig->encoding = "raw";
> + sig->pkey_algo = "ecrdsa";
> + sig->encoding = "raw";
> break;
> case OID_id_ml_dsa_44:
> - ctx->sinfo->sig->pkey_algo = "mldsa44";
> - ctx->sinfo->sig->encoding = "raw";
> - ctx->sinfo->sig->algo_does_hash = true;
> + sig->pkey_algo = "mldsa44";
> + sig->encoding = "raw";
> + sig->algo_does_hash = true;
> break;
> case OID_id_ml_dsa_65:
> - ctx->sinfo->sig->pkey_algo = "mldsa65";
> - ctx->sinfo->sig->encoding = "raw";
> - ctx->sinfo->sig->algo_does_hash = true;
> + sig->pkey_algo = "mldsa65";
> + sig->encoding = "raw";
> + sig->algo_does_hash = true;
> break;
> case OID_id_ml_dsa_87:
> - ctx->sinfo->sig->pkey_algo = "mldsa87";
> - ctx->sinfo->sig->encoding = "raw";
> - ctx->sinfo->sig->algo_does_hash = true;
> + sig->pkey_algo = "mldsa87";
> + sig->encoding = "raw";
> + sig->algo_does_hash = true;
> break;
> default:
> - printk("Unsupported pkey algo: %u\n", ctx->last_oid);
> + pr_notice("Unsupported pkey algo: %u\n", ctx->last_oid);
> return -ENOPKG;
> }
> +
> +out:
> + ctx->algo_params = NULL;
> + ctx->algo_params_size = 0;
> return 0;
> +
> +rsassa_pss:
> + if (!ctx->algo_params || !ctx->algo_params_size) {
> + pr_debug("RSASSA-PSS sig algo without parameters\n");
> + return -EBADMSG;
> + }
> +
> + err = rsassa_parse_sig_params(sig, ctx->algo_params, ctx->algo_params_size);
> + if (err < 0)
> + return err;
> +
> + sig->pkey_algo = "rsa";
> + sig->encoding = "emsa-pss";
> + goto out;
> }
>
> /*
> diff --git a/crypto/asymmetric_keys/public_key.c b/crypto/asymmetric_keys/public_key.c
> index 61dc4f626620..13a5616becaa 100644
> --- a/crypto/asymmetric_keys/public_key.c
> +++ b/crypto/asymmetric_keys/public_key.c
> @@ -100,6 +100,16 @@ software_key_determine_akcipher(const struct public_key *pkey,
> }
> return n >= CRYPTO_MAX_ALG_NAME ? -EINVAL : 0;
> }
> + if (strcmp(encoding, "emsa-pss") == 0) {
> + if (op != kernel_pkey_sign &&
> + op != kernel_pkey_verify)
> + return -EINVAL;
> + *sig = true;
> + if (!hash_algo)
> + hash_algo = "none";
> + n = snprintf(alg_name, CRYPTO_MAX_ALG_NAME, "rsassa-pss");
> + return n >= CRYPTO_MAX_ALG_NAME ? -EINVAL : 0;
> + }
> if (strcmp(encoding, "raw") != 0)
> return -EINVAL;
> /*
> diff --git a/crypto/asymmetric_keys/rsassa_params.asn1 b/crypto/asymmetric_keys/rsassa_params.asn1
> new file mode 100644
> index 000000000000..95a4e5f0dcd5
> --- /dev/null
> +++ b/crypto/asymmetric_keys/rsassa_params.asn1
> @@ -0,0 +1,25 @@
> +-- SPDX-License-Identifier: BSD-3-Clause
> +--
> +-- Copyright (C) 2009 IETF Trust and the persons identified as authors
> +-- of the code
> +--
> +--
> +-- https://datatracker.ietf.org/doc/html/rfc4055 Section 6.
> +
> +RSASSA-PSS-params ::= SEQUENCE {
> + hashAlgorithm [0] HashAlgorithm,
> + maskGenAlgorithm [1] MaskGenAlgorithm,
> + saltLength [2] INTEGER ({ rsassa_note_salt_length }),
> + trailerField [3] TrailerField OPTIONAL
> +}
> +
> +TrailerField ::= INTEGER ({ rsassa_note_trailer })
> +-- { trailerFieldBC(1) }
> +
> +HashAlgorithm ::= AlgorithmIdentifier ({ rsassa_note_hash_algo })
> +MaskGenAlgorithm ::= AlgorithmIdentifier ({ rsassa_note_maskgen_algo })
> +
> +AlgorithmIdentifier ::= SEQUENCE {
> + algorithm OBJECT IDENTIFIER ({ rsassa_note_OID }),
> + parameters ANY OPTIONAL ({ rsassa_note_params })
> +}
> diff --git a/crypto/asymmetric_keys/rsassa_parser.c b/crypto/asymmetric_keys/rsassa_parser.c
> new file mode 100644
> index 000000000000..8c598517f785
> --- /dev/null
> +++ b/crypto/asymmetric_keys/rsassa_parser.c
> @@ -0,0 +1,233 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/* RSASSA-PSS ASN.1 parameter parser
> + *
> + * Copyright (C) 2025 Red Hat, Inc. All Rights Reserved.
> + * Written by David Howells (dhowells@redhat.com)
> + */
> +
> +#define pr_fmt(fmt) "RSAPSS: "fmt
> +#include <linux/kernel.h>
> +#include <linux/slab.h>
> +#include <linux/err.h>
> +#include <linux/asn1.h>
> +#include <crypto/hash.h>
> +#include <crypto/hash_info.h>
> +#include <crypto/public_key.h>
> +#include "x509_parser.h"
> +#include "rsassa_parser.h"
> +#include "rsassa_params.asn1.h"
> +#include "mgf1_params.asn1.h"
> +
> +struct rsassa_parse_context {
> + struct rsassa_parameters *rsassa; /* The parsed parameters */
> + unsigned long data; /* Start of data */
> + const void *params; /* Algo parameters */
> + unsigned int params_len; /* Length of algo parameters */
> + enum OID last_oid; /* Last OID encountered */
> + enum OID mgf1_last_oid; /* Last OID encountered in MGF1 */
> +};
> +
> +/*
> + * Parse an RSASSA parameter block.
> + */
> +struct rsassa_parameters *rsassa_params_parse(const void *data, size_t datalen)
> +{
> + struct rsassa_parse_context ctx = {};
> + struct rsassa_parameters *rsassa __free(kfree);
Declare when initialised
> + long ret;
> +
> + rsassa = kzalloc(sizeof(*rsassa), GFP_KERNEL);
> + if (!rsassa)
> + return ERR_PTR(-ENOMEM);
> +
> + ctx.rsassa = rsassa;
> + ctx.data = (unsigned long)data;
> +
> + /* Attempt to decode the parameters */
> + ret = asn1_ber_decoder(&rsassa_params_decoder, &ctx, data, datalen);
> + if (ret < 0) {
> + pr_debug("RSASSA parse failed %ld\n", ret);
> + return ERR_PTR(ret);
> + }
> +
> + return no_free_ptr(rsassa);
> +}
> +
> +/*
> + * Note an OID when we find one for later processing when we know how
> + * to interpret it.
> + */
> +int rsassa_note_OID(void *context, size_t hdrlen, unsigned char tag,
> + const void *value, size_t vlen)
> +{
> + struct rsassa_parse_context *ctx = context;
> +
> + ctx->last_oid = look_up_OID(value, vlen);
> + if (ctx->last_oid == OID__NR) {
> + char buffer[56];
> + sprint_oid(value, vlen, buffer, sizeof(buffer));
> + pr_debug("Unknown OID: %s\n", buffer);
> + }
> + return 0;
> +}
> +
> +/*
> + * Parse trailerField. We only accept trailerFieldBC.
> +*/
> +int rsassa_note_trailer(void *context, size_t hdrlen, unsigned char tag,
> + const void *value, size_t vlen)
> +{
> + if (vlen != 1 || *(u8 *)value != 0x01) {
> + pr_debug("Unknown trailerField\n");
> + return -EINVAL;
> + }
> + return 0;
> +}
> +
> +int rsassa_note_hash_algo(void *context, size_t hdrlen, unsigned char tag,
> + const void *value, size_t vlen)
> +{
> + struct rsassa_parse_context *ctx = context;
> +
> + ctx->rsassa->hash_algo = ctx->last_oid;
> + pr_debug("HASH-ALGO %u %u\n", ctx->rsassa->hash_algo, ctx->params_len);
> + ctx->params = NULL;
> + return 0;
> +}
> +
> +int rsassa_note_maskgen_algo(void *context, size_t hdrlen, unsigned char tag,
> + const void *value, size_t vlen)
> +{
> + struct rsassa_parse_context *ctx = context;
> + int ret;
> +
> + ctx->rsassa->maskgen_algo = ctx->last_oid;
> + pr_debug("MGF-ALGO %u %u\n", ctx->rsassa->maskgen_algo, ctx->params_len);
> +
> + switch (ctx->rsassa->maskgen_algo) {
> + case OID_id_mgf1:
> + if (!vlen) {
> + pr_debug("MGF1 missing parameters\n");
> + return -EBADMSG;
> + }
> +
> + ret = asn1_ber_decoder(&mgf1_params_decoder, ctx,
> + ctx->params, ctx->params_len);
> + if (ret < 0) {
> + pr_debug("MGF1 parse failed %d\n", ret);
> + return ret;
> + }
> + ctx->rsassa->maskgen_hash = ctx->mgf1_last_oid;
> + break;
> +
> + default:
> + pr_debug("Unsupported MaskGenAlgorithm %d\n", ret);
> + return -ENOPKG;
> + }
> +
> + ctx->params = NULL;
> + return 0;
> +}
> +
> +int rsassa_note_salt_length(void *context, size_t hdrlen, unsigned char tag,
> + const void *value, size_t vlen)
> +{
> + struct rsassa_parse_context *ctx = context;
> + u32 salt_len = 0;
> +
> + if (!vlen) {
> + pr_debug("Salt len bad integer\n");
> + return -EBADMSG;
> + }
> + if (vlen > 4) {
> + pr_debug("Salt len too long %zu\n", vlen);
> + return -EBADMSG;
> + }
> + if (((u8 *)value)[0] & 0x80) {
> + pr_debug("Salt len negative\n");
> + return -EBADMSG;
> + }
> +
> + for (size_t i = 0; i < vlen; i++) {
> + salt_len <<= 8;
> + salt_len |= ((u8 *)value)[i];
> + }
> +
> + ctx->rsassa->salt_len = salt_len;
> + pr_debug("Salt-Len %u\n", salt_len);
> + return 0;
> +}
> +
> +/*
> + * Extract arbitrary parameters.
> + */
> +int rsassa_note_params(void *context, size_t hdrlen, unsigned char tag,
> + const void *value, size_t vlen)
> +{
> + struct rsassa_parse_context *ctx = context;
> +
> + ctx->params = value - hdrlen;
> + ctx->params_len = vlen + hdrlen;
> + return 0;
> +}
> +
> +/*
> + * Note an OID when we find one for later processing when we know how to
> + * interpret it.
> + */
> +int mgf1_note_OID(void *context, size_t hdrlen, unsigned char tag,
> + const void *value, size_t vlen)
> +{
> + struct rsassa_parse_context *ctx = context;
> +
> + ctx->mgf1_last_oid = look_up_OID(value, vlen);
> + if (ctx->mgf1_last_oid == OID__NR) {
> + char buffer[56];
> + sprint_oid(value, vlen, buffer, sizeof(buffer));
> + pr_debug("Unknown MGF1 OID: %s\n", buffer);
> + }
> + return 0;
> +}
> +
> +/*
> + * Parse the signature parameter block and generate a suitable info string from
> + * it.
> + */
> +int rsassa_parse_sig_params(struct public_key_signature *sig,
> + const u8 *sig_params, unsigned int sig_params_size)
> +{
> + struct rsassa_parameters *rsassa __free(rsassa_params_free) = NULL;
Declare when initialised or just flip the two declarations
> + const char *mf, *mh;
> +
> + rsassa = rsassa_params_parse(sig_params, sig_params_size);
> + if (IS_ERR(rsassa))
> + return PTR_ERR(rsassa);
> +
> + sig->hash_algo = oid_to_hash(rsassa->hash_algo);
> + if (!sig->hash_algo) {
> + pr_notice("Unsupported hash: %u\n", rsassa->hash_algo);
> + return -ENOPKG;
> + }
> +
> + switch (rsassa->maskgen_algo) {
> + case OID_id_mgf1:
> + mf = "mgf1";
> + break;
> + default:
> + pr_notice("Unsupported maskgen algo: %u\n", rsassa->maskgen_algo);
> + return -ENOPKG;
> + }
> +
> + mh = oid_to_hash(rsassa->maskgen_hash);
> + if (!mh) {
> + pr_notice("Unsupported MGF1 hash: %u\n", rsassa->maskgen_hash);
> + return -ENOPKG;
> + }
> +
> + sig->info = kasprintf(GFP_KERNEL, "sighash=%s pss_mask=%s,%s pss_salt=%u",
> + sig->hash_algo, mf, mh, rsassa->salt_len);
> + if (!sig->info)
> + return -ENOMEM;
> + pr_debug("Info string: %s\n", sig->info);
> + return 0;
> +}
> diff --git a/crypto/asymmetric_keys/rsassa_parser.h b/crypto/asymmetric_keys/rsassa_parser.h
> new file mode 100644
> index 000000000000..b80401a3de8f
> --- /dev/null
> +++ b/crypto/asymmetric_keys/rsassa_parser.h
> @@ -0,0 +1,25 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/* RSASSA-PSS parameter parsing context
> + *
> + * Copyright (C) 2025 Red Hat, Inc. All Rights Reserved.
> + * Written by David Howells (dhowells@redhat.com)
> + */
> +
> +#include <linux/oid_registry.h>
> +
> +struct rsassa_parameters {
> + enum OID hash_algo; /* Hash algorithm identifier */
> + enum OID maskgen_algo; /* Mask gen algorithm identifier */
> + enum OID maskgen_hash; /* Mask gen hash algorithm identifier */
> + u32 salt_len;
> +};
> +
> +struct rsassa_parameters *rsassa_params_parse(const void *data, size_t datalen);
> +int rsassa_parse_sig_params(struct public_key_signature *sig,
> + const u8 *sig_params, unsigned int sig_params_size);
> +
> +static inline void rsassa_params_free(struct rsassa_parameters *params)
> +{
> + kfree(params);
> +}
> +DEFINE_FREE(rsassa_params_free, struct rsassa_parameters*, rsassa_params_free(_T))
> diff --git a/crypto/asymmetric_keys/x509.asn1 b/crypto/asymmetric_keys/x509.asn1
> index feb9573cacce..453b72eba1fe 100644
> --- a/crypto/asymmetric_keys/x509.asn1
> +++ b/crypto/asymmetric_keys/x509.asn1
> @@ -29,7 +29,7 @@ CertificateSerialNumber ::= INTEGER
>
> AlgorithmIdentifier ::= SEQUENCE {
> algorithm OBJECT IDENTIFIER ({ x509_note_OID }),
> - parameters ANY OPTIONAL ({ x509_note_params })
> + parameters ANY OPTIONAL ({ x509_note_algo_id_params })
> }
>
> Name ::= SEQUENCE OF RelativeDistinguishedName
> diff --git a/crypto/asymmetric_keys/x509_cert_parser.c b/crypto/asymmetric_keys/x509_cert_parser.c
> index 5ab5b4e5f1b4..6c431bf181f2 100644
> --- a/crypto/asymmetric_keys/x509_cert_parser.c
> +++ b/crypto/asymmetric_keys/x509_cert_parser.c
> @@ -15,28 +15,7 @@
> #include "x509_parser.h"
> #include "x509.asn1.h"
> #include "x509_akid.asn1.h"
> -
> -struct x509_parse_context {
> - struct x509_certificate *cert; /* Certificate being constructed */
> - unsigned long data; /* Start of data */
> - const void *key; /* Key data */
> - size_t key_size; /* Size of key data */
> - const void *params; /* Key parameters */
> - size_t params_size; /* Size of key parameters */
> - enum OID key_algo; /* Algorithm used by the cert's key */
> - enum OID last_oid; /* Last OID encountered */
> - enum OID sig_algo; /* Algorithm used to sign the cert */
> - u8 o_size; /* Size of organizationName (O) */
> - u8 cn_size; /* Size of commonName (CN) */
> - u8 email_size; /* Size of emailAddress */
> - u16 o_offset; /* Offset of organizationName (O) */
> - u16 cn_offset; /* Offset of commonName (CN) */
> - u16 email_offset; /* Offset of emailAddress */
> - unsigned raw_akid_size;
> - const void *raw_akid; /* Raw authorityKeyId in ASN.1 */
> - const void *akid_raw_issuer; /* Raw directoryName in authorityKeyId */
> - unsigned akid_raw_issuer_size;
> -};
> +#include "rsassa_parser.h"
>
> /*
> * Free an X.509 certificate
> @@ -104,15 +83,15 @@ struct x509_certificate *x509_cert_parse(const void *data, size_t datalen)
>
> cert->pub->keylen = ctx->key_size;
>
> - cert->pub->params = kmemdup(ctx->params, ctx->params_size, GFP_KERNEL);
> + cert->pub->params = kmemdup(ctx->key_params, ctx->key_params_size, GFP_KERNEL);
> if (!cert->pub->params)
> return ERR_PTR(-ENOMEM);
>
> - cert->pub->paramlen = ctx->params_size;
> + cert->pub->paramlen = ctx->key_params_size;
> cert->pub->algo = ctx->key_algo;
>
> /* Grab the signature bits */
> - ret = x509_get_sig_params(cert);
> + ret = x509_get_sig_params(cert, ctx);
> if (ret < 0)
> return ERR_PTR(ret);
>
> @@ -146,7 +125,7 @@ int x509_note_OID(void *context, size_t hdrlen,
>
> ctx->last_oid = look_up_OID(value, vlen);
> if (ctx->last_oid == OID__NR) {
> - char buffer[50];
> + char buffer[56];
I've seen this elsewhere in the crypto code (namely in ECC) but is it
generally a good idea to declare long buffers on the stack?
> sprint_oid(value, vlen, buffer, sizeof(buffer));
> pr_debug("Unknown OID: [%lu] %s\n",
> (unsigned long)value - ctx->data, buffer);
> @@ -179,6 +158,7 @@ int x509_note_sig_algo(void *context, size_t hdrlen, unsigned char tag,
> const void *value, size_t vlen)
> {
> struct x509_parse_context *ctx = context;
> + int err;
>
> pr_debug("PubKey Algo: %u\n", ctx->last_oid);
>
> @@ -210,6 +190,9 @@ int x509_note_sig_algo(void *context, size_t hdrlen, unsigned char tag,
> ctx->cert->sig->hash_algo = "sha1";
> goto ecdsa;
>
> + case OID_id_rsassa_pss:
> + goto rsassa_pss;
> +
> case OID_id_rsassa_pkcs1_v1_5_with_sha3_256:
> ctx->cert->sig->hash_algo = "sha3-256";
> goto rsa_pkcs1;
> @@ -268,6 +251,24 @@ int x509_note_sig_algo(void *context, size_t hdrlen, unsigned char tag,
> goto ml_dsa;
> }
>
> +rsassa_pss:
> + if (!ctx->algo_params || !ctx->algo_params_size) {
> + pr_debug("RSASSA-PSS sig algo without parameters\n");
> + return -EBADMSG;
> + }
> +
> + err = rsassa_parse_sig_params(ctx->cert->sig,
> + ctx->algo_params, ctx->algo_params_size);
> + if (err < 0)
> + return err;
> +
> + ctx->cert->sig->pkey_algo = "rsa";
> + ctx->cert->sig->encoding = "emsa-pss";
> + ctx->sig_algo = ctx->last_oid;
> + ctx->algo_params = NULL;
> + ctx->algo_params_size = 0;
> + return 0;
> +
> rsa_pkcs1:
> ctx->cert->sig->pkey_algo = "rsa";
> ctx->cert->sig->encoding = "pkcs1";
> @@ -324,8 +325,8 @@ int x509_note_signature(void *context, size_t hdrlen,
> vlen--;
> }
>
> - ctx->cert->raw_sig = value;
> - ctx->cert->raw_sig_size = vlen;
> + ctx->sig = value;
> + ctx->sig_size = vlen;
> return 0;
> }
>
> @@ -479,23 +480,16 @@ int x509_note_subject(void *context, size_t hdrlen,
> }
>
> /*
> - * Extract the parameters for the public key
> + * Extract the parameters for an AlgorithmIdentifier.
> */
> -int x509_note_params(void *context, size_t hdrlen,
> - unsigned char tag,
> - const void *value, size_t vlen)
> +int x509_note_algo_id_params(void *context, size_t hdrlen,
> + unsigned char tag,
> + const void *value, size_t vlen)
> {
> struct x509_parse_context *ctx = context;
>
> - /*
> - * AlgorithmIdentifier is used three times in the x509, we should skip
> - * first and ignore third, using second one which is after subject and
> - * before subjectPublicKey.
> - */
> - if (!ctx->cert->raw_subject || ctx->key)
> - return 0;
> - ctx->params = value - hdrlen;
> - ctx->params_size = vlen + hdrlen;
> + ctx->algo_params = value - hdrlen;
> + ctx->algo_params_size = vlen + hdrlen;
> return 0;
> }
>
> @@ -514,12 +508,28 @@ int x509_extract_key_data(void *context, size_t hdrlen,
> case OID_rsaEncryption:
> ctx->cert->pub->pkey_algo = "rsa";
> break;
> + case OID_id_rsassa_pss:
> + /* Parameters are optional for the key itself. */
> + if (ctx->algo_params_size) {
> + struct rsassa_parameters *params __free(rsassa_params_free) = NULL;
> + ctx->key_params = ctx->algo_params;
> + ctx->key_params_size = ctx->algo_params_size;
> + ctx->algo_params = NULL;
> + ctx->algo_params_size = 0;
> +
> + params = rsassa_params_parse(ctx->key_params, ctx->key_params_size);
> + if (IS_ERR(params))
> + return PTR_ERR(params);
> + break;
> + }
> + ctx->cert->pub->pkey_algo = "rsa";
> + break;
> case OID_gost2012PKey256:
> case OID_gost2012PKey512:
> ctx->cert->pub->pkey_algo = "ecrdsa";
> break;
> case OID_id_ecPublicKey:
> - if (parse_OID(ctx->params, ctx->params_size, &oid) != 0)
> + if (parse_OID(ctx->algo_params, ctx->algo_params_size, &oid) != 0)
> return -EBADMSG;
>
> switch (oid) {
> @@ -557,6 +567,8 @@ int x509_extract_key_data(void *context, size_t hdrlen,
> return -EBADMSG;
> ctx->key = value + 1;
> ctx->key_size = vlen - 1;
> + ctx->algo_params = NULL;
> + ctx->algo_params_size = 0;
> return 0;
> }
>
> diff --git a/crypto/asymmetric_keys/x509_parser.h b/crypto/asymmetric_keys/x509_parser.h
> index 0688c222806b..be2e1f6cb9f5 100644
> --- a/crypto/asymmetric_keys/x509_parser.h
> +++ b/crypto/asymmetric_keys/x509_parser.h
> @@ -23,8 +23,6 @@ struct x509_certificate {
> time64_t valid_to;
> const void *tbs; /* Signed data */
> unsigned tbs_size; /* Size of signed data */
> - unsigned raw_sig_size; /* Size of signature */
> - const void *raw_sig; /* Signature data */
> const void *raw_serial; /* Raw serial number in ASN.1 */
> unsigned raw_serial_size;
> unsigned raw_issuer_size;
> @@ -41,6 +39,34 @@ struct x509_certificate {
> bool blacklisted;
> };
>
> +struct x509_parse_context {
> + struct x509_certificate *cert; /* Certificate being constructed */
> + unsigned long data; /* Start of data */
> + const void *key; /* Key data */
> + size_t key_size; /* Size of key data */
> + const void *algo_params; /* AlgorithmIdentifier: parameters */
> + size_t algo_params_size; /* AlgorithmIdentifier: parameters size */
> + const void *key_params; /* Key parameters */
> + size_t key_params_size; /* Size of key parameters */
> + const void *sig_params; /* Signature parameters */
> + unsigned int sig_params_size; /* Size of sig parameters */
> + unsigned int sig_size; /* Size of signature */
> + const void *sig; /* Signature data */
> + enum OID key_algo; /* Algorithm used by the cert's key */
> + enum OID last_oid; /* Last OID encountered */
> + enum OID sig_algo; /* Algorithm used to sign the cert */
> + u8 o_size; /* Size of organizationName (O) */
> + u8 cn_size; /* Size of commonName (CN) */
> + u8 email_size; /* Size of emailAddress */
> + u16 o_offset; /* Offset of organizationName (O) */
> + u16 cn_offset; /* Offset of commonName (CN) */
> + u16 email_offset; /* Offset of emailAddress */
> + unsigned raw_akid_size;
> + const void *raw_akid; /* Raw authorityKeyId in ASN.1 */
> + const void *akid_raw_issuer; /* Raw directoryName in authorityKeyId */
> + unsigned akid_raw_issuer_size;
> +};
> +
> /*
> * x509_cert_parser.c
> */
> @@ -55,5 +81,6 @@ extern int x509_decode_time(time64_t *_t, size_t hdrlen,
> /*
> * x509_public_key.c
> */
> -extern int x509_get_sig_params(struct x509_certificate *cert);
> +extern const char *oid_to_hash(enum OID oid);
> +extern int x509_get_sig_params(struct x509_certificate *cert, struct x509_parse_context *parse);
> extern int x509_check_for_self_signed(struct x509_certificate *cert);
> diff --git a/crypto/asymmetric_keys/x509_public_key.c b/crypto/asymmetric_keys/x509_public_key.c
> index 12e3341e806b..b2f8542accc4 100644
> --- a/crypto/asymmetric_keys/x509_public_key.c
> +++ b/crypto/asymmetric_keys/x509_public_key.c
> @@ -17,11 +17,32 @@
> #include "asymmetric_keys.h"
> #include "x509_parser.h"
>
> +/*
> + * Translate OIDs to hash algorithm names.
> + */
> +const char *oid_to_hash(enum OID oid)
> +{
> + switch (oid) {
> + case OID_sha1: return "sha1";
> + case OID_sha256: return "sha256";
> + case OID_sha384: return "sha384";
> + case OID_sha512: return "sha512";
> + case OID_sha224: return "sha224";
> + case OID_sm3: return "sm3";
> + case OID_gost2012Digest256: return "streebog256";
> + case OID_gost2012Digest512: return "streebog512";
> + case OID_sha3_256: return "sha3-256";
> + case OID_sha3_384: return "sha3-384";
> + case OID_sha3_512: return "sha3-512";
> + default: return NULL;
> + }
> +}
> +
> /*
> * Set up the signature parameters in an X.509 certificate. This involves
> * digesting the signed data and extracting the signature.
> */
> -int x509_get_sig_params(struct x509_certificate *cert)
> +int x509_get_sig_params(struct x509_certificate *cert, struct x509_parse_context *parse)
> {
> struct public_key_signature *sig = cert->sig;
> struct crypto_shash *tfm;
> @@ -31,11 +52,11 @@ int x509_get_sig_params(struct x509_certificate *cert)
>
> pr_devel("==>%s()\n", __func__);
>
> - sig->s = kmemdup(cert->raw_sig, cert->raw_sig_size, GFP_KERNEL);
> + sig->s = kmemdup(parse->sig, parse->sig_size, GFP_KERNEL);
> if (!sig->s)
> return -ENOMEM;
>
> - sig->s_size = cert->raw_sig_size;
> + sig->s_size = parse->sig_size;
>
> /* Allocate the hashing algorithm we're going to need and find out how
> * big the hash operational data will be.
> @@ -43,6 +64,7 @@ int x509_get_sig_params(struct x509_certificate *cert)
> tfm = crypto_alloc_shash(sig->hash_algo, 0, 0);
> if (IS_ERR(tfm)) {
> if (PTR_ERR(tfm) == -ENOENT) {
> + pr_debug("Unsupported hash %s\n", sig->hash_algo);
> cert->unsupported_sig = true;
> return 0;
> }
> diff --git a/include/linux/oid_registry.h b/include/linux/oid_registry.h
> index 30821a6a4f72..d546ea7999b9 100644
> --- a/include/linux/oid_registry.h
> +++ b/include/linux/oid_registry.h
> @@ -31,6 +31,8 @@ enum OID {
> /* PKCS#1 {iso(1) member-body(2) us(840) rsadsi(113549) pkcs(1) pkcs-1(1)} */
> OID_rsaEncryption, /* 1.2.840.113549.1.1.1 */
> OID_sha1WithRSAEncryption, /* 1.2.840.113549.1.1.5 */
> + OID_id_mgf1, /* 1.2.840.113549.1.1.8 */
> + OID_id_rsassa_pss, /* 1.2.840.113549.1.1.10 */
> OID_sha256WithRSAEncryption, /* 1.2.840.113549.1.1.11 */
> OID_sha384WithRSAEncryption, /* 1.2.840.113549.1.1.12 */
> OID_sha512WithRSAEncryption, /* 1.2.840.113549.1.1.13 */
>
Ignat
^ permalink raw reply
* 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