* [PATCH v3] rust: add procedural macro for declaring configfs attributes
From: Malte Wechter @ 2026-06-12 13:29 UTC (permalink / raw)
To: Andreas Hindborg, Breno Leitao, Miguel Ojeda, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Alice Ryhl,
Trevor Gross, Danilo Krummrich, Jens Axboe, Luis Chamberlain,
Petr Pavlu, Daniel Gomez, Sami Tolvanen, Aaron Tomlin
Cc: linux-kernel, rust-for-linux, linux-block, linux-modules,
Malte Wechter
Implement `configfs_attrs!` as a procedural macro using `syn`, this
improves readability and maintainability. Remove the old macro and
replace all uses with the new macro. Add the new macro implementation
file to MAINTAINERS.
Signed-off-by: Malte Wechter <maltewechter@gmail.com>
---
Changes in v3:
- Remove 'make_static_ident' function, make names for static variables simpler
- Move 'parse_ordered_fields' macro from module.rs into helpers
- Use 'parse_ordered_fields' macro for parsing instead of doing it ad-hoc
- Link to v2: https://lore.kernel.org/r/20260603-configfs-syn-v2-1-cb58489c2647@gmail.com
Changes in v2:
- Add a try_parse helper function to macros/helpers.rs
- Fix bug where 'child' configuration gets dropped if trailing comma is missing (sashiko)
- Link to v1: https://lore.kernel.org/r/20260520-configfs-syn-v1-1-6c5b80a9cef2@gmail.com
---
MAINTAINERS | 1 +
drivers/block/rnull/configfs.rs | 2 +-
rust/kernel/configfs.rs | 251 ----------------------------------------
rust/macros/configfs_attrs.rs | 135 +++++++++++++++++++++
rust/macros/helpers.rs | 139 ++++++++++++++++++++++
rust/macros/lib.rs | 85 ++++++++++++++
rust/macros/module.rs | 137 ----------------------
samples/rust/rust_configfs.rs | 2 +-
8 files changed, 362 insertions(+), 390 deletions(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index 2fb1c75afd16..45f7a1ec93b4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6464,6 +6464,7 @@ T: git git://git.kernel.org/pub/scm/linux/kernel/git/a.hindborg/linux.git config
F: fs/configfs/
F: include/linux/configfs.h
F: rust/kernel/configfs.rs
+F: rust/macros/configfs_attrs.rs
F: samples/configfs/
F: samples/rust/rust_configfs.rs
diff --git a/drivers/block/rnull/configfs.rs b/drivers/block/rnull/configfs.rs
index 7c2eb5c0b722..f28ec69d7984 100644
--- a/drivers/block/rnull/configfs.rs
+++ b/drivers/block/rnull/configfs.rs
@@ -4,8 +4,8 @@
use kernel::{
block::mq::gen_disk::{GenDisk, GenDiskBuilder},
configfs::{self, AttributeOperations},
- configfs_attrs,
fmt::{self, Write as _},
+ macros::configfs_attrs,
new_mutex,
page::PAGE_SIZE,
prelude::*,
diff --git a/rust/kernel/configfs.rs b/rust/kernel/configfs.rs
index 2339c6467325..7a91e36677f5 100644
--- a/rust/kernel/configfs.rs
+++ b/rust/kernel/configfs.rs
@@ -791,254 +791,3 @@ fn as_ptr(&self) -> *const bindings::config_item_type {
self.item_type.get()
}
}
-
-/// Define a list of configfs attributes statically.
-///
-/// Invoking the macro in the following manner:
-///
-/// ```ignore
-/// let item_type = configfs_attrs! {
-/// container: configfs::Subsystem<Configuration>,
-/// data: Configuration,
-/// child: Child,
-/// attributes: [
-/// message: 0,
-/// bar: 1,
-/// ],
-/// };
-/// ```
-///
-/// Expands the following output:
-///
-/// ```ignore
-/// let item_type = {
-/// static CONFIGURATION_MESSAGE_ATTR: kernel::configfs::Attribute<
-/// 0,
-/// Configuration,
-/// Configuration,
-/// > = unsafe {
-/// kernel::configfs::Attribute::new({
-/// const S: &str = "message\u{0}";
-/// const C: &kernel::str::CStr = match kernel::str::CStr::from_bytes_with_nul(
-/// S.as_bytes()
-/// ) {
-/// Ok(v) => v,
-/// Err(_) => {
-/// core::panicking::panic_fmt(core::const_format_args!(
-/// "string contains interior NUL"
-/// ));
-/// }
-/// };
-/// C
-/// })
-/// };
-///
-/// static CONFIGURATION_BAR_ATTR: kernel::configfs::Attribute<
-/// 1,
-/// Configuration,
-/// Configuration
-/// > = unsafe {
-/// kernel::configfs::Attribute::new({
-/// const S: &str = "bar\u{0}";
-/// const C: &kernel::str::CStr = match kernel::str::CStr::from_bytes_with_nul(
-/// S.as_bytes()
-/// ) {
-/// Ok(v) => v,
-/// Err(_) => {
-/// core::panicking::panic_fmt(core::const_format_args!(
-/// "string contains interior NUL"
-/// ));
-/// }
-/// };
-/// C
-/// })
-/// };
-///
-/// const N: usize = (1usize + (1usize + 0usize)) + 1usize;
-///
-/// static CONFIGURATION_ATTRS: kernel::configfs::AttributeList<N, Configuration> =
-/// unsafe { kernel::configfs::AttributeList::new() };
-///
-/// {
-/// const N: usize = 0usize;
-/// unsafe { CONFIGURATION_ATTRS.add::<N, 0, _>(&CONFIGURATION_MESSAGE_ATTR) };
-/// }
-///
-/// {
-/// const N: usize = (1usize + 0usize);
-/// unsafe { CONFIGURATION_ATTRS.add::<N, 1, _>(&CONFIGURATION_BAR_ATTR) };
-/// }
-///
-/// static CONFIGURATION_TPE:
-/// kernel::configfs::ItemType<configfs::Subsystem<Configuration> ,Configuration>
-/// = kernel::configfs::ItemType::<
-/// configfs::Subsystem<Configuration>,
-/// Configuration
-/// >::new_with_child_ctor::<N,Child>(
-/// &THIS_MODULE,
-/// &CONFIGURATION_ATTRS
-/// );
-///
-/// &CONFIGURATION_TPE
-/// }
-/// ```
-#[macro_export]
-macro_rules! configfs_attrs {
- (
- container: $container:ty,
- data: $data:ty,
- attributes: [
- $($name:ident: $attr:literal),* $(,)?
- ] $(,)?
- ) => {
- $crate::configfs_attrs!(
- count:
- @container($container),
- @data($data),
- @child(),
- @no_child(x),
- @attrs($($name $attr)*),
- @eat($($name $attr,)*),
- @assign(),
- @cnt(0usize),
- )
- };
- (
- container: $container:ty,
- data: $data:ty,
- child: $child:ty,
- attributes: [
- $($name:ident: $attr:literal),* $(,)?
- ] $(,)?
- ) => {
- $crate::configfs_attrs!(
- count:
- @container($container),
- @data($data),
- @child($child),
- @no_child(),
- @attrs($($name $attr)*),
- @eat($($name $attr,)*),
- @assign(),
- @cnt(0usize),
- )
- };
- (count:
- @container($container:ty),
- @data($data:ty),
- @child($($child:ty)?),
- @no_child($($no_child:ident)?),
- @attrs($($aname:ident $aattr:literal)*),
- @eat($name:ident $attr:literal, $($rname:ident $rattr:literal,)*),
- @assign($($assign:block)*),
- @cnt($cnt:expr),
- ) => {
- $crate::configfs_attrs!(
- count:
- @container($container),
- @data($data),
- @child($($child)?),
- @no_child($($no_child)?),
- @attrs($($aname $aattr)*),
- @eat($($rname $rattr,)*),
- @assign($($assign)* {
- const N: usize = $cnt;
- // The following macro text expands to a call to `Attribute::add`.
-
- // SAFETY: By design of this macro, the name of the variable we
- // invoke the `add` method on below, is not visible outside of
- // the macro expansion. The macro does not operate concurrently
- // on this variable, and thus we have exclusive access to the
- // variable.
- unsafe {
- $crate::macros::paste!(
- [< $data:upper _ATTRS >]
- .add::<N, $attr, _>(&[< $data:upper _ $name:upper _ATTR >])
- )
- };
- }),
- @cnt(1usize + $cnt),
- )
- };
- (count:
- @container($container:ty),
- @data($data:ty),
- @child($($child:ty)?),
- @no_child($($no_child:ident)?),
- @attrs($($aname:ident $aattr:literal)*),
- @eat(),
- @assign($($assign:block)*),
- @cnt($cnt:expr),
- ) =>
- {
- $crate::configfs_attrs!(
- final:
- @container($container),
- @data($data),
- @child($($child)?),
- @no_child($($no_child)?),
- @attrs($($aname $aattr)*),
- @assign($($assign)*),
- @cnt($cnt),
- )
- };
- (final:
- @container($container:ty),
- @data($data:ty),
- @child($($child:ty)?),
- @no_child($($no_child:ident)?),
- @attrs($($name:ident $attr:literal)*),
- @assign($($assign:block)*),
- @cnt($cnt:expr),
- ) =>
- {
- $crate::macros::paste!{
- {
- $(
- // SAFETY: We are expanding `configfs_attrs`.
- static [< $data:upper _ $name:upper _ATTR >]:
- $crate::configfs::Attribute<$attr, $data, $data> =
- unsafe {
- $crate::configfs::Attribute::new(
- $crate::c_str!(::core::stringify!($name)),
- )
- };
- )*
-
-
- // We need space for a null terminator.
- const N: usize = $cnt + 1usize;
-
- // SAFETY: We are expanding `configfs_attrs`.
- static [< $data:upper _ATTRS >]:
- $crate::configfs::AttributeList<N, $data> =
- unsafe { $crate::configfs::AttributeList::new() };
-
- $($assign)*
-
- $(
- const [<$no_child:upper>]: bool = true;
-
- static [< $data:upper _TPE >] : $crate::configfs::ItemType<$container, $data> =
- $crate::configfs::ItemType::<$container, $data>::new::<N>(
- &THIS_MODULE, &[<$ data:upper _ATTRS >]
- );
- )?
-
- $(
- static [< $data:upper _TPE >]:
- $crate::configfs::ItemType<$container, $data> =
- $crate::configfs::ItemType::<$container, $data>::
- new_with_child_ctor::<N, $child>(
- &THIS_MODULE, &[<$ data:upper _ATTRS >]
- );
- )?
-
- & [< $data:upper _TPE >]
- }
- }
- };
-
-}
-
-pub use crate::configfs_attrs;
diff --git a/rust/macros/configfs_attrs.rs b/rust/macros/configfs_attrs.rs
new file mode 100644
index 000000000000..81037bc38188
--- /dev/null
+++ b/rust/macros/configfs_attrs.rs
@@ -0,0 +1,135 @@
+// SPDX-License-Identifier: GPL-2.0
+
+use quote::{
+ format_ident,
+ quote, //
+};
+
+use syn::{
+ bracketed,
+ ext::IdentExt,
+ parse::{
+ Parse,
+ ParseStream, //
+ },
+ punctuated::Punctuated,
+ spanned::Spanned,
+ Error,
+ Ident,
+ LitInt,
+ Token,
+ Type, //
+};
+
+use crate::helpers::parse_ordered_fields;
+
+pub(crate) struct ConfigfsAttrs {
+ container: Type,
+ data: Type,
+ child: Option<Type>,
+ attributes: Vec<(Ident, LitInt)>,
+}
+
+fn parse_attribute_field(stream: ParseStream<'_>) -> syn::Result<(Ident, LitInt)> {
+ let id = stream.parse::<syn::Ident>()?;
+ let _colon = stream.parse::<Token![:]>()?;
+ let v = stream.parse::<LitInt>()?;
+ Ok((id, v))
+}
+
+fn parse_attributes(stream: ParseStream<'_>) -> syn::Result<Vec<(Ident, LitInt)>> {
+ let attr_stream;
+ let _bracket = bracketed!(attr_stream in stream);
+ let attributes = Punctuated::<(Ident, LitInt), Token![,]>::parse_terminated_with(
+ &attr_stream,
+ parse_attribute_field,
+ )?;
+ Ok(attributes.into_iter().collect::<Vec<_>>())
+}
+
+impl Parse for ConfigfsAttrs {
+ fn parse(input: ParseStream<'_>) -> syn::Result<Self> {
+ parse_ordered_fields!(
+ from input;
+ container [required] => (input.parse::<Type>())?,
+ data [required] => (input.parse::<Type>())?,
+ child => (input.parse::<Type>())?,
+ attributes [required] => parse_attributes(input)?,
+ );
+
+ Ok(ConfigfsAttrs {
+ container,
+ data,
+ child,
+ attributes,
+ })
+ }
+}
+
+pub(crate) fn configfs_attrs(cfs_attrs: ConfigfsAttrs) -> proc_macro2::TokenStream {
+ let (container_ty, data_ty) = (&cfs_attrs.container, &cfs_attrs.data);
+
+ let data_tp_ident = Ident::new("DATA_TPE", cfs_attrs.data.span());
+ let data_attr_ident = Ident::new("DATA_ATTR_LIST", cfs_attrs.data.span());
+
+ let n = cfs_attrs.attributes.len() + 1;
+
+ let attr_list = quote! {
+ static #data_attr_ident: kernel::configfs::AttributeList<#n, #data_ty> =
+ // SAFETY: We are expanding `configfs_attrs`.
+ unsafe { kernel::configfs::AttributeList::new() };
+ };
+
+ let mut attrs = Vec::new();
+ for (attr_idx, (name, id)) in cfs_attrs.attributes.iter().enumerate() {
+ let name_with_attr = format_ident!("{}_ATTR_{}", name.to_string().to_uppercase(), attr_idx);
+
+ let id: u64 = match id.base10_parse::<u64>() {
+ Ok(v) => v,
+ Err(_) => {
+ return syn::Error::new(id.span(), "Could not parse attribute ID as a u64")
+ .to_compile_error();
+ }
+ };
+
+ attrs.push(quote! {
+ static #name_with_attr: kernel::configfs::Attribute<#id, #data_ty, #data_ty> =
+ // SAFETY: We are expanding `configfs_attrs`.
+ unsafe {
+ kernel::configfs::Attribute::new(kernel::c_str!(::core::stringify!(#name)))
+ };
+
+ // SAFETY: By design of this macro, the name of the variable we
+ // invoke the `add` method on below, is not visible outside of
+ // the macro expansion. The macro does not operate concurrently
+ // on this variable, and thus we have exclusive access to the
+ // variable.
+ unsafe { #data_attr_ident.add::<#attr_idx, #id, _>(&#name_with_attr) }
+ });
+ }
+
+ let has_child_code = if let Some(child) = cfs_attrs.child {
+ quote! { new_with_child_ctor::<#n, #child>}
+ } else {
+ quote! { new::<#n> }
+ };
+
+ let data_type = quote! {
+ {
+ static #data_tp_ident:
+ kernel::configfs::ItemType<#container_ty, #data_ty> =
+ kernel::configfs::ItemType::<#container_ty, #data_ty>::#has_child_code(
+ &THIS_MODULE, &#data_attr_ident
+ );
+ &#data_tp_ident
+ }
+ };
+
+ quote! {
+ {
+ #attr_list
+ #(#attrs)*
+ #data_type
+ }
+ }
+}
diff --git a/rust/macros/helpers.rs b/rust/macros/helpers.rs
index d18fbf4daa0a..df524749631a 100644
--- a/rust/macros/helpers.rs
+++ b/rust/macros/helpers.rs
@@ -58,3 +58,142 @@ pub(crate) fn file() -> String {
pub(crate) fn gather_cfg_attrs(attr: &[Attribute]) -> impl Iterator<Item = &Attribute> + '_ {
attr.iter().filter(|a| a.path().is_ident("cfg"))
}
+
+/// Parse fields that are required to use a specific order.
+///
+/// As fields must follow a specific order, we *could* just parse fields one by one by peeking.
+/// However the error message generated when implementing that way is not very friendly.
+///
+/// So instead we parse fields in an arbitrary order, but only enforce the ordering after parsing,
+/// and if the wrong order is used, the proper order is communicated to the user with error message.
+///
+/// Usage looks like this:
+/// ```ignore
+/// parse_ordered_fields! {
+/// from input;
+///
+/// // This will extract "foo: <field>" into a variable named "foo".
+/// // The variable will have type `Option<_>`.
+/// foo => <expression that parses the field>,
+///
+/// // If you need the variable name to be different than the key name.
+/// // This extracts "baz: <field>" into a variable named "bar".
+/// // You might want this if "baz" is a keyword.
+/// baz as bar => <expression that parse the field>,
+///
+/// // You can mark a key as required, and the variable will no longer be `Option`.
+/// // foobar will be of type `Expr` instead of `Option<Expr>`.
+/// foobar [required] => input.parse::<Expr>()?,
+/// }
+/// ```
+macro_rules! parse_ordered_fields {
+ (@gen
+ [$input:expr]
+ [$([$name:ident; $key:ident; $parser:expr])*]
+ [$([$req_name:ident; $req_key:ident])*]
+ ) => {
+ $(let mut $name = None;)*
+
+ const EXPECTED_KEYS: &[&str] = &[$(stringify!($key),)*];
+ const REQUIRED_KEYS: &[&str] = &[$(stringify!($req_key),)*];
+
+ let span = $input.span();
+ let mut seen_keys = Vec::new();
+
+ while !$input.is_empty() {
+ let key = $input.call(Ident::parse_any)?;
+
+ if seen_keys.contains(&key) {
+ Err(Error::new_spanned(
+ &key,
+ format!(r#"duplicated key "{key}". Keys can only be specified once."#),
+ ))?
+ }
+
+ $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:?}."#),
+ ))?
+ }
+ }
+
+ $input.parse::<Token![,]>()?;
+ seen_keys.push(key);
+ }
+
+ for key in REQUIRED_KEYS {
+ if !seen_keys.iter().any(|e| e == key) {
+ Err(Error::new(span, format!(r#"missing required key "{key}""#)))?
+ }
+ }
+
+ let mut ordered_keys: Vec<&str> = Vec::new();
+ for key in EXPECTED_KEYS {
+ if seen_keys.iter().any(|e| e == key) {
+ ordered_keys.push(key);
+ }
+ }
+
+ if seen_keys != ordered_keys {
+ Err(Error::new(
+ span,
+ format!(r#"keys are not ordered as expected. Order them like: {ordered_keys:?}."#),
+ ))?
+ }
+
+ $(let $req_name = $req_name.expect("required field");)*
+ };
+
+ // Handle required fields.
+ (@gen
+ [$input:expr] [$($tok:tt)*] [$($req:tt)*]
+ $key:ident as $name:ident [required] => $parser:expr,
+ $($rest:tt)*
+ ) => {
+ parse_ordered_fields!(
+ @gen [$input] [$($tok)* [$name; $key; $parser]] [$($req)* [$name; $key]] $($rest)*
+ )
+ };
+ (@gen
+ [$input:expr] [$($tok:tt)*] [$($req:tt)*]
+ $name:ident [required] => $parser:expr,
+ $($rest:tt)*
+ ) => {
+ parse_ordered_fields!(
+ @gen [$input] [$($tok)* [$name; $name; $parser]] [$($req)* [$name; $name]] $($rest)*
+ )
+ };
+
+ // Handle optional fields.
+ (@gen
+ [$input:expr] [$($tok:tt)*] [$($req:tt)*]
+ $key:ident as $name:ident => $parser:expr,
+ $($rest:tt)*
+ ) => {
+ parse_ordered_fields!(
+ @gen [$input] [$($tok)* [$name; $key; $parser]] [$($req)*] $($rest)*
+ )
+ };
+ (@gen
+ [$input:expr] [$($tok:tt)*] [$($req:tt)*]
+ $name:ident => $parser:expr,
+ $($rest:tt)*
+ ) => {
+ parse_ordered_fields!(
+ @gen [$input] [$($tok)* [$name; $name; $parser]] [$($req)*] $($rest)*
+ )
+ };
+
+ (from $input:expr; $($tok:tt)*) => {
+ parse_ordered_fields!(@gen [$input] [] [] $($tok)*)
+ }
+}
+
+pub(crate) use parse_ordered_fields;
diff --git a/rust/macros/lib.rs b/rust/macros/lib.rs
index 2cfd59e0f9e7..be04d94d0bc5 100644
--- a/rust/macros/lib.rs
+++ b/rust/macros/lib.rs
@@ -15,6 +15,8 @@
#![cfg_attr(not(CONFIG_RUSTC_HAS_SPAN_FILE), feature(proc_macro_span))]
mod concat_idents;
+#[cfg(CONFIG_CONFIGFS_FS)]
+mod configfs_attrs;
mod export;
mod fmt;
mod helpers;
@@ -489,3 +491,86 @@ pub fn kunit_tests(attr: TokenStream, input: TokenStream) -> TokenStream {
.unwrap_or_else(|e| e.into_compile_error())
.into()
}
+
+/// Define a list of configfs attributes statically.
+///
+/// # Examples
+///
+/// ```ignore
+/// let item_type = configfs_attrs! {
+/// container: configfs::Subsystem<Configuration>,
+/// data: Configuration,
+/// child: Child,
+/// attributes: [
+/// message: 0,
+/// bar: 1,
+/// ],
+/// };
+///```
+///
+/// Expands the following output:
+/// let item_type = {
+/// static DATA_ATTR_LIST: kernel::configfs::AttributeList<
+/// 3usize,
+/// Configuration,
+/// > = unsafe { kernel::configfs::AttributeList::new() };
+/// static MESSAGE_ATTR_0: kernel::configfs::Attribute<
+/// 0u64,
+/// Configuration,
+/// Configuration,
+/// > = unsafe {
+/// kernel::configfs::Attribute::new({
+/// const S: &str = "message\u{0}";
+/// const C: &kernel::str::CStr = match kernel::str::CStr::from_bytes_with_nul(
+/// S.as_bytes(),
+/// ) {
+/// Ok(v) => v,
+/// Err(_) => {
+/// ::core::panicking::panic_fmt(
+/// format_args!("string contains interior NUL"),
+/// );
+/// }
+/// };
+/// C
+/// })
+/// };
+/// unsafe { DATA_ATTR_LIST.add::<0usize, 0u64, _>(&MESSAGE_ATTR_0) }
+/// static BAR_ATTR_1: kernel::configfs::Attribute<
+/// 1u64,
+/// Configuration,
+/// Configuration,
+/// > = unsafe {
+/// kernel::configfs::Attribute::new({
+/// const S: &str = "bar\u{0}";
+/// const C: &kernel::str::CStr = match kernel::str::CStr::from_bytes_with_nul(
+/// S.as_bytes(),
+/// ) {
+/// Ok(v) => v,
+/// Err(_) => {
+/// ::core::panicking::panic_fmt(
+/// format_args!("string contains interior NUL"),
+/// );
+/// }
+/// };
+/// C
+/// })
+/// };
+/// unsafe { DATA_ATTR_LIST.add::<1usize, 1u64, _>(&BAR_ATTR_1) }
+/// {
+/// static DATA_TPE: kernel::configfs::ItemType<
+/// Subsystem<Configuration>,
+/// Configuration,
+/// > = kernel::configfs::ItemType::<
+/// Subsystem<Configuration>,
+/// Configuration,
+/// >::new_with_child_ctor::<3usize, Child>(&THIS_MODULE, &DATA_ATTR_LIST);
+/// &DATA_TPE
+/// }
+/// };
+///
+#[cfg(CONFIG_CONFIGFS_FS)]
+#[proc_macro]
+pub fn configfs_attrs(input: TokenStream) -> TokenStream {
+ configfs_attrs::configfs_attrs(parse_macro_input!(input as configfs_attrs::ConfigfsAttrs))
+ .into()
+}
diff --git a/rust/macros/module.rs b/rust/macros/module.rs
index 06c18e207508..7ff6ad09b1a2 100644
--- a/rust/macros/module.rs
+++ b/rust/macros/module.rs
@@ -196,143 +196,6 @@ fn param_ops_path(param_type: &str) -> Path {
}
}
-/// Parse fields that are required to use a specific order.
-///
-/// As fields must follow a specific order, we *could* just parse fields one by one by peeking.
-/// However the error message generated when implementing that way is not very friendly.
-///
-/// So instead we parse fields in an arbitrary order, but only enforce the ordering after parsing,
-/// and if the wrong order is used, the proper order is communicated to the user with error message.
-///
-/// Usage looks like this:
-/// ```ignore
-/// parse_ordered_fields! {
-/// from input;
-///
-/// // This will extract "foo: <field>" into a variable named "foo".
-/// // The variable will have type `Option<_>`.
-/// foo => <expression that parses the field>,
-///
-/// // If you need the variable name to be different than the key name.
-/// // This extracts "baz: <field>" into a variable named "bar".
-/// // You might want this if "baz" is a keyword.
-/// baz as bar => <expression that parse the field>,
-///
-/// // You can mark a key as required, and the variable will no longer be `Option`.
-/// // foobar will be of type `Expr` instead of `Option<Expr>`.
-/// foobar [required] => input.parse::<Expr>()?,
-/// }
-/// ```
-macro_rules! parse_ordered_fields {
- (@gen
- [$input:expr]
- [$([$name:ident; $key:ident; $parser:expr])*]
- [$([$req_name:ident; $req_key:ident])*]
- ) => {
- $(let mut $name = None;)*
-
- const EXPECTED_KEYS: &[&str] = &[$(stringify!($key),)*];
- const REQUIRED_KEYS: &[&str] = &[$(stringify!($req_key),)*];
-
- let span = $input.span();
- let mut seen_keys = Vec::new();
-
- while !$input.is_empty() {
- let key = $input.call(Ident::parse_any)?;
-
- if seen_keys.contains(&key) {
- Err(Error::new_spanned(
- &key,
- format!(r#"duplicated key "{key}". Keys can only be specified once."#),
- ))?
- }
-
- $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:?}."#),
- ))?
- }
- }
-
- $input.parse::<Token![,]>()?;
- seen_keys.push(key);
- }
-
- for key in REQUIRED_KEYS {
- if !seen_keys.iter().any(|e| e == key) {
- Err(Error::new(span, format!(r#"missing required key "{key}""#)))?
- }
- }
-
- let mut ordered_keys: Vec<&str> = Vec::new();
- for key in EXPECTED_KEYS {
- if seen_keys.iter().any(|e| e == key) {
- ordered_keys.push(key);
- }
- }
-
- if seen_keys != ordered_keys {
- Err(Error::new(
- span,
- format!(r#"keys are not ordered as expected. Order them like: {ordered_keys:?}."#),
- ))?
- }
-
- $(let $req_name = $req_name.expect("required field");)*
- };
-
- // Handle required fields.
- (@gen
- [$input:expr] [$($tok:tt)*] [$($req:tt)*]
- $key:ident as $name:ident [required] => $parser:expr,
- $($rest:tt)*
- ) => {
- parse_ordered_fields!(
- @gen [$input] [$($tok)* [$name; $key; $parser]] [$($req)* [$name; $key]] $($rest)*
- )
- };
- (@gen
- [$input:expr] [$($tok:tt)*] [$($req:tt)*]
- $name:ident [required] => $parser:expr,
- $($rest:tt)*
- ) => {
- parse_ordered_fields!(
- @gen [$input] [$($tok)* [$name; $name; $parser]] [$($req)* [$name; $name]] $($rest)*
- )
- };
-
- // Handle optional fields.
- (@gen
- [$input:expr] [$($tok:tt)*] [$($req:tt)*]
- $key:ident as $name:ident => $parser:expr,
- $($rest:tt)*
- ) => {
- parse_ordered_fields!(
- @gen [$input] [$($tok)* [$name; $key; $parser]] [$($req)*] $($rest)*
- )
- };
- (@gen
- [$input:expr] [$($tok:tt)*] [$($req:tt)*]
- $name:ident => $parser:expr,
- $($rest:tt)*
- ) => {
- parse_ordered_fields!(
- @gen [$input] [$($tok)* [$name; $name; $parser]] [$($req)*] $($rest)*
- )
- };
-
- (from $input:expr; $($tok:tt)*) => {
- parse_ordered_fields!(@gen [$input] [] [] $($tok)*)
- }
-}
-
struct Parameter {
name: Ident,
ptype: Ident,
diff --git a/samples/rust/rust_configfs.rs b/samples/rust/rust_configfs.rs
index a1bd9db6010d..876462f7789d 100644
--- a/samples/rust/rust_configfs.rs
+++ b/samples/rust/rust_configfs.rs
@@ -4,7 +4,7 @@
use kernel::alloc::flags;
use kernel::configfs;
-use kernel::configfs::configfs_attrs;
+use kernel::macros::configfs_attrs;
use kernel::new_mutex;
use kernel::page::PAGE_SIZE;
use kernel::prelude::*;
---
base-commit: 254f49634ee16a731174d2ae34bc50bd5f45e731
change-id: 20260417-configfs-syn-191e07130027
Best regards,
--
Malte Wechter <maltewechter@gmail.com>
^ permalink raw reply related
* Re: [PATCH 0/2] module: restrict module auto-loading to privileged users
From: Michal Gorlas @ 2026-06-12 12:41 UTC (permalink / raw)
To: Kees Cook, Sami Tolvanen
Cc: Michal Gorlas, Jonathan Corbet, Shuah Khan, Luis Chamberlain,
Petr Pavlu, Daniel Gomez, Aaron Tomlin, linux-doc, linux-kernel,
linux-modules
In-Reply-To: <202606101317.D23383F465@keescook>
On Wed Jun 10, 2026 at 10:23 PM CEST, Kees Cook wrote:
> On Fri, Jun 05, 2026 at 06:36:46PM +0000, Sami Tolvanen wrote:
>> On Fri, May 15, 2026 at 07:20:18PM +0200, Michal Gorlas wrote:
>> > Add option to restrict the module auto-loading to CAP_SYS_ADMIN.
>> > This is heavily inspired by CONFIG_GRKERNSEC_MODHARDEN of the latest
>> > available Grsecurity patches [1]. Instead of checking whether the
>> > callers' UID is 0, check whether the calling process has CAP_SYS_ADMIN.
>> > The reasoning here is that many modules are autoloaded by systemd
>> > services which are running as privileged users, but do not have UID 0.
>> > While systemd-udevd runs as root, systemd-network (which often
>> > auto-loads a module) for example runs as system user (UID range 6 to
>> > 999).
>> >
>> > When enabled, reduces attack surface where unprivileged users can trigger
>> > vulnerable module to be auto-loaded, to then exploit it. Recent LPEs
>> > (CopyFail [3], DirtyFrag [4]) for example, would have been mitigated
>> > with this option enabled as long as the vulnerable modules are not built-in
>> > (or already loaded at the point of running the exploit).
>>
>> This sounds potentially useful as an optional feature. Kees, you've
>> looked at grsec features in the past, do you have any thoughts about
>> this?
>
> This doesn't really look like GRKERNSEC_MODHARDEN to me? In that
> feature, the credentials of the usermode helper are passed down so that
> udev or whatever can examine them and make choices (instead of seeing
> the uid-0 usermode helper credentials).
It is based on a part of GRKERNSEC_MODHARDEN policy check in
____request_module in [1]. By no means it reasembles the full
feature. Very similar check was proposed for linux-hardened
tree few years back (with the difference of checking for
CAP_SYS_MODULE) [2].
>
> This looks like it is just doing a request-time policy check, but that's
> already covered by the security_kernel_module_request() call immediately
> before the proposed module_autoload_restrict check.
>
> Also note that module loading is _already_ controlled by CAP_SYS_MODULE,
> not uid 0 nor CAP_SYS_ADMIN.
>
> Sashiko has similar feedback, and some other notes too:
> https://sashiko.dev/#/patchset/20260515-autoload_restrict-v1-0-40b7c03ddd04%409elements.com
My understanding is that CAP_SYS_MODULE is for processes that are
using load/unload directly (i.e. by doing init_module/delete_module
syscall), and the kmod's__request_module, is a user mode call
(at least that's what the comment in __request_module suggests),
so CAP_SYS_MODULE does not have to be set for processes that are
just using __request_module. One example of this is systemd-networkd
(there are probably more but that's one that I tested), i.e. it will
trigger the module autoload even though its not given CAP_SYS_MODULE.
Please correct me if I am wrong here.
>
> I'm not clear what problem this patch is trying to solve?
To have an option to completely disable module auto-loading for
non-root in general. By root here I also think of system users so
UID 1-999 [3] (had typo in cover for the patch, sorry for that).
Not sure if this is a best approach, it could be also implemented
as small LSM hook on security_kernel_module_request() (just a thought
after you mentioned it). Either way, the idea is to limit the
auto-loading, not direct loading.
[1] - https://github.com/minipli/linux-grsec/blob/v4.9.24-grsec/kernel/kmod.c#L153
[2] - https://github.com/anthraxx/linux-hardened/pull/23
[3] - https://systemd.io/UIDS-GIDS/
Best,
Michal
^ permalink raw reply
* Re: [PATCH 04/11] treewide: Convert struct kernel_param_ops initializers to DEFINE_KERNEL_PARAM_OPS
From: jim.cromie @ 2026-06-10 21:06 UTC (permalink / raw)
To: Petr Pavlu
Cc: Kees Cook, Luis Chamberlain, Pengpeng Hou, Richard Weinberger,
Anton Ivanov, Johannes Berg, Rafael J. Wysocki, Len Brown,
Corey Minyard, Gabriel Somlo, Michael S. Tsirkin, Jani Nikula,
Joonas Lahtinen, Rodrigo Vivi, Tvrtko Ursulin, David Airlie,
Simona Vetter, Bart Van Assche, Jason Gunthorpe, Leon Romanovsky,
Laurent Pinchart, Hans de Goede, Mauro Carvalho Chehab,
Bjorn Helgaas, Hannes Reinecke, James E.J. Bottomley,
Martin K. Petersen, Daniel Lezcano, Zhang Rui, Lukasz Luba,
Greg Kroah-Hartman, Jiri Slaby, Alan Stern, Jason Wang, Xuan Zhuo,
Eugenio Pérez, Jason Baron, Tiwei Bie, Benjamin Berg,
Ilpo Järvinen, David E. Box, Maciej W. Rozycki,
Srinivas Pandruvada, Peter Zijlstra, Heiko Carstens,
Vasily Gorbik, Sean Christopherson, Paolo Bonzini,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Vinod Koul, Frank Li, Daniel Gomez, Sami Tolvanen,
Aaron Tomlin, Alexander Potapenko, Marco Elver, Dmitry Vyukov,
Andrew Morton, John Johansen, Paul Moore, James Morris,
Serge E. Hallyn, Andy Shevchenko, Georgia Garcia, kvm, dmaengine,
linux-modules, kasan-dev, linux-mm, apparmor,
linux-security-module, linux-um, linux-acpi, openipmi-developer,
qemu-devel, intel-gfx, dri-devel, linux-rdma, linux-media,
linux-pci, linux-scsi, linux-pm, linuxppc-dev, linux-serial,
linux-usb, usb-storage, virtualization, linux-kernel, linux-arch,
netdev, linux-fsdevel, linux-hardening
In-Reply-To: <da358ae1-91b4-4a16-ac76-ffab99c230b9@suse.com>
On Mon, May 25, 2026 at 7:35 AM Petr Pavlu <petr.pavlu@suse.com> wrote:
>
> On 5/21/26 3:33 PM, Kees Cook wrote:
> > Using Coccinelle, rewrite every struct kernel_param_ops initializer that
> > sets .get into a DEFINE_KERNEL_PARAM_OPS-family macro invocation,
> > for example:
> >
> > @@
> > declarer name DEFINE_KERNEL_PARAM_OPS;
> > identifier OPS;
> > expression SET, GET;
> > @@
> > - const struct kernel_param_ops OPS = {
> > - .set = SET,
> > - .get = GET,
> > - };
> > + DEFINE_KERNEL_PARAM_OPS(OPS, SET, GET);
> >
> > Using the macro for initialization means future changes can manipulate
> > the struct layout and callback prototypes without having to change every
> > initializer.
>
> Nit: For consistency, I suggest also converting the few remaining
> kernel_param_ops instances that specify only .set and no .get, such as
> simdisk_param_ops_filename.
>
> --
> Thanks,
> Petr
for the dynamic-debug changes
Reviewed-by: Jim Cromie <jim.cromie@gmail.com>
^ permalink raw reply
* Re: [PATCH 0/2] module: restrict module auto-loading to privileged users
From: Kees Cook @ 2026-06-10 20:23 UTC (permalink / raw)
To: Sami Tolvanen
Cc: Michal Gorlas, Jonathan Corbet, Shuah Khan, Luis Chamberlain,
Petr Pavlu, Daniel Gomez, Aaron Tomlin, linux-doc, linux-kernel,
linux-modules
In-Reply-To: <20260605183646.GC2939956@google.com>
On Fri, Jun 05, 2026 at 06:36:46PM +0000, Sami Tolvanen wrote:
> On Fri, May 15, 2026 at 07:20:18PM +0200, Michal Gorlas wrote:
> > Add option to restrict the module auto-loading to CAP_SYS_ADMIN.
> > This is heavily inspired by CONFIG_GRKERNSEC_MODHARDEN of the latest
> > available Grsecurity patches [1]. Instead of checking whether the
> > callers' UID is 0, check whether the calling process has CAP_SYS_ADMIN.
> > The reasoning here is that many modules are autoloaded by systemd
> > services which are running as privileged users, but do not have UID 0.
> > While systemd-udevd runs as root, systemd-network (which often
> > auto-loads a module) for example runs as system user (UID range 6 to
> > 999).
> >
> > When enabled, reduces attack surface where unprivileged users can trigger
> > vulnerable module to be auto-loaded, to then exploit it. Recent LPEs
> > (CopyFail [3], DirtyFrag [4]) for example, would have been mitigated
> > with this option enabled as long as the vulnerable modules are not built-in
> > (or already loaded at the point of running the exploit).
>
> This sounds potentially useful as an optional feature. Kees, you've
> looked at grsec features in the past, do you have any thoughts about
> this?
This doesn't really look like GRKERNSEC_MODHARDEN to me? In that
feature, the credentials of the usermode helper are passed down so that
udev or whatever can examine them and make choices (instead of seeing
the uid-0 usermode helper credentials).
This looks like it is just doing a request-time policy check, but that's
already covered by the security_kernel_module_request() call immediately
before the proposed module_autoload_restrict check.
Also note that module loading is _already_ controlled by CAP_SYS_MODULE,
not uid 0 nor CAP_SYS_ADMIN.
Sashiko has similar feedback, and some other notes too:
https://sashiko.dev/#/patchset/20260515-autoload_restrict-v1-0-40b7c03ddd04%409elements.com
I'm not clear what problem this patch is trying to solve?
-Kees
--
Kees Cook
^ permalink raw reply
* Re: [PATCH v2] module: use strscpy() to copy module names in stats and dup tracking
From: Petr Pavlu @ 2026-06-09 14:50 UTC (permalink / raw)
To: Naveen Kumar Chaudhary
Cc: mcgrof, da.gomez, samitolvanen, atomlin, linux-modules
In-Reply-To: <slebbzn7p6c4ybokolvey2sgvrooit66zjewwggqac2755ujgp@ptanmtdclmw5>
On 6/4/26 7:45 PM, Naveen Kumar Chaudhary wrote:
> Both try_add_failed_module() and kmod_dup_request_exists_wait() use
> memcpy() with strlen() to copy module names into fixed-size
> char[MODULE_NAME_LEN] buffers. Neither performs a bounds check on the
> copy. Current callers always pass names originating from
> mod->name (itself char[MODULE_NAME_LEN]), so this is not exploitable
> today. However both functions accept a plain const char * with no
> documented length contract, making them latent buffer overflows if a
> future caller passes a longer string.
>
> Replace memcpy() with strscpy() in both sites, which bounds the copy
> to MODULE_NAME_LEN and always NUL-terminates.
>
> Signed-off-by: Naveen Kumar Chaudhary <naveen.osdev@gmail.com>
Reviewed-by: Petr Pavlu <petr.pavlu@suse.com>
--
Thanks,
Petr
^ permalink raw reply
* Re: [PATCH v5 0/4] Enable sysfs module symlink for more built-in drivers
From: Danilo Krummrich @ 2026-06-09 10:29 UTC (permalink / raw)
To: Suzuki K Poulose
Cc: Shashank Balaji, James Clark, Alexander Shishkin,
Greg Kroah-Hartman, Rafael J . Wysocki, Miguel Ojeda, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Alice Ryhl, Trevor Gross, Jonathan Corbet, Shuah Khan,
Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
Aaron Tomlin, Mike Leach, Leo Yan, Thierry Reding,
Jonathan Hunter, Rahul Bukte, linux-kernel, coresight,
linux-arm-kernel, driver-core, rust-for-linux, linux-doc,
Daniel Palmer, Tim Bird, linux-modules, linux-tegra, Sumit Gupta
In-Reply-To: <1c8e441a-6b33-465a-88f9-9552f346ae18@arm.com>
On Tue Jun 9, 2026 at 11:08 AM CEST, Suzuki K Poulose wrote:
> On 08/06/2026 23:24, Danilo Krummrich wrote:
>> On Mon, 18 May 2026 19:19:56 +0900, Shashank Balaji wrote:
>>> [PATCH v5 0/4] Enable sysfs module symlink for more built-in drivers
>>
>> Applied, thanks!
>>
>> Branch: driver-core-testing
>> Tree: git://git.kernel.org/pub/scm/linux/kernel/git/driver-core/driver-core.git
>>
>> [1/4] soc/tegra: cbb: Move driver registration from pure_initcall to core_initcall
>> commit: cd6e95e7ab29
>> [2/4] kernel: param: initialize module_kset in a pure_initcall
>> commit: c82dfce47833
>> [3/4] coresight: pass THIS_MODULE implicitly through a macro
>> commit: efc22b3f89a3
>> [4/4] driver core: platform: set mod_name in driver registration
>> commit: a7a7dc5c46a0
>>
>> The patches will appear in the next linux-next integration (typically within 24
>> hours on weekdays).
>>
>> The patches are in the driver-core-testing branch and will be promoted to
>> driver-core-next after validation.
>
> Apologies, I missed your emails. I am fine with those, happy to fixup
> anything if the linux-next screams.
Thanks for confirming! I did a test merge with linux-next and an allmodconfig
arm64 build before picking it up, so it should be fine.
Thanks,
Danilo
^ permalink raw reply
* Re: [PATCH 2/2] module: restrict autoload to CAP_SYS_ADMIN if CONFIG_MODULE_RESTRICT_AUTOLOAD
From: Michal Gorlas @ 2026-06-09 10:19 UTC (permalink / raw)
To: Sami Tolvanen
Cc: Jonathan Corbet, Shuah Khan, Luis Chamberlain, Petr Pavlu,
Daniel Gomez, Aaron Tomlin, linux-doc, linux-kernel,
linux-modules
In-Reply-To: <20260605183002.GB2939956@google.com>
On Fri Jun 5, 2026 at 8:30 PM CEST, Sami Tolvanen wrote:
> On Fri, May 15, 2026 at 07:20:20PM +0200, Michal Gorlas wrote:
>> Restrict module auto-loading to CAP_SYS_ADMIN if
>> CONFIG_MODULE_RESTRICT_AUTOLOAD is enabled, cmdline parameter
>> modrestrict=true, or kernel.modrestrict=1 is set with sysctl.
>>
>> Signed-off-by: Michal Gorlas <michal.gorlas@9elements.com>
>> ---
>> kernel/module/internal.h | 1 +
>> kernel/module/kmod.c | 5 +++++
>> kernel/module/main.c | 11 +++++++++++
>> 3 files changed, 17 insertions(+)
>>
>> diff --git a/kernel/module/internal.h b/kernel/module/internal.h
>> index 061161cc79d9..496d8703f0c6 100644
>> --- a/kernel/module/internal.h
>> +++ b/kernel/module/internal.h
>> @@ -46,6 +46,7 @@ struct kernel_symbol {
>>
>> extern struct mutex module_mutex;
>> extern struct list_head modules;
>> +extern bool module_autoload_restrict;
>>
>> extern const struct module_attribute *const modinfo_attrs[];
>> extern const size_t modinfo_attrs_count;
>> diff --git a/kernel/module/kmod.c b/kernel/module/kmod.c
>> index a25dccdf7aa7..58b28c23f571 100644
>> --- a/kernel/module/kmod.c
>> +++ b/kernel/module/kmod.c
>> @@ -156,6 +156,11 @@ int __request_module(bool wait, const char *fmt, ...)
>> if (ret)
>> return ret;
>>
>> + if (module_autoload_restrict && !capable(CAP_SYS_ADMIN)) {
>> + pr_alert("denied attempt to auto-load module %s\n", module_name);
>
> Is pr_alert appropriate here or can this be a warning? Also, use the _ratelimited
> variant like the pre-existing warning in this function.
pr_alert was here in the grsec version (thus I assumed it makes sense
here), but agree, pr_warn_ratelimited makes more sense.
Best,
Michal
^ permalink raw reply
* Re: [PATCH 1/2] module: add CONFIG_MODULE_RESTRICT_AUTOLOAD
From: Michal Gorlas @ 2026-06-09 10:07 UTC (permalink / raw)
To: Sami Tolvanen
Cc: Jonathan Corbet, Shuah Khan, Luis Chamberlain, Petr Pavlu,
Daniel Gomez, Aaron Tomlin, linux-doc, linux-kernel,
linux-modules
In-Reply-To: <20260605182517.GA2939956@google.com>
On Fri Jun 5, 2026 at 8:25 PM CEST, Sami Tolvanen wrote:
> On Fri, May 15, 2026 at 07:20:19PM +0200, Michal Gorlas wrote:
>> Add CONFIG_MODULE_RESTRICT_AUTOLOAD and modrestrict parameter
>> documentation.
>>
>> Signed-off-by: Michal Gorlas <michal.gorlas@9elements.com>
>> ---
>> Documentation/admin-guide/kernel-parameters.txt | 5 +++++
>> kernel/module/Kconfig | 15 +++++++++++++++
>> 2 files changed, 20 insertions(+)
>>
>> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
>> index 03a550630644..1013104f0943 100644
>> --- a/Documentation/admin-guide/kernel-parameters.txt
>> +++ b/Documentation/admin-guide/kernel-parameters.txt
>> @@ -4185,6 +4185,11 @@ Kernel parameters
>> For details see:
>> Documentation/admin-guide/hw-vuln/processor_mmio_stale_data.rst
>>
>> + modrestrict=<bool>
>> + Control the restriction of module auto-loading to
>> + CAP_SYS_ADMIN. If no <bool> value is specified, this
>> + is set to the value of CONFIG_MODULE_RESTRICT_AUTOLOAD.
>
> Doesn't this default to true if no bool value is specified? It only uses
> the config if modrestrict is not passed to the kernel at all.
Right. Will adjust the description here.
>
>> <module>.async_probe[=<bool>] [KNL]
>> If no <bool> value is specified or if the value
>> specified is not a valid <bool>, enable asynchronous
>> diff --git a/kernel/module/Kconfig b/kernel/module/Kconfig
>> index 43b1bb01fd27..c9e01bb848c0 100644
>> --- a/kernel/module/Kconfig
>> +++ b/kernel/module/Kconfig
>> @@ -337,6 +337,21 @@ config MODULE_SIG_HASH
>>
>> endif # MODULE_SIG || IMA_APPRAISE_MODSIG
>>
>> +config MODULE_RESTRICT_AUTOLOAD
>> + bool "Restrict module auto-loading to privileged users"
>> + default n
>
> You don't need to specify default n here.
>
> Also, I think you can just squash the two patches. There's no benefit
> in splitting the config/documentation into a separate patch.
Alright, will squash them in v2.
Best,
Michal
^ permalink raw reply
* Re: [PATCH v5 0/4] Enable sysfs module symlink for more built-in drivers
From: Suzuki K Poulose @ 2026-06-09 9:08 UTC (permalink / raw)
To: Danilo Krummrich, Shashank Balaji
Cc: James Clark, Alexander Shishkin, Greg Kroah-Hartman,
Rafael J . Wysocki, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Jonathan Corbet, Shuah Khan, Luis Chamberlain,
Petr Pavlu, Daniel Gomez, Sami Tolvanen, Aaron Tomlin, Mike Leach,
Leo Yan, Thierry Reding, Jonathan Hunter, Rahul Bukte,
linux-kernel, coresight, linux-arm-kernel, driver-core,
rust-for-linux, linux-doc, Daniel Palmer, Tim Bird, linux-modules,
linux-tegra, Sumit Gupta
In-Reply-To: <20260608222448.1353773-1-dakr@kernel.org>
On 08/06/2026 23:24, Danilo Krummrich wrote:
> On Mon, 18 May 2026 19:19:56 +0900, Shashank Balaji wrote:
>> [PATCH v5 0/4] Enable sysfs module symlink for more built-in drivers
>
> Applied, thanks!
>
> Branch: driver-core-testing
> Tree: git://git.kernel.org/pub/scm/linux/kernel/git/driver-core/driver-core.git
>
> [1/4] soc/tegra: cbb: Move driver registration from pure_initcall to core_initcall
> commit: cd6e95e7ab29
> [2/4] kernel: param: initialize module_kset in a pure_initcall
> commit: c82dfce47833
> [3/4] coresight: pass THIS_MODULE implicitly through a macro
> commit: efc22b3f89a3
> [4/4] driver core: platform: set mod_name in driver registration
> commit: a7a7dc5c46a0
>
> The patches will appear in the next linux-next integration (typically within 24
> hours on weekdays).
>
> The patches are in the driver-core-testing branch and will be promoted to
> driver-core-next after validation.
Apologies, I missed your emails. I am fine with those, happy to fixup
anything if the linux-next screams.
Cheers
Suzuki
^ permalink raw reply
* Re: [PATCH v5 0/4] Enable sysfs module symlink for more built-in drivers
From: Danilo Krummrich @ 2026-06-08 22:24 UTC (permalink / raw)
To: Shashank Balaji
Cc: Suzuki K Poulose, James Clark, Alexander Shishkin,
Greg Kroah-Hartman, Rafael J . Wysocki, Danilo Krummrich,
Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Jonathan Corbet, Shuah Khan, Luis Chamberlain, Petr Pavlu,
Daniel Gomez, Sami Tolvanen, Aaron Tomlin, Mike Leach, Leo Yan,
Thierry Reding, Jonathan Hunter, Rahul Bukte, linux-kernel,
coresight, linux-arm-kernel, driver-core, rust-for-linux,
linux-doc, Daniel Palmer, Tim Bird, linux-modules, linux-tegra,
Sumit Gupta
In-Reply-To: <20260518-acpi_mod_name-v5-0-705ccc430885@sony.com>
On Mon, 18 May 2026 19:19:56 +0900, Shashank Balaji wrote:
> [PATCH v5 0/4] Enable sysfs module symlink for more built-in drivers
Applied, thanks!
Branch: driver-core-testing
Tree: git://git.kernel.org/pub/scm/linux/kernel/git/driver-core/driver-core.git
[1/4] soc/tegra: cbb: Move driver registration from pure_initcall to core_initcall
commit: cd6e95e7ab29
[2/4] kernel: param: initialize module_kset in a pure_initcall
commit: c82dfce47833
[3/4] coresight: pass THIS_MODULE implicitly through a macro
commit: efc22b3f89a3
[4/4] driver core: platform: set mod_name in driver registration
commit: a7a7dc5c46a0
The patches will appear in the next linux-next integration (typically within 24
hours on weekdays).
The patches are in the driver-core-testing branch and will be promoted to
driver-core-next after validation.
^ permalink raw reply
* Re: [PATCH v5 0/4] Enable sysfs module symlink for more built-in drivers
From: Danilo Krummrich @ 2026-06-08 22:16 UTC (permalink / raw)
To: Shashank Balaji
Cc: Suzuki K Poulose, James Clark, Alexander Shishkin,
Greg Kroah-Hartman, Rafael J. Wysocki, Miguel Ojeda, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Alice Ryhl, Trevor Gross, Jonathan Corbet, Shuah Khan,
Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
Aaron Tomlin, Mike Leach, Leo Yan, Thierry Reding,
Jonathan Hunter, Rahul Bukte, linux-kernel, coresight,
linux-arm-kernel, driver-core, rust-for-linux, linux-doc,
Daniel Palmer, Tim Bird, linux-modules, linux-tegra, Sumit Gupta
In-Reply-To: <20260518-acpi_mod_name-v5-0-705ccc430885@sony.com>
On Mon May 18, 2026 at 12:19 PM CEST, Shashank Balaji wrote:
> Shashank Balaji (4):
> soc/tegra: cbb: Move driver registration from pure_initcall to core_initcall
> kernel: param: initialize module_kset in a pure_initcall
> coresight: pass THIS_MODULE implicitly through a macro
> driver core: platform: set mod_name in driver registration
Picking this up now, so it can still make it for 7.2-rc1 and get some time in
linux-next.
Suzuki, since I haven't heard back I figured it should be fine to also pick the
coresight change as it is purely mechanic and driver-core motivated, but please
let me know if you have any concerns.
Thanks,
Danilo
^ permalink raw reply
* Re: [PATCH 1/2] xtensa/simdisk: Avoid referring to module::args
From: Max Filippov @ 2026-06-07 20:44 UTC (permalink / raw)
To: Petr Pavlu
Cc: Chris Zankel, Luis Chamberlain, Daniel Gomez, Sami Tolvanen,
Aaron Tomlin, Matthew Wood, linux-modules, linux-kernel
In-Reply-To: <20260604110455.1608038-2-petr.pavlu@suse.com>
On Thu, Jun 4, 2026 at 4:05 AM Petr Pavlu <petr.pavlu@suse.com> wrote:
>
> When simdisk support is built as a loadable module,
> simdisk_param_set_filename() receives a pointer into module::args and
> stores each filename pointer as is.
>
> In preparation for removing module::args, update the simdisk.filename
> parameter code to copy the provided string. This is somewhat complicated by
> the fact that simdisk support can also be built-in, in which case the
> parameters are parsed during early boot before slab is available. In that
> case, the command line itself is preserved for the lifetime of the kernel,
> so continue storing the incoming pointer directly.
>
> Signed-off-by: Petr Pavlu <petr.pavlu@suse.com>
> ---
> arch/xtensa/platforms/iss/simdisk.c | 38 +++++++++++++++++++++++++----
> 1 file changed, 33 insertions(+), 5 deletions(-)
Reviewed-by: Max Filippov <jcmvbkbc@gmail.com>
--
Thanks.
-- Max
^ permalink raw reply
* [PATCH v2] module: procfs: fix signed integer overflow in module_total_size()
From: Naveen Kumar Chaudhary @ 2026-06-07 4:18 UTC (permalink / raw)
To: samitolvanen; +Cc: mcgrof, petr.pavlu, da.gomez, atomlin, linux-modules
In-Reply-To: <20260605205215.GA3054112@google.com>
module_total_size() returns unsigned int but uses a signed int
accumulator. While the result is numerically correct, the type
mismatch is misleading.
Change the accumulator to unsigned int to match the return type.
Signed-off-by: Naveen Kumar Chaudhary <naveen.osdev@gmail.com>
---
Changes in v2:
- Modify the commit message
Thanks Sami for the catch. I have corrected the commit message.
Regards,
Naveen
kernel/module/procfs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/module/procfs.c b/kernel/module/procfs.c
index 0a4841e88adb..90712aa9dd13 100644
--- a/kernel/module/procfs.c
+++ b/kernel/module/procfs.c
@@ -64,7 +64,7 @@ static void m_stop(struct seq_file *m, void *p)
static unsigned int module_total_size(struct module *mod)
{
- int size = 0;
+ unsigned int size = 0;
for_each_mod_mem_type(type)
size += mod->mem[type].size;
--
2.43.0
^ permalink raw reply related
* Re: [PATCH] rust: module_param: use pr_warn_once! for null pointer warning
From: Miguel Ojeda @ 2026-06-06 20:15 UTC (permalink / raw)
To: Maurice Hieronymus
Cc: Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
Aaron Tomlin, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, linux-modules, linux-kernel,
rust-for-linux
In-Reply-To: <20260606-module-param-pr-warn-once-v1-1-96d9a93e8a1b@mailbox.org>
On Sat, Jun 6, 2026 at 8:24 PM Maurice Hieronymus <mhi@mailbox.org> wrote:
>
> This resolves the existing TODO that asked for pr_warn_once! once
> it became available.
>
> Signed-off-by: Maurice Hieronymus <mhi@mailbox.org>
Thanks for the patch!
This was already sent and applied, please see commit:
ac2f40107cf1 ("rust: module_param: use `pr_warn_once!` for null
pointer warning")
in linux-next.
Cheers,
Miguel
^ permalink raw reply
* [PATCH] rust: module_param: use pr_warn_once! for null pointer warning
From: Maurice Hieronymus @ 2026-06-06 18:24 UTC (permalink / raw)
To: Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
Aaron Tomlin, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich
Cc: linux-modules, linux-kernel, rust-for-linux, Maurice Hieronymus
This resolves the existing TODO that asked for pr_warn_once! once
it became available.
Signed-off-by: Maurice Hieronymus <mhi@mailbox.org>
---
rust/kernel/module_param.rs | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/rust/kernel/module_param.rs b/rust/kernel/module_param.rs
index 6a8a7a875643..dd6d663a0a3c 100644
--- a/rust/kernel/module_param.rs
+++ b/rust/kernel/module_param.rs
@@ -62,8 +62,7 @@ pub trait ModuleParam: Sized + Copy {
// NOTE: If we start supporting arguments without values, val _is_ allowed
// to be null here.
if val.is_null() {
- // TODO: Use pr_warn_once available.
- crate::pr_warn!("Null pointer passed to `module_param::set_param`");
+ crate::pr_warn_once!("Null pointer passed to `module_param::set_param`");
return EINVAL.to_errno();
}
---
base-commit: 838a0871cb3cf5988560d17afaaf57592bdb486b
change-id: 20260606-module-param-pr-warn-once-ed2f548ea741
Best regards,
--
Maurice Hieronymus <mhi@mailbox.org>
^ permalink raw reply related
* Re: [PATCH] module: procfs: fix signed integer overflow in module_total_size()
From: Sami Tolvanen @ 2026-06-05 20:52 UTC (permalink / raw)
To: Naveen Kumar Chaudhary
Cc: mcgrof, petr.pavlu, da.gomez, atomlin, linux-modules,
linux-kernel
In-Reply-To: <2k7x7245kygzpa5wzssebsrt5ivpykvvoeiljb5at2yiwgp4fy@fqkcethe4hyx>
On Wed, Jun 03, 2026 at 09:54:11PM +0530, Naveen Kumar Chaudhary wrote:
> module_total_size() accumulates unsigned section sizes into a signed int
> before returning as unsigned int. If the total exceeds INT_MAX, this is
> signed integer overflow.
This doesn't sound accurate to me. The compiler performs an implicit
type conversion, but there's no signed integer overflow:
https://godbolt.org/z/hzGrYMsPW
> Change the accumulator to unsigned int to match the return type.
>
> Signed-off-by: Naveen Kumar Chaudhary <naveen.osdev@gmail.com>
> ---
> kernel/module/procfs.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/module/procfs.c b/kernel/module/procfs.c
> index 0a4841e88adb..90712aa9dd13 100644
> --- a/kernel/module/procfs.c
> +++ b/kernel/module/procfs.c
> @@ -64,7 +64,7 @@ static void m_stop(struct seq_file *m, void *p)
>
> static unsigned int module_total_size(struct module *mod)
> {
> - int size = 0;
> + unsigned int size = 0;
While there's no behavioral difference, using unsigned int seems like
good hygiene. With the commit message corrected:
Reviewed-by: Sami Tolvanen <samitolvanen@google.com>
Sami
^ permalink raw reply
* Re: [PATCH 0/2] module: restrict module auto-loading to privileged users
From: Sami Tolvanen @ 2026-06-05 18:36 UTC (permalink / raw)
To: Michal Gorlas, Kees Cook
Cc: Jonathan Corbet, Shuah Khan, Luis Chamberlain, Petr Pavlu,
Daniel Gomez, Aaron Tomlin, linux-doc, linux-kernel,
linux-modules
In-Reply-To: <20260515-autoload_restrict-v1-0-40b7c03ddd04@9elements.com>
On Fri, May 15, 2026 at 07:20:18PM +0200, Michal Gorlas wrote:
> Add option to restrict the module auto-loading to CAP_SYS_ADMIN.
> This is heavily inspired by CONFIG_GRKERNSEC_MODHARDEN of the latest
> available Grsecurity patches [1]. Instead of checking whether the
> callers' UID is 0, check whether the calling process has CAP_SYS_ADMIN.
> The reasoning here is that many modules are autoloaded by systemd
> services which are running as privileged users, but do not have UID 0.
> While systemd-udevd runs as root, systemd-network (which often
> auto-loads a module) for example runs as system user (UID range 6 to
> 999).
>
> When enabled, reduces attack surface where unprivileged users can trigger
> vulnerable module to be auto-loaded, to then exploit it. Recent LPEs
> (CopyFail [3], DirtyFrag [4]) for example, would have been mitigated
> with this option enabled as long as the vulnerable modules are not built-in
> (or already loaded at the point of running the exploit).
This sounds potentially useful as an optional feature. Kees, you've
looked at grsec features in the past, do you have any thoughts about
this?
Sami
^ permalink raw reply
* Re: [PATCH 2/2] module: restrict autoload to CAP_SYS_ADMIN if CONFIG_MODULE_RESTRICT_AUTOLOAD
From: Sami Tolvanen @ 2026-06-05 18:30 UTC (permalink / raw)
To: Michal Gorlas
Cc: Jonathan Corbet, Shuah Khan, Luis Chamberlain, Petr Pavlu,
Daniel Gomez, Aaron Tomlin, linux-doc, linux-kernel,
linux-modules
In-Reply-To: <20260515-autoload_restrict-v1-2-40b7c03ddd04@9elements.com>
On Fri, May 15, 2026 at 07:20:20PM +0200, Michal Gorlas wrote:
> Restrict module auto-loading to CAP_SYS_ADMIN if
> CONFIG_MODULE_RESTRICT_AUTOLOAD is enabled, cmdline parameter
> modrestrict=true, or kernel.modrestrict=1 is set with sysctl.
>
> Signed-off-by: Michal Gorlas <michal.gorlas@9elements.com>
> ---
> kernel/module/internal.h | 1 +
> kernel/module/kmod.c | 5 +++++
> kernel/module/main.c | 11 +++++++++++
> 3 files changed, 17 insertions(+)
>
> diff --git a/kernel/module/internal.h b/kernel/module/internal.h
> index 061161cc79d9..496d8703f0c6 100644
> --- a/kernel/module/internal.h
> +++ b/kernel/module/internal.h
> @@ -46,6 +46,7 @@ struct kernel_symbol {
>
> extern struct mutex module_mutex;
> extern struct list_head modules;
> +extern bool module_autoload_restrict;
>
> extern const struct module_attribute *const modinfo_attrs[];
> extern const size_t modinfo_attrs_count;
> diff --git a/kernel/module/kmod.c b/kernel/module/kmod.c
> index a25dccdf7aa7..58b28c23f571 100644
> --- a/kernel/module/kmod.c
> +++ b/kernel/module/kmod.c
> @@ -156,6 +156,11 @@ int __request_module(bool wait, const char *fmt, ...)
> if (ret)
> return ret;
>
> + if (module_autoload_restrict && !capable(CAP_SYS_ADMIN)) {
> + pr_alert("denied attempt to auto-load module %s\n", module_name);
Is pr_alert appropriate here or can this be a warning? Also, use the _ratelimited
variant like the pre-existing warning in this function.
Sami
^ permalink raw reply
* Re: [PATCH 1/2] module: add CONFIG_MODULE_RESTRICT_AUTOLOAD
From: Sami Tolvanen @ 2026-06-05 18:25 UTC (permalink / raw)
To: Michal Gorlas
Cc: Jonathan Corbet, Shuah Khan, Luis Chamberlain, Petr Pavlu,
Daniel Gomez, Aaron Tomlin, linux-doc, linux-kernel,
linux-modules
In-Reply-To: <20260515-autoload_restrict-v1-1-40b7c03ddd04@9elements.com>
On Fri, May 15, 2026 at 07:20:19PM +0200, Michal Gorlas wrote:
> Add CONFIG_MODULE_RESTRICT_AUTOLOAD and modrestrict parameter
> documentation.
>
> Signed-off-by: Michal Gorlas <michal.gorlas@9elements.com>
> ---
> Documentation/admin-guide/kernel-parameters.txt | 5 +++++
> kernel/module/Kconfig | 15 +++++++++++++++
> 2 files changed, 20 insertions(+)
>
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 03a550630644..1013104f0943 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -4185,6 +4185,11 @@ Kernel parameters
> For details see:
> Documentation/admin-guide/hw-vuln/processor_mmio_stale_data.rst
>
> + modrestrict=<bool>
> + Control the restriction of module auto-loading to
> + CAP_SYS_ADMIN. If no <bool> value is specified, this
> + is set to the value of CONFIG_MODULE_RESTRICT_AUTOLOAD.
Doesn't this default to true if no bool value is specified? It only uses
the config if modrestrict is not passed to the kernel at all.
> <module>.async_probe[=<bool>] [KNL]
> If no <bool> value is specified or if the value
> specified is not a valid <bool>, enable asynchronous
> diff --git a/kernel/module/Kconfig b/kernel/module/Kconfig
> index 43b1bb01fd27..c9e01bb848c0 100644
> --- a/kernel/module/Kconfig
> +++ b/kernel/module/Kconfig
> @@ -337,6 +337,21 @@ config MODULE_SIG_HASH
>
> endif # MODULE_SIG || IMA_APPRAISE_MODSIG
>
> +config MODULE_RESTRICT_AUTOLOAD
> + bool "Restrict module auto-loading to privileged users"
> + default n
You don't need to specify default n here.
Also, I think you can just squash the two patches. There's no benefit
in splitting the config/documentation into a separate patch.
Sami
^ permalink raw reply
* Re: [PATCH v2] module: use strscpy() to copy module names in stats and dup tracking
From: sashiko-bot @ 2026-06-04 18:19 UTC (permalink / raw)
To: Naveen Kumar Chaudhary; +Cc: linux-modules
In-Reply-To: <slebbzn7p6c4ybokolvey2sgvrooit66zjewwggqac2755ujgp@ptanmtdclmw5>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [Medium] Incomplete fix: Truncating overlong module names breaks deduplication and causes list bloat/memory leak.
Pre-existing issues:
- [High] Race condition in duplicate module load tracking causing Use-After-Free.
--
commit 609988fdef836043f78467817ec8e3cba34b5daf
Author: Naveen Kumar Chaudhary <naveen.osdev@gmail.com>
module: use strscpy() to copy module names in stats and dup tracking
Both try_add_failed_module() and kmod_dup_request_exists_wait() use
memcpy() with strlen() to copy module names into fixed-size buffers.
Neither performs a bounds check on the copy. This patch replaces
memcpy() with strscpy() to ensure the copy is bounded and NUL-terminated.
> diff --git a/kernel/module/dups.c b/kernel/module/dups.c
> index 1d720a5311ba0..8b6f8f177de4f 100644
> --- a/kernel/module/dups.c
> +++ b/kernel/module/dups.c
> @@ -129,7 +129,7 @@ bool kmod_dup_request_exists_wait(char *module_name, bool wait, int *dup_ret)
> if (!new_kmod_req)
> return false;
>
> - memcpy(new_kmod_req->name, module_name, strlen(module_name));
> + strscpy(new_kmod_req->name, module_name);
[Severity: High]
This is a pre-existing issue, but looking at kmod_dup_request_exists_wait(),
is there a race condition that could lead to a use-after-free?
In kmod_dup_request_exists_wait(), the kmod_req pointer is retrieved while
holding kmod_dup_mutex, but the lock is dropped before waiting on the
completion:
kernel/module/dups.c:kmod_dup_request_exists_wait() {
...
mutex_unlock(&kmod_dup_mutex);
...
ret = wait_for_completion_state(&kmod_req->first_req_done, TASK_KILLABLE);
if (ret) {
*dup_ret = ret;
return true;
}
...
*dup_ret = kmod_req->dup_ret;
...
}
There doesn't appear to be a reference count or RCU read-side critical section
protecting kmod_req during this wait.
The completion is signaled by kmod_dup_request_complete(), which then
schedules kmod_dup_request_delete() via delayed work after 60 seconds.
If a duplicate request arrives exactly 60 seconds later, it could drop the
mutex just before the delayed work executes and frees the request:
kernel/module/dups.c:kmod_dup_request_delete() {
...
mutex_lock(&kmod_dup_mutex);
list_del_rcu(&kmod_req->list);
synchronize_rcu();
mutex_unlock(&kmod_dup_mutex);
kfree(kmod_req);
}
When the duplicate thread resumes and accesses kmod_req->dup_ret, would it
dereference the freed memory?
[ ... ]
> diff --git a/kernel/module/stats.c b/kernel/module/stats.c
> index 3a9672f93a8ef..78b59bc05387f 100644
> --- a/kernel/module/stats.c
> +++ b/kernel/module/stats.c
> @@ -253,7 +253,7 @@ int try_add_failed_module(const char *name, enum fail_dup_mod_reason reason)
> mod_fail = kzalloc_obj(*mod_fail);
> if (!mod_fail)
> return -ENOMEM;
> - memcpy(mod_fail->name, name, strlen(name));
> + strscpy(mod_fail->name, name);
[Severity: Medium]
Does truncating overlong module names here break deduplication?
If try_add_failed_module() is called with an overlong name (which can
originate from the .modinfo ELF section), strscpy() safely truncates the
string stored in mod_fail->name.
However, the deduplication lookup uses strcmp(), which compares the truncated
stored name against the untruncated original string:
kernel/module/stats.c:try_add_failed_module() {
...
list_for_each_entry_rcu(mod_fail, &dup_failed_modules, list, lockdep_is_held(&module_mutex)) {
if (!strcmp(mod_fail->name, name)) {
atomic_long_inc(&mod_fail->count);
...
}
This comparison will always fail for overlong names.
Would every attempt to load a malformed module allocate a new mod_fail struct
and add it to dup_failed_modules, potentially allowing an attacker with
CAP_SYS_MODULE to trigger unbounded list bloat and a memory leak?
> __set_bit(reason, &mod_fail->dup_fail_mask);
> atomic_long_inc(&mod_fail->count);
> list_add_rcu(&mod_fail->list, &dup_failed_modules);
--
Sashiko AI review · https://sashiko.dev/#/patchset/slebbzn7p6c4ybokolvey2sgvrooit66zjewwggqac2755ujgp@ptanmtdclmw5?part=1
^ permalink raw reply
* [PATCH v2] module: use strscpy() to copy module names in stats and dup tracking
From: Naveen Kumar Chaudhary @ 2026-06-04 17:45 UTC (permalink / raw)
To: petr.pavlu; +Cc: mcgrof, da.gomez, samitolvanen, atomlin, linux-modules
In-Reply-To: <59c7b5d1-643b-4ca5-94ad-eaf82a66fc9e@suse.com>
Both try_add_failed_module() and kmod_dup_request_exists_wait() use
memcpy() with strlen() to copy module names into fixed-size
char[MODULE_NAME_LEN] buffers. Neither performs a bounds check on the
copy. Current callers always pass names originating from
mod->name (itself char[MODULE_NAME_LEN]), so this is not exploitable
today. However both functions accept a plain const char * with no
documented length contract, making them latent buffer overflows if a
future caller passes a longer string.
Replace memcpy() with strscpy() in both sites, which bounds the copy
to MODULE_NAME_LEN and always NUL-terminates.
Signed-off-by: Naveen Kumar Chaudhary <naveen.osdev@gmail.com>
---
v1 -> v2:
- Dropped third argument to strscpy
- Merged the other patch https://lore.kernel.org/linux-modules/jmm7r4r3k3qt767tl7lojglosgc3umhc63cdp2fckdkgb3fzki@3fgvxgvzo5ex/
Thanks Petr for the reviews. Taken care of the suggestions.
Regards,
Naveen
kernel/module/dups.c | 2 +-
kernel/module/stats.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/module/dups.c b/kernel/module/dups.c
index 1d720a5311ba..33bddfb57317 100644
--- a/kernel/module/dups.c
+++ b/kernel/module/dups.c
@@ -129,7 +129,7 @@ bool kmod_dup_request_exists_wait(char *module_name, bool wait, int *dup_ret)
if (!new_kmod_req)
return false;
- memcpy(new_kmod_req->name, module_name, strlen(module_name));
+ strscpy(new_kmod_req->name, module_name);
INIT_WORK(&new_kmod_req->complete_work, kmod_dup_request_complete);
INIT_DELAYED_WORK(&new_kmod_req->delete_work, kmod_dup_request_delete);
init_completion(&new_kmod_req->first_req_done);
diff --git a/kernel/module/stats.c b/kernel/module/stats.c
index 3a9672f93a8e..08724baca773 100644
--- a/kernel/module/stats.c
+++ b/kernel/module/stats.c
@@ -253,7 +253,7 @@ int try_add_failed_module(const char *name, enum fail_dup_mod_reason reason)
mod_fail = kzalloc_obj(*mod_fail);
if (!mod_fail)
return -ENOMEM;
- memcpy(mod_fail->name, name, strlen(name));
+ strscpy(mod_fail->name, name);
__set_bit(reason, &mod_fail->dup_fail_mask);
atomic_long_inc(&mod_fail->count);
list_add_rcu(&mod_fail->list, &dup_failed_modules);
--
2.43.0
^ permalink raw reply related
* Re: [PATCH] module: stats: add lockdep annotation for dup_failed_modules list traversal
From: Naveen Kumar Chaudhary @ 2026-06-04 17:19 UTC (permalink / raw)
To: Petr Pavlu
Cc: mcgrof, da.gomez, samitolvanen, atomlin, linux-modules,
linux-kernel
In-Reply-To: <2d797a44-bdd4-4b37-8bdd-2e2052c6dd38@suse.com>
Thanks Petr. Noted.
On Thu 04 Jun 10:40 AM, Petr Pavlu wrote:
> On 6/3/26 6:22 PM, Naveen Kumar Chaudhary wrote:
> > read_file_mod_stats() traverses dup_failed_modules with
> > list_for_each_entry_rcu() while holding module_mutex, but does not pass
> > the lockdep condition. This triggers a false-positive "RCU-list traversed
> > in non-reader section" warning with CONFIG_PROVE_RCU_LIST=y.
> >
> > The warning can be reproduced by:
> > 1. Racing two loads of the same module to populate dup_failed_modules:
> > insmod dummy.ko &
> > insmod dummy.ko &
> > 2. Reading the stats debugfs file:
> > cat /sys/kernel/debug/modules/stats
> >
> > =============================
> > WARNING: suspicious RCU usage
> > 7.1.0-rc5-gae12a56ba16a #1 Not tainted
> > -----------------------------
> > kernel/module/stats.c:385 RCU-list traversed in non-reader section!!
> >
> > other info that might help us debug this:
> >
> > rcu_scheduler_active = 2, debug_locks = 1
> > 1 lock held by cat/128:
> > #0: ffff80008288f7a8 (module_mutex){+.+.}-{4:4}, at: read_file_mod_stats+0x46c/0x5ec
> >
> > stack backtrace:
> > CPU: 1 UID: 0 PID: 128 Comm: cat Kdump: loaded Not tainted 7.1.0-rc5-gae12a56ba16a #1 PREEMPT
> > Hardware name: linux,dummy-virt (DT)
> > Call trace:
> > show_stack+0x18/0x24 (C)
> > __dump_stack+0x28/0x38
> > dump_stack_lvl+0x64/0x84
> > dump_stack+0x18/0x24
> > lockdep_rcu_suspicious+0x134/0x1d4
> > read_file_mod_stats+0x554/0x5ec
> > full_proxy_read+0xe0/0x1ac
> > vfs_read+0xd8/0x2b0
> > ksys_read+0x70/0xe4
> > __arm64_sys_read+0x1c/0x28
> > invoke_syscall+0x48/0xf8
> > el0_svc_common+0x8c/0xd8
> > do_el0_svc+0x1c/0x28
> > el0_svc+0x58/0x1d8
> > el0t_64_sync_handler+0x84/0x12c
> > el0t_64_sync+0x198/0x19c
> >
> > The traversal is protected by module_mutex, so pass
> > lockdep_is_held(&module_mutex) to inform the checker.
> >
> > Signed-off-by: Naveen Kumar Chaudhary <naveen.osdev@gmail.com>
> > ---
> > kernel/module/stats.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/kernel/module/stats.c b/kernel/module/stats.c
> > index 3a9672f93a8e..a62961acd8e3 100644
> > --- a/kernel/module/stats.c
> > +++ b/kernel/module/stats.c
> > @@ -382,7 +382,8 @@ static ssize_t read_file_mod_stats(struct file *file, char __user *user_buf,
> > mutex_lock(&module_mutex);
> >
> >
> > - list_for_each_entry_rcu(mod_fail, &dup_failed_modules, list) {
> > + list_for_each_entry_rcu(mod_fail, &dup_failed_modules, list,
> > + lockdep_is_held(&module_mutex)) {
> > if (WARN_ON_ONCE(++count_failed >= MAX_FAILED_MOD_PRINT))
> > goto out_unlock;
> > len += scnprintf(buf + len, size - len, "%25s\t%15lu\t%25s\n", mod_fail->name,
>
> I mentioned in my reply to your previous patch fixing the use of
> synchronize_rcu() in the module dups code [1] that the overall RCU usage
> in this code appears to be incorrect. I also noticed that Sashiko
> reported the same issue. I think it is not very productive to try to fix
> these specific RCU-related problems and instead the code should be
> properly reworked. It most likely should not be using RCU at all and
> kmod_dup_req should instead be reference-counted.
>
> [1] https://lore.kernel.org/linux-modules/ajydyxgaea27rhcopp5eauji24znotu65d2b4uw344yvmwcc6f@7l5re6f2xcuk/
>
> --
> Thanks,
> Petr
^ permalink raw reply
* Re: [PATCH 2/2] module: Remove unnecessary module::args
From: Aaron Tomlin @ 2026-06-04 13:25 UTC (permalink / raw)
To: Petr Pavlu
Cc: Chris Zankel, Max Filippov, Luis Chamberlain, Daniel Gomez,
Sami Tolvanen, Matthew Wood, linux-modules, linux-kernel
In-Reply-To: <20260604110455.1608038-3-petr.pavlu@suse.com>
On Thu, Jun 04, 2026 at 01:04:46PM +0200, Petr Pavlu wrote:
> Historically, various parameter-handling code kept pointers into
> module::args, most notably the charp support. However, in 2009,
> commit e180a6b7759a ("param: fix charp parameters set via sysfs") changed
> charp parameters to kstrdup() the input string as well. As a result,
> module::args now mostly wastes memory.
>
> The last users that still pointed into module::args have now been cleaned
> up, so remove this data.
>
> Signed-off-by: Petr Pavlu <petr.pavlu@suse.com>
> ---
> include/linux/module.h | 4 ----
> kernel/module/main.c | 15 ++++++++-------
> 2 files changed, 8 insertions(+), 11 deletions(-)
>
> diff --git a/include/linux/module.h b/include/linux/module.h
> index 7566815fabbe..96cc98568eea 100644
> --- a/include/linux/module.h
> +++ b/include/linux/module.h
> @@ -477,10 +477,6 @@ struct module {
> struct module_notes_attrs *notes_attrs;
> #endif
>
> - /* The command line arguments (may be mangled). People like
> - keeping pointers to this stuff */
> - char *args;
> -
> #ifdef CONFIG_SMP
> /* Per-cpu data. */
> void __percpu *percpu;
> diff --git a/kernel/module/main.c b/kernel/module/main.c
> index 46dd8d25a605..528690ba160b 100644
> --- a/kernel/module/main.c
> +++ b/kernel/module/main.c
> @@ -1458,7 +1458,6 @@ static void free_module(struct module *mod)
>
> /* This may be empty, but that's OK */
> module_arch_freeing_init(mod);
> - kfree(mod->args);
> percpu_modfree(mod);
>
> free_mod_mem(mod);
> @@ -3425,7 +3424,7 @@ static int load_module(struct load_info *info, const char __user *uargs,
> struct module *mod;
> bool module_allocated = false;
> long err = 0;
> - char *after_dashes;
> + char *args = NULL, *after_dashes;
>
> /*
> * Do the signature check (if any) first. All that
> @@ -3523,9 +3522,9 @@ static int load_module(struct load_info *info, const char __user *uargs,
> flush_module_icache(mod);
>
> /* Now copy in args */
> - mod->args = strndup_user(uargs, ~0UL >> 1);
> - if (IS_ERR(mod->args)) {
> - err = PTR_ERR(mod->args);
> + args = strndup_user(uargs, ~0UL >> 1);
> + if (IS_ERR(args)) {
> + err = PTR_ERR(args);
> goto free_arch_cleanup;
> }
>
> @@ -3546,7 +3545,7 @@ static int load_module(struct load_info *info, const char __user *uargs,
> mod->async_probe_requested = async_probe;
>
> /* Module is ready to execute: parsing args may do that. */
> - after_dashes = parse_args(mod->name, mod->args, mod->kp, mod->num_kp,
> + after_dashes = parse_args(mod->name, args, mod->kp, mod->num_kp,
> -32768, 32767, mod,
> unknown_module_param_cb);
> if (IS_ERR(after_dashes)) {
> @@ -3556,6 +3555,8 @@ static int load_module(struct load_info *info, const char __user *uargs,
> pr_warn("%s: parameters '%s' after `--' ignored\n",
> mod->name, after_dashes);
> }
> + kfree(args);
> + args = NULL;
>
> /* Link in to sysfs. */
> err = mod_sysfs_setup(mod, info, mod->kp, mod->num_kp);
> @@ -3597,7 +3598,7 @@ static int load_module(struct load_info *info, const char __user *uargs,
> ddebug_cleanup:
> ftrace_release_mod(mod);
> synchronize_rcu();
> - kfree(mod->args);
> + kfree(args);
> free_arch_cleanup:
> module_arch_cleanup(mod);
> free_modinfo:
> --
> 2.54.0
>
LGTM.
Reviewed-by: Aaron Tomlin <atomlin@atomlin.com>
--
Aaron Tomlin
^ permalink raw reply
* Re: [PATCH 1/2] xtensa/simdisk: Avoid referring to module::args
From: sashiko-bot @ 2026-06-04 11:18 UTC (permalink / raw)
To: Petr Pavlu; +Cc: linux-modules
In-Reply-To: <20260604110455.1608038-2-petr.pavlu@suse.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Concurrent access to `dev->filename` in `proc_read_simdisk` and `simdisk_detach` causes a Use-After-Free.
--
commit e0a519c0b452f39fc91f2ac38f46b81a32db738a
Author: Petr Pavlu <petr.pavlu@suse.com>
xtensa/simdisk: Avoid referring to module::args
This commit prepares for the removal of module::args by copying the filename
string provided as a parameter instead of storing a direct pointer when built
as a loadable module. It also adds a free function to clean up these copied
strings upon module unload.
> diff --git a/arch/xtensa/platforms/iss/simdisk.c b/arch/xtensa/platforms/iss/simdisk.c
> index 7c7a2aa749f82..a2ddb49c269dc 100644
> --- a/arch/xtensa/platforms/iss/simdisk.c
> +++ b/arch/xtensa/platforms/iss/simdisk.c
[ ... ]
> static const struct kernel_param_ops simdisk_param_ops_filename = {
> .set = simdisk_param_set_filename,
> + .free = simdisk_param_free_filename,
> };
> -module_param_cb(filename, &simdisk_param_ops_filename, &n_files, 0);
> +module_param_cb(filename, &simdisk_param_ops_filename, NULL, 0);
> MODULE_PARM_DESC(filename, "Backing storage filename.");
>
> static int simdisk_major = SIMDISK_MAJOR;
[Severity: High]
This isn't a bug introduced by this patch, but is it possible to trigger a
use-after-free here due to concurrent access to dev->filename?
The procfs entry for simdisk devices is created with 0644 permissions, allowing
any user to read it. In proc_read_simdisk(), dev->filename is fetched without
acquiring dev->lock:
arch/xtensa/platforms/iss/simdisk.c:proc_read_simdisk() {
...
const char *s = dev->filename;
if (s) {
ssize_t len = strlen(s);
char *temp = kmalloc(len + 2, GFP_KERNEL);
...
}
If an administrator concurrently writes to the same procfs entry, it triggers
proc_write_simdisk() -> simdisk_detach(), which acquires dev->lock and frees
the filename:
arch/xtensa/platforms/iss/simdisk.c:simdisk_detach() {
...
spin_lock(&dev->lock);
...
} else {
pr_info("SIMDISK: %s detached from %s\n",
dev->gd->disk_name, dev->filename);
dev->fd = -1;
kfree(dev->filename);
dev->filename = NULL;
}
...
}
If proc_read_simdisk() fetches the pointer right before simdisk_detach() frees
it, the subsequent strlen() and scnprintf() operations could dereference freed
memory. Does this need locking or RCU protection to prevent a potential oops
or local information leak?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260604110455.1608038-1-petr.pavlu@suse.com?part=1
^ permalink raw reply
* [PATCH 2/2] module: Remove unnecessary module::args
From: Petr Pavlu @ 2026-06-04 11:04 UTC (permalink / raw)
To: Chris Zankel, Max Filippov, Luis Chamberlain, Petr Pavlu,
Daniel Gomez, Sami Tolvanen
Cc: Aaron Tomlin, Matthew Wood, linux-modules, linux-kernel
In-Reply-To: <20260604110455.1608038-1-petr.pavlu@suse.com>
Historically, various parameter-handling code kept pointers into
module::args, most notably the charp support. However, in 2009,
commit e180a6b7759a ("param: fix charp parameters set via sysfs") changed
charp parameters to kstrdup() the input string as well. As a result,
module::args now mostly wastes memory.
The last users that still pointed into module::args have now been cleaned
up, so remove this data.
Signed-off-by: Petr Pavlu <petr.pavlu@suse.com>
---
include/linux/module.h | 4 ----
kernel/module/main.c | 15 ++++++++-------
2 files changed, 8 insertions(+), 11 deletions(-)
diff --git a/include/linux/module.h b/include/linux/module.h
index 7566815fabbe..96cc98568eea 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -477,10 +477,6 @@ struct module {
struct module_notes_attrs *notes_attrs;
#endif
- /* The command line arguments (may be mangled). People like
- keeping pointers to this stuff */
- char *args;
-
#ifdef CONFIG_SMP
/* Per-cpu data. */
void __percpu *percpu;
diff --git a/kernel/module/main.c b/kernel/module/main.c
index 46dd8d25a605..528690ba160b 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -1458,7 +1458,6 @@ static void free_module(struct module *mod)
/* This may be empty, but that's OK */
module_arch_freeing_init(mod);
- kfree(mod->args);
percpu_modfree(mod);
free_mod_mem(mod);
@@ -3425,7 +3424,7 @@ static int load_module(struct load_info *info, const char __user *uargs,
struct module *mod;
bool module_allocated = false;
long err = 0;
- char *after_dashes;
+ char *args = NULL, *after_dashes;
/*
* Do the signature check (if any) first. All that
@@ -3523,9 +3522,9 @@ static int load_module(struct load_info *info, const char __user *uargs,
flush_module_icache(mod);
/* Now copy in args */
- mod->args = strndup_user(uargs, ~0UL >> 1);
- if (IS_ERR(mod->args)) {
- err = PTR_ERR(mod->args);
+ args = strndup_user(uargs, ~0UL >> 1);
+ if (IS_ERR(args)) {
+ err = PTR_ERR(args);
goto free_arch_cleanup;
}
@@ -3546,7 +3545,7 @@ static int load_module(struct load_info *info, const char __user *uargs,
mod->async_probe_requested = async_probe;
/* Module is ready to execute: parsing args may do that. */
- after_dashes = parse_args(mod->name, mod->args, mod->kp, mod->num_kp,
+ after_dashes = parse_args(mod->name, args, mod->kp, mod->num_kp,
-32768, 32767, mod,
unknown_module_param_cb);
if (IS_ERR(after_dashes)) {
@@ -3556,6 +3555,8 @@ static int load_module(struct load_info *info, const char __user *uargs,
pr_warn("%s: parameters '%s' after `--' ignored\n",
mod->name, after_dashes);
}
+ kfree(args);
+ args = NULL;
/* Link in to sysfs. */
err = mod_sysfs_setup(mod, info, mod->kp, mod->num_kp);
@@ -3597,7 +3598,7 @@ static int load_module(struct load_info *info, const char __user *uargs,
ddebug_cleanup:
ftrace_release_mod(mod);
synchronize_rcu();
- kfree(mod->args);
+ kfree(args);
free_arch_cleanup:
module_arch_cleanup(mod);
free_modinfo:
--
2.54.0
^ permalink raw reply related
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