public inbox for linux-modules@vger.kernel.org
 help / color / mirror / Atom feed

* [PATCH 6/8] rust: macros: add early_param support to module! macro
From: Matthew Wood @ 2026-02-26 23:47 UTC (permalink / raw)
  To: Miguel Ojeda, Luis Chamberlain, Petr Pavlu, Daniel Gomez,
	Sami Tolvanen
  Cc: Aaron Tomlin, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Danilo Krummrich, Tamir Duberstein, David Gow,
	José Expósito, linux-modules, rust-for-linux,
	linux-kernel
In-Reply-To: <20260226234736.428341-1-thepacketgeek@gmail.com>

Allow module parameters to be registered as early boot command-line
parameters via the existing __setup infrastructure.

Add an optional `early_param` field to the parameter block in the
module! macro.  When specified, the macro generates:

  - A setup callback that calls ModuleParam::from_setup_arg() and
    stores the result via ModuleParamAccess::set_value().
  - A static string in .init.rodata with the command-line prefix.
  - An ObsKernelParam entry in the .init.setup section linking the
    two together.

The generated code is gated behind #[cfg(not(MODULE))] since __setup
parameters are only meaningful for built-in modules — loadable modules
receive their parameters through the standard module_param mechanism.

Signed-off-by: Matthew Wood <thepacketgeek@gmail.com>
---
 rust/macros/lib.rs    |  5 +++
 rust/macros/module.rs | 81 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 86 insertions(+)

diff --git a/rust/macros/lib.rs b/rust/macros/lib.rs
index 0c36194d9971..83dcc89a425a 100644
--- a/rust/macros/lib.rs
+++ b/rust/macros/lib.rs
@@ -35,9 +35,14 @@
 /// parameter_name: type {
 ///     default: default_value,
 ///     description: "Description",
+///     early_param: "cmdline_prefix=",  // optional
 /// }
 /// ```
 ///
+/// The optional `early_param` field registers the parameter as an early
+/// boot command-line parameter via `__setup`. The value is the command-line
+/// prefix string (e.g. `"netconsole="`).
+///
 /// `type` may be one of
 ///
 /// - [`i8`]
diff --git a/rust/macros/module.rs b/rust/macros/module.rs
index 0d76743741fb..4d2e144fa6de 100644
--- a/rust/macros/module.rs
+++ b/rust/macros/module.rs
@@ -38,6 +38,7 @@ struct ModInfoBuilder<'a> {
     counter: usize,
     ts: TokenStream,
     param_ts: TokenStream,
+    setup_ts: TokenStream,
 }
 
 impl<'a> ModInfoBuilder<'a> {
@@ -47,6 +48,7 @@ fn new(module: &'a str) -> Self {
             counter: 0,
             ts: TokenStream::new(),
             param_ts: TokenStream::new(),
+            setup_ts: TokenStream::new(),
         }
     }
 
@@ -204,6 +206,78 @@ fn emit_params(&mut self, info: &ModuleInfo) {
             });
         }
     }
+
+    fn emit_setup(&mut self, info: &ModuleInfo) {
+        let Some(params) = &info.params else {
+            return;
+        };
+
+        for param in params {
+            let Some(early_param) = &param.early_param else {
+                continue;
+            };
+
+            let setup_str_value = early_param.value();
+            let param_name = &param.name;
+
+            // Resolve `string` shorthand to the real `StringParam` type.
+            let param_type_str = param.ptype.to_token_stream().to_string();
+            let actual_type: Type = if param_type_str == "string" {
+                parse_quote!(::kernel::module_param::StringParam)
+            } else {
+                param.ptype.clone()
+            };
+
+            // Create identifiers for the generated statics
+            let setup_fn_name = format_ident!("__setup_fn_{}", param_name);
+            let setup_str_name =
+                format_ident!("__SETUP_STR_{}", param_name.to_string().to_uppercase());
+            let setup_name = format_ident!("__SETUP_{}", param_name.to_string().to_uppercase());
+
+            // The setup string with null terminator
+            let setup_str_bytes = format!("{}\0", setup_str_value);
+            let setup_str_len = setup_str_bytes.len();
+            let setup_str_lit = Literal::byte_string(setup_str_bytes.as_bytes());
+
+            self.setup_ts.extend(quote! {
+                #[cfg(not(MODULE))]
+                const _: () = {
+                    unsafe extern "C" fn #setup_fn_name(
+                        val: *mut ::kernel::ffi::c_char,
+                    ) -> ::kernel::ffi::c_int {
+                        if val.is_null() {
+                            return 0;
+                        }
+                        // SAFETY: The kernel passes a valid null-terminated string from
+                        // `static_command_line`.
+                        match unsafe {
+                            <#actual_type as ::kernel::module_param::ModuleParam>::from_setup_arg(
+                                val as *const _
+                            )
+                        } {
+                            Ok(v) => {
+                                if module_parameters::#param_name.set_value(v) { 1 } else { 0 }
+                            }
+                            Err(_) => 0,
+                        }
+                    }
+
+                    #[link_section = ".init.rodata"]
+                    #[used(compiler)]
+                    static #setup_str_name: [u8; #setup_str_len] = *#setup_str_lit;
+
+                    #[link_section = ".init.setup"]
+                    #[used(compiler)]
+                    static #setup_name: ::kernel::module_param::ObsKernelParam =
+                        ::kernel::module_param::ObsKernelParam {
+                            str_: #setup_str_name.as_ptr().cast(),
+                            setup_func: ::core::option::Option::Some(#setup_fn_name),
+                            early: 0,
+                        };
+                };
+            });
+        }
+    }
 }
 
 fn param_ops_path(param_type: &str) -> Path {
@@ -367,6 +441,7 @@ struct Parameter {
     ptype: Type,
     default: Expr,
     description: LitStr,
+    early_param: Option<LitStr>,
 }
 
 impl Parse for Parameter {
@@ -382,6 +457,7 @@ fn parse(input: ParseStream<'_>) -> Result<Self> {
             from fields;
             default [required] => fields.parse()?,
             description [required] => fields.parse()?,
+            early_param => fields.parse()?,
         }
 
         Ok(Self {
@@ -389,6 +465,7 @@ fn parse(input: ParseStream<'_>) -> Result<Self> {
             ptype,
             default,
             description,
+            early_param,
         })
     }
 }
@@ -501,9 +578,11 @@ pub(crate) fn module(info: ModuleInfo) -> Result<TokenStream> {
     modinfo.emit_only_builtin("file", &file, false);
 
     modinfo.emit_params(&info);
+    modinfo.emit_setup(&info);
 
     let modinfo_ts = modinfo.ts;
     let params_ts = modinfo.param_ts;
+    let setup_ts = modinfo.setup_ts;
 
     let ident_init = format_ident!("__{ident}_init");
     let ident_exit = format_ident!("__{ident}_exit");
@@ -678,5 +757,7 @@ unsafe fn __exit() {
         mod module_parameters {
             #params_ts
         }
+
+        #setup_ts
     })
 }
-- 
2.52.0


^ permalink raw reply related

* [PATCH 5/8] rust: module_param: add from_setup_arg() to ModuleParam trait
From: Matthew Wood @ 2026-02-26 23:47 UTC (permalink / raw)
  To: Miguel Ojeda, Luis Chamberlain, Petr Pavlu, Daniel Gomez,
	Sami Tolvanen
  Cc: Aaron Tomlin, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Danilo Krummrich, Tamir Duberstein, David Gow,
	José Expósito, linux-modules, rust-for-linux,
	linux-kernel
In-Reply-To: <20260226234736.428341-1-thepacketgeek@gmail.com>

Extend the ModuleParam trait with from_setup_arg(), which constructs
a parameter value from a raw C string pointer received via a __setup
callback during early boot.

The default implementation converts the pointer to a CStr and
delegates to try_from_param_arg(), which handles the parse-from-string
path used by integer types.

StringParam overrides this method to store the raw pointer directly
without parsing, since the pointer originates from static_command_line
and remains valid for the kernel's lifetime.

Also add ModuleParamAccess::set_value() to allow the __setup callback
generated by the module! macro to populate the SetOnce container with
first-write-wins semantics.

Signed-off-by: Matthew Wood <thepacketgeek@gmail.com>
---
 rust/kernel/module_param.rs | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/rust/kernel/module_param.rs b/rust/kernel/module_param.rs
index 54379a2bba51..5e14dbe03865 100644
--- a/rust/kernel/module_param.rs
+++ b/rust/kernel/module_param.rs
@@ -45,6 +45,21 @@ unsafe impl Sync for ObsKernelParam {}
 pub trait ModuleParam: Sized + Copy {
     /// Parse a parameter argument into the parameter value.
     fn try_from_param_arg(arg: &BStr) -> Result<Self>;
+
+    /// Create a parameter value from a raw `__setup` callback argument.
+    ///
+    /// Default implementation: parse the null-terminated C string via
+    /// [`ModuleParam::try_from_param_arg`]. `StringParam` overrides this to store the pointer
+    /// directly.
+    ///
+    /// # Safety
+    ///
+    /// `val` must point to a valid null-terminated string.
+    unsafe fn from_setup_arg(val: *const c_char) -> Result<Self> {
+        // SAFETY: Caller guarantees `val` points to a valid null-terminated string.
+        let cstr = unsafe { CStr::from_char_ptr(val) };
+        Self::try_from_param_arg(cstr.as_ref())
+    }
 }
 
 /// Set the module parameter from a string.
@@ -226,6 +241,12 @@ fn try_from_param_arg(_arg: &BStr) -> Result<Self> {
         // when using PARAM_OPS_STRING.
         Err(EINVAL)
     }
+
+    unsafe fn from_setup_arg(val: *const c_char) -> Result<Self> {
+        // SAFETY: Caller guarantees `val` points to a valid null-terminated string.
+        // The pointer comes from `static_command_line` which is valid for the kernel's lifetime.
+        Ok(unsafe { StringParam::from_ptr(val) })
+    }
 }
 
 /// A wrapper for kernel parameters.
@@ -266,6 +287,14 @@ pub fn value(&self) -> &T {
     pub const fn as_void_ptr(&self) -> *mut c_void {
         core::ptr::from_ref(self).cast_mut().cast()
     }
+
+    /// Set the parameter value directly.
+    ///
+    /// Returns `true` if successfully set, `false` if already populated
+    /// (first-write-wins semantics via [`SetOnce`]).
+    pub fn set_value(&self, val: T) -> bool {
+        self.value.populate(val)
+    }
 }
 
 #[doc(hidden)]
-- 
2.52.0


^ permalink raw reply related

* [PATCH 4/8] rust: module_param: add ObsKernelParam type
From: Matthew Wood @ 2026-02-26 23:47 UTC (permalink / raw)
  To: Miguel Ojeda, Luis Chamberlain, Petr Pavlu, Daniel Gomez,
	Sami Tolvanen
  Cc: Aaron Tomlin, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Danilo Krummrich, Tamir Duberstein, David Gow,
	José Expósito, linux-modules, rust-for-linux,
	linux-kernel
In-Reply-To: <20260226234736.428341-1-thepacketgeek@gmail.com>

Add ObsKernelParam, a #[repr(C)] struct that mirrors the C
obs_kernel_param structure used by the __setup / early_param
infrastructure to register boot-time command-line handlers.

The struct contains:
  - str_: pointer to the null-terminated setup string
  - setup_func: the callback invoked when the kernel parses the
    matching command-line token
  - early: flag distinguishing early_param (1) from __setup (0)

Mark it Sync because instances are written at compile time and only
read during single-threaded early boot initialization.

This type will be used by the module! macro to emit __setup entries
in the .init.setup ELF section in subsequent patches.

Signed-off-by: Matthew Wood <thepacketgeek@gmail.com>
---
 rust/kernel/module_param.rs | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/rust/kernel/module_param.rs b/rust/kernel/module_param.rs
index 67ff6f2ea9c2..54379a2bba51 100644
--- a/rust/kernel/module_param.rs
+++ b/rust/kernel/module_param.rs
@@ -26,6 +26,20 @@ pub const fn new(val: bindings::kernel_param) -> Self {
 // from Rust module.
 unsafe impl Sync for KernelParam {}
 
+/// Placed in the `.init.setup` ELF section by the `module!` macro.
+#[repr(C)]
+pub struct ObsKernelParam {
+    /// Pointer to the null-terminated setup string (e.g., `"netconsole=\0"`).
+    pub str_: *const c_char,
+    /// The setup callback function.
+    pub setup_func: Option<unsafe extern "C" fn(*mut c_char) -> c_int>,
+    /// Whether this is an early parameter (0 for `__setup`, 1 for `early_param`).
+    pub early: c_int,
+}
+
+// SAFETY: Only written at compile time, read during single-threaded boot init.
+unsafe impl Sync for ObsKernelParam {}
+
 /// Types that can be used for module parameters.
 // NOTE: This trait is `Copy` because drop could produce unsoundness during teardown.
 pub trait ModuleParam: Sized + Copy {
-- 
2.52.0


^ permalink raw reply related

* [PATCH 3/8] samples: rust_minimal: demonstrate string module parameter
From: Matthew Wood @ 2026-02-26 23:47 UTC (permalink / raw)
  To: Miguel Ojeda, Luis Chamberlain, Petr Pavlu, Daniel Gomez,
	Sami Tolvanen
  Cc: Aaron Tomlin, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Danilo Krummrich, Tamir Duberstein, David Gow,
	José Expósito, linux-modules, rust-for-linux,
	linux-kernel
In-Reply-To: <20260226234736.428341-1-thepacketgeek@gmail.com>

Add a `test_str` string parameter alongside the existing integer
parameter (renamed from `test_parameter` to `test_int` for clarity)
in the rust_minimal sample module.

The init function now prints both parameters to the kernel log,
showing how string parameters are declared, defaulted, and read
back via StringParam::as_cstr().

Also add module-level documentation showing usage via insmod and
kernel command-line with dotted notation.

Signed-off-by: Matthew Wood <thepacketgeek@gmail.com>
---
 samples/rust/rust_minimal.rs | 31 ++++++++++++++++++++++++++++---
 1 file changed, 28 insertions(+), 3 deletions(-)

diff --git a/samples/rust/rust_minimal.rs b/samples/rust/rust_minimal.rs
index 8eb9583571d7..59955e95e31a 100644
--- a/samples/rust/rust_minimal.rs
+++ b/samples/rust/rust_minimal.rs
@@ -1,6 +1,23 @@
 // SPDX-License-Identifier: GPL-2.0
 
 //! Rust minimal sample.
+//!
+//! This is a sample module written in Rust. It is intended to be a minimal
+//! example of how to write a module in Rust. It does not do anything useful,
+//! except print a message when it is loaded and unloaded.
+//!
+//! It provides examples of how to receive module parameters, which can be provided
+//! by the user when the module is loaded:
+//!
+//! ```
+//! insmod /lib/modules/$(uname -r)/kernel/samples/rust/rust_minimal.ko test_int=2 test_str=world
+//! ```
+//!
+//! or via kernel cmdline with module dotted notation (when built-in and not built as a module):
+//!
+//! ```
+//! ... rust_minimal.test_int=2 rust_minimal.test_str=world ...
+//! ```
 
 use kernel::prelude::*;
 
@@ -11,10 +28,14 @@
     description: "Rust minimal sample",
     license: "GPL",
     params: {
-        test_parameter: i64 {
+        test_int: i64 {
             default: 1,
             description: "This parameter has a default of 1",
         },
+        test_str: string {
+            default: "hello",
+            description: "This parameter has a default of hello",
+        }
     },
 }
 
@@ -26,9 +47,13 @@ impl kernel::Module for RustMinimal {
     fn init(_module: &'static ThisModule) -> Result<Self> {
         pr_info!("Rust minimal sample (init)\n");
         pr_info!("Am I built-in? {}\n", !cfg!(MODULE));
+        pr_info!("test_int: {}\n", *module_parameters::test_int.value());
         pr_info!(
-            "test_parameter: {}\n",
-            *module_parameters::test_parameter.value()
+            "test_str: {}\n",
+            module_parameters::test_str
+                .value()
+                .as_cstr()
+                .expect("test_str has a default value")
         );
 
         let mut numbers = KVec::new();
-- 
2.52.0


^ permalink raw reply related

* [PATCH 2/8] rust: module_param: wire StringParam into the module! macro
From: Matthew Wood @ 2026-02-26 23:47 UTC (permalink / raw)
  To: Miguel Ojeda, Luis Chamberlain, Petr Pavlu, Daniel Gomez,
	Sami Tolvanen
  Cc: Aaron Tomlin, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Danilo Krummrich, Tamir Duberstein, David Gow,
	José Expósito, linux-modules, rust-for-linux,
	linux-kernel
In-Reply-To: <20260226234736.428341-1-thepacketgeek@gmail.com>

Add support for `string` as a parameter type in the module! macro.

On the runtime side, add:
  - set_string_param(): an extern "C" callback matching the
    kernel_param_ops::set signature that stores the raw C string
    pointer directly into the SetOnce<StringParam> container, avoiding
    an unnecessary copy-and-parse round-trip.
  - PARAM_OPS_STRING: a static kernel_param_ops that uses
    set_string_param as its setter.
  - ModuleParam impl for StringParam with try_from_param_arg()
    returning -EINVAL, since string parameters are populated
    exclusively through the kernel's set callback.

On the macro side:
  - Change the Parameter::ptype field from Ident to syn::Type to
    support path-qualified types.
  - Recognize the `string` shorthand and resolve it to the fully
    qualified ::kernel::module_param::StringParam type during code
    generation.
  - Wrap string default values with StringParam::from_c_str(c_str!(...))
    to produce a compile-time CStr-backed default.
  - Route `string` to PARAM_OPS_STRING in param_ops_path().

Signed-off-by: Matthew Wood <thepacketgeek@gmail.com>
---
 rust/kernel/module_param.rs | 48 +++++++++++++++++++++++++++++++++++++
 rust/macros/module.rs       | 42 +++++++++++++++++++++++++-------
 2 files changed, 81 insertions(+), 9 deletions(-)

diff --git a/rust/kernel/module_param.rs b/rust/kernel/module_param.rs
index 80fe8643c0ab..67ff6f2ea9c2 100644
--- a/rust/kernel/module_param.rs
+++ b/rust/kernel/module_param.rs
@@ -86,6 +86,36 @@ pub trait ModuleParam: Sized + Copy {
     })
 }
 
+/// Set a string module parameter from a string.
+///
+/// Similar to [`set_param`] but for [`StringParam`].
+///
+/// # Safety
+///
+/// Same requirements as [`set_param`].
+unsafe extern "C" fn set_string_param(
+    val: *const c_char,
+    param: *const bindings::kernel_param,
+) -> c_int {
+    if val.is_null() {
+        crate::pr_warn!("Null pointer passed to `module_param::set_string_param`");
+        return EINVAL.to_errno();
+    }
+
+    crate::error::from_result(|| {
+        // SAFETY: val points to a valid C string from the kernel.
+        let cstr_param = unsafe { StringParam::from_ptr(val) };
+
+        // SAFETY: By function safety requirements, param.arg points to our SetOnce<StringParam>.
+        let container = unsafe { &*((*param).__bindgen_anon_1.arg.cast::<SetOnce<StringParam>>()) };
+
+        container
+            .populate(cstr_param)
+            .then_some(0)
+            .ok_or(kernel::error::code::EEXIST)
+    })
+}
+
 macro_rules! impl_int_module_param {
     ($ty:ident) => {
         impl ModuleParam for $ty {
@@ -175,6 +205,15 @@ pub fn as_bytes(&self) -> Option<&[u8]> {
 unsafe impl Send for StringParam {}
 unsafe impl Sync for StringParam {}
 
+impl ModuleParam for StringParam {
+    fn try_from_param_arg(_arg: &BStr) -> Result<Self> {
+        // For StringParam, we don't parse here - the kernel's set callback
+        // directly stores the pointer. This method should not be called
+        // when using PARAM_OPS_STRING.
+        Err(EINVAL)
+    }
+}
+
 /// A wrapper for kernel parameters.
 ///
 /// This type is instantiated by the [`module!`] macro when module parameters are
@@ -249,3 +288,12 @@ macro_rules! make_param_ops {
 make_param_ops!(PARAM_OPS_U64, u64);
 make_param_ops!(PARAM_OPS_ISIZE, isize);
 make_param_ops!(PARAM_OPS_USIZE, usize);
+
+/// Parameter ops for string parameters.
+#[doc(hidden)]
+pub static PARAM_OPS_STRING: bindings::kernel_param_ops = bindings::kernel_param_ops {
+    flags: 0,
+    set: Some(set_string_param),
+    get: None,
+    free: None,
+};
diff --git a/rust/macros/module.rs b/rust/macros/module.rs
index e16298e520c7..0d76743741fb 100644
--- a/rust/macros/module.rs
+++ b/rust/macros/module.rs
@@ -8,7 +8,8 @@
 };
 use quote::{
     format_ident,
-    quote, //
+    quote,
+    ToTokens, //
 };
 use syn::{
     braced,
@@ -120,13 +121,15 @@ fn emit_params(&mut self, info: &ModuleInfo) {
 
         for param in params {
             let param_name_str = param.name.to_string();
-            let param_type_str = param.ptype.to_string();
+            let param_type_str = param.ptype.to_token_stream().to_string();
+            // Clean up the type string for modinfo (remove spaces around ::)
+            let param_type_clean = param_type_str.replace(" ", "");
 
             let ops = param_ops_path(&param_type_str);
 
             // Note: The spelling of these fields is dictated by the user space
             // tool `modinfo`.
-            self.emit_param("parmtype", &param_name_str, &param_type_str);
+            self.emit_param("parmtype", &param_name_str, &param_type_clean);
             self.emit_param("parm", &param_name_str, &param.description.value());
 
             let static_name = format_ident!("__{}_{}_struct", self.module, param.name);
@@ -137,14 +140,32 @@ fn emit_params(&mut self, info: &ModuleInfo) {
                     .expect("name contains NUL-terminator");
 
             let param_name = &param.name;
-            let param_type = &param.ptype;
             let param_default = &param.default;
 
+            // `string` is a shorthand for `StringParam` in the macro — resolve to
+            // the real type for code generation.
+            let is_str_param = param_type_str == "string";
+            let actual_type: Type = if is_str_param {
+                parse_quote!(::kernel::module_param::StringParam)
+            } else {
+                param.ptype.clone()
+            };
+
+            // For `string` params the default is always a string literal which
+            // gets wrapped with StringParam::from_c_str(kernel::c_str!(...)).
+            let default_expr = if is_str_param {
+                quote! {
+                    ::kernel::module_param::StringParam::from_c_str(::kernel::c_str!(#param_default))
+                }
+            } else {
+                quote!(#param_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);
+                    ::kernel::module_param::ModuleParamAccess<#actual_type> =
+                        ::kernel::module_param::ModuleParamAccess::new(#default_expr);
 
                 const _: () = {
                     #[allow(non_upper_case_globals)]
@@ -186,7 +207,9 @@ fn emit_params(&mut self, info: &ModuleInfo) {
 }
 
 fn param_ops_path(param_type: &str) -> Path {
-    match param_type {
+    let type_name = param_type.rsplit("::").next().unwrap_or(param_type).trim();
+
+    match type_name {
         "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),
@@ -197,6 +220,7 @@ fn param_ops_path(param_type: &str) -> Path {
         "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),
+        "string" => parse_quote!(::kernel::module_param::PARAM_OPS_STRING),
         t => panic!("Unsupported parameter type {}", t),
     }
 }
@@ -340,7 +364,7 @@ macro_rules! parse_ordered_fields {
 
 struct Parameter {
     name: Ident,
-    ptype: Ident,
+    ptype: Type,
     default: Expr,
     description: LitStr,
 }
@@ -349,7 +373,7 @@ impl Parse for Parameter {
     fn parse(input: ParseStream<'_>) -> Result<Self> {
         let name = input.parse()?;
         input.parse::<Token![:]>()?;
-        let ptype = input.parse()?;
+        let ptype: Type = input.parse()?;
 
         let fields;
         braced!(fields in input);
-- 
2.52.0


^ permalink raw reply related

* [PATCH 1/8] rust: module_param: add StringParam type for C string parameters
From: Matthew Wood @ 2026-02-26 23:47 UTC (permalink / raw)
  To: Miguel Ojeda, Luis Chamberlain, Petr Pavlu, Daniel Gomez,
	Sami Tolvanen
  Cc: Aaron Tomlin, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Danilo Krummrich, Tamir Duberstein, David Gow,
	José Expósito, linux-modules, rust-for-linux,
	linux-kernel
In-Reply-To: <20260226234736.428341-1-thepacketgeek@gmail.com>

Introduce StringParam, a Copy wrapper around *const c_char that
represents a string module parameter whose memory is managed by the
kernel parameter subsystem.

StringParam provides:
  - from_ptr(): construct from a raw C string pointer
  - from_c_str(): construct from a static CStr (for compile-time
    default values)
  - null(): construct an unset/empty parameter
  - as_cstr() / as_bytes(): safe accessors that return None when the
    pointer is null

The type is marked Send + Sync because the underlying pointer is
effectively 'static for the module's lifetime — the kernel guarantees
the string memory remains valid while the module is loaded.

This is a prerequisite for wiring string parameters into the module!
macro in subsequent patches.

Signed-off-by: Matthew Wood <thepacketgeek@gmail.com>
---
 rust/kernel/module_param.rs | 69 +++++++++++++++++++++++++++++++++++++
 1 file changed, 69 insertions(+)

diff --git a/rust/kernel/module_param.rs b/rust/kernel/module_param.rs
index 6a8a7a875643..80fe8643c0ab 100644
--- a/rust/kernel/module_param.rs
+++ b/rust/kernel/module_param.rs
@@ -6,6 +6,7 @@
 
 use crate::prelude::*;
 use crate::str::BStr;
+use crate::str::CStr;
 use bindings;
 use kernel::sync::SetOnce;
 
@@ -106,6 +107,74 @@ fn try_from_param_arg(arg: &BStr) -> Result<Self> {
 impl_int_module_param!(isize);
 impl_int_module_param!(usize);
 
+/// A module parameter that holds a C string pointer.
+///
+/// This type is `Copy` by storing only a raw pointer. The underlying string
+/// memory is managed by the kernel's parameter subsystem.
+///
+/// # Safety
+///
+/// The pointer is only valid while the module is loaded. The kernel ensures
+/// the string memory remains valid for the module's lifetime.
+#[derive(Copy, Clone)]
+#[repr(transparent)]
+pub struct StringParam {
+    ptr: *const c_char,
+}
+
+impl StringParam {
+    /// Creates a new `StringParam` from a raw pointer.
+    ///
+    /// # Safety
+    ///
+    /// The pointer must be valid and point to a null-terminated string,
+    /// or be null for an empty/unset parameter.
+    pub const unsafe fn from_ptr(ptr: *const c_char) -> Self {
+        Self { ptr }
+    }
+
+    /// Creates a `StringParam` from a static `CStr` reference.
+    ///
+    /// Useful for compile-time default values in module parameter declarations.
+    pub const fn from_c_str(s: &'static CStr) -> Self {
+        Self {
+            ptr: crate::str::as_char_ptr_in_const_context(s),
+        }
+    }
+
+    /// Creates a null/empty `StringParam`.
+    pub const fn null() -> Self {
+        Self {
+            ptr: core::ptr::null(),
+        }
+    }
+
+    /// Returns `true` if the parameter is null/unset.
+    pub fn is_null(&self) -> bool {
+        self.ptr.is_null()
+    }
+
+    /// Returns the string as a `CStr` reference, if set.
+    pub fn as_cstr(&self) -> Option<&CStr> {
+        if self.ptr.is_null() {
+            None
+        } else {
+            // SAFETY: pointer validity is checked above
+            Some(unsafe { CStr::from_char_ptr(self.ptr) })
+        }
+    }
+
+    /// Returns the string as bytes, if set.
+    pub fn as_bytes(&self) -> Option<&[u8]> {
+        self.as_cstr().map(|s| s.to_bytes())
+    }
+}
+
+// SAFETY: The pointer is managed by the kernel and is effectively 'static
+// for the module's lifetime.
+unsafe impl Send for StringParam {}
+unsafe impl Sync for StringParam {}
+
 /// A wrapper for kernel parameters.
 ///
 /// This type is instantiated by the [`module!`] macro when module parameters are
-- 
2.52.0


^ permalink raw reply related

* [PATCH 0/8] rust: module parameter extensions
From: Matthew Wood @ 2026-02-26 23:47 UTC (permalink / raw)
  To: Miguel Ojeda, Luis Chamberlain, Petr Pavlu, Daniel Gomez,
	Sami Tolvanen
  Cc: Aaron Tomlin, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Danilo Krummrich, Tamir Duberstein, David Gow,
	José Expósito, linux-modules, rust-for-linux,
	linux-kernel

This series extends the Rust module! macro with three capabilities that
are available to C modules but currently missing from the Rust
abstractions: string parameters, early boot command-line parameters, and
configurable initcall levels.

The existing Rust module parameter infrastructure supports integer types
(i8..u64, isize, usize) but has no way to accept string values.
Additionally, built-in Rust modules cannot register parameters for early
boot command-line parsing (__setup / early_param), and all Rust modules
are hard-coded to initcall level 6 (device_initcall) with no way to
control initialization ordering.

String parameter support (patches 1-3):

  Introduce StringParam, a Copy wrapper around *const c_char whose
  memory is managed by the kernel parameter subsystem.  Wire it into
  the module! macro as the `string` parameter type with a dedicated
  kernel_param_ops (PARAM_OPS_STRING) that stores the pointer directly
  into a SetOnce<StringParam> container.  The rust_minimal sample is
  updated to demonstrate usage.

  An earlier implementation of string parameter support was proposed in:

      https://github.com/Rust-for-Linux/linux/pull/110/

  This series takes a different approach for StringParam by storing the
  raw C string pointer directly rather than copying the string data,
  since the kernel guarantees the backing memory remains valid for the
  module's lifetime.  This avoids allocation in the parameter setter and
  keeps StringParam as a simple Copy type consistent with the existing
  integer parameter design.

Early boot parameter support (patches 4-7):

  Add ObsKernelParam (mirroring the C obs_kernel_param), extend the
  ModuleParam trait with from_setup_arg() for constructing parameter
  values from raw __setup callback arguments, and integrate an optional
  `early_param` field into the module! macro.  When specified, the macro
  emits a __setup entry in the .init.setup ELF section, gated behind
  #[cfg(not(MODULE))] since this mechanism is only meaningful for
  built-in modules.

Configurable initcall levels (patch 8):

  Add an optional `initcall` field to the module! macro that accepts
  any of the eight standard levels (pure through late).  The default
  remains level 6 (device) so existing modules are unaffected.

Matthew Wood (8):
  rust: module_param: add StringParam type for C string parameters
  rust: module_param: wire StringParam into the module! macro
  samples: rust_minimal: demonstrate string module parameter
  rust: module_param: add ObsKernelParam type
  rust: module_param: add from_setup_arg() to ModuleParam trait
  rust: macros: add early_param support to module! macro
  samples: rust_minimal: demonstrate early_param usage
  rust: macros: add configurable initcall levels to module! macro

 rust/kernel/module_param.rs  | 160 +++++++++++++++++++++++++++++++
 rust/macros/lib.rs           |   9 ++
 rust/macros/module.rs        | 179 +++++++++++++++++++++++++++++++++--
 samples/rust/rust_minimal.rs |  33 ++++++-
 4 files changed, 368 insertions(+), 13 deletions(-)

-- 
2.52.0


^ permalink raw reply

* Re: IMA and PQC
From: Stefan Berger @ 2026-02-26 21:05 UTC (permalink / raw)
  To: Eric Biggers
  Cc: Simo Sorce, Coiby Xu, Johannes Wiesböck, dhowells,
	dmitry.kasatkin, eric.snowberg, keyrings, linux-crypto,
	linux-integrity, linux-kernel, linux-modules, roberto.sassu,
	zohar, michael.weiss
In-Reply-To: <20260226194406.GG2251@sol>



On 2/26/26 2:44 PM, Eric Biggers wrote:
> On Thu, Feb 26, 2026 at 02:21:41PM -0500, Stefan Berger wrote:
>>
>>
>> On 2/26/26 1:32 PM, Eric Biggers wrote:
>>> On Thu, Feb 26, 2026 at 12:22:32PM -0500, Stefan Berger wrote:
>>>>> I see that IMA indeed never upgraded full file hashes to use
>>>>> 'struct ima_file_id'.  Building a new feature that relies on this seems
>>>>> like a bad idea though, given that it's a security bug that makes the> IMA
>>>> protocol cryptographically ambiguous.  I.e., it means that in IMA,
>>>>> when the contents of some file are signed, that signature is sometimes
>>>>> also valid for some other file contents which the signer didn't intend.
>>>>
>>>> You mean IMA should not sign the digest in the ima_file_id structure but
>>>> hash the ima_file_id structure in which this file digest is written into
>>>> (that we currently sign) and sign/verify this digest? And we would do this
>>>> to avoid two different files (with presumably different content) from having
>>>> the same hashes leading to the same signature? Which hashes (besides the
>>>> non-recommended ones) are so weak now that you must not merely sign a file's
>>>> hash?
>>>>
>>>> The problem with this is that older kernels (without patching) won't be able
>>>> to handle newer signatures.
>>>
>>> IMA needs to sign the entire ima_file_id structure, which is indeed what
>>> IMA already does when it uses that structure.  (Well, actually it signs
>>> a hash of the struct, but that's best thought of an implementation
>>> detail of legacy signature algorithms that can only sign hashes.  For a
>>> modern algorithm the whole struct should be passed instead.)  Just IMA
>>> uses that structure only for fsverity hashes, which is a bug that makes
>>> the IMA protocol ambiguous.  It needs to use ima_file_id consistently,
>>> otherwise a signed message sometimes corresponds to multiple unique file
>>> contents even without a break in the cryptographic hash function.
>>
>> Before we jump into making changes on this old stuff I think it's good to
>> understand the underlying problem and the likelyhood of signatures
>> validating different data, such as a file and fsverity data. How likely is
>> this?
>>
>> Assuming a strong hash I suppose that is not a concern with RSA because here
>> the digest is padded and then directly encrypted with the private key. Upon
>> verification (pub key decrypt) we would unpad and memcmp the digests.
>>
>> Again, assuming a strong hash: With ECDSA NIST P256 for example we have a 32
>> byte signature. With a SHA512 being used for hashing for example we would be
>> doing a projection of a 64byte hash space to a 32byte signature space with.
>> Just by this projection of a much larger space into a smaller space
>> signatures that validate multiple input data could be a problem. One 'easy'
>> case where signatures for different input data is the same (not exactly the
>> same due to nonce involved the signature is verifyable), albeit unlikely, is
>> that there could be different input data for the SHA512 that lead to the
>> same 32bytes prefix, which is then used after truncating the sha512 to the
>> first 32 bytes for the ECDSA signature, and this then leads to a signature
>> that is verifyable for different input data. So that's the 'simple' case at
>> least for this thought experiment for a non-expert.
>>
>> Now what should still be difficult to do is given a file and a hash-to-use
>> that you can create fsverity content that leads to a hash that in turn leads
>> to a NIST-P256 signature that can be used for signature verification(s) of
>> the file and the totally different fsverity data. Is this a problem that is
>> as difficult to solve just as finding different input data for a hash that
>> leads to the same digest?
> 
> There's no differentiation between a 'struct ima_file_id' that
> *represents* the contents of some file, and a file whose contents are
> *equal to* that 'struct ima_file_id' and that uses a full-file hash.  In
> both cases the same key and message are used for signing and verifying.

I hadn't been thinking of this... It's a side-effect of starting to sign 
ima_file_id for v3 that a file *content* could now hold the ima_file_id 
structure (as signed with v3) and the signature would validate when used 
with the v2 signature verification scheme. So, the content of the file 
would presumably be odd/useless (2 bytes + hash) but it would verify 
with the signature created for v3. We will have to offer the possibility 
to move to v3 signatures for all signing schemes and offer the 
possibility to deactivate older versions (<v3).

> 
> This means that every time a file is signed using the ima_file_id
> scheme, it also implicitly signs some other file contents, which an
> attacker can freely replace the file with.  Similarly, every time a file
> that happens to be a valid ima_file_id is signed using the older scheme,
> it also implicitly signs the contents that the ima_file_id correspond
> to, which the attacker can freely replace the file with.  In either
> case, no collision in the cryptographic hash function is required.
> 
> It's simply a broken protocol.  To fix this, IMA must only support
> signatures that use the ima_file_id scheme.
> 
> Of course, that will require making them support full-file hashes and
> not just fsverity hashes.  If I recall correctly, this was actually part
> of the original design of the ima_file_id-based signatures.  It's
> unclear why the implementation is still incomplete.
> 
> - Eric


^ permalink raw reply

* Re: IMA and PQC
From: Eric Biggers @ 2026-02-26 19:44 UTC (permalink / raw)
  To: Stefan Berger
  Cc: Simo Sorce, Coiby Xu, Johannes Wiesböck, dhowells,
	dmitry.kasatkin, eric.snowberg, keyrings, linux-crypto,
	linux-integrity, linux-kernel, linux-modules, roberto.sassu,
	zohar, michael.weiss
In-Reply-To: <13ebe763-dcaf-4379-b9a7-82d06fd0fdb3@linux.ibm.com>

On Thu, Feb 26, 2026 at 02:21:41PM -0500, Stefan Berger wrote:
> 
> 
> On 2/26/26 1:32 PM, Eric Biggers wrote:
> > On Thu, Feb 26, 2026 at 12:22:32PM -0500, Stefan Berger wrote:
> > > > I see that IMA indeed never upgraded full file hashes to use
> > > > 'struct ima_file_id'.  Building a new feature that relies on this seems
> > > > like a bad idea though, given that it's a security bug that makes the> IMA
> > > protocol cryptographically ambiguous.  I.e., it means that in IMA,
> > > > when the contents of some file are signed, that signature is sometimes
> > > > also valid for some other file contents which the signer didn't intend.
> > > 
> > > You mean IMA should not sign the digest in the ima_file_id structure but
> > > hash the ima_file_id structure in which this file digest is written into
> > > (that we currently sign) and sign/verify this digest? And we would do this
> > > to avoid two different files (with presumably different content) from having
> > > the same hashes leading to the same signature? Which hashes (besides the
> > > non-recommended ones) are so weak now that you must not merely sign a file's
> > > hash?
> > > 
> > > The problem with this is that older kernels (without patching) won't be able
> > > to handle newer signatures.
> > 
> > IMA needs to sign the entire ima_file_id structure, which is indeed what
> > IMA already does when it uses that structure.  (Well, actually it signs
> > a hash of the struct, but that's best thought of an implementation
> > detail of legacy signature algorithms that can only sign hashes.  For a
> > modern algorithm the whole struct should be passed instead.)  Just IMA
> > uses that structure only for fsverity hashes, which is a bug that makes
> > the IMA protocol ambiguous.  It needs to use ima_file_id consistently,
> > otherwise a signed message sometimes corresponds to multiple unique file
> > contents even without a break in the cryptographic hash function.
> 
> Before we jump into making changes on this old stuff I think it's good to
> understand the underlying problem and the likelyhood of signatures
> validating different data, such as a file and fsverity data. How likely is
> this?
> 
> Assuming a strong hash I suppose that is not a concern with RSA because here
> the digest is padded and then directly encrypted with the private key. Upon
> verification (pub key decrypt) we would unpad and memcmp the digests.
> 
> Again, assuming a strong hash: With ECDSA NIST P256 for example we have a 32
> byte signature. With a SHA512 being used for hashing for example we would be
> doing a projection of a 64byte hash space to a 32byte signature space with.
> Just by this projection of a much larger space into a smaller space
> signatures that validate multiple input data could be a problem. One 'easy'
> case where signatures for different input data is the same (not exactly the
> same due to nonce involved the signature is verifyable), albeit unlikely, is
> that there could be different input data for the SHA512 that lead to the
> same 32bytes prefix, which is then used after truncating the sha512 to the
> first 32 bytes for the ECDSA signature, and this then leads to a signature
> that is verifyable for different input data. So that's the 'simple' case at
> least for this thought experiment for a non-expert.
> 
> Now what should still be difficult to do is given a file and a hash-to-use
> that you can create fsverity content that leads to a hash that in turn leads
> to a NIST-P256 signature that can be used for signature verification(s) of
> the file and the totally different fsverity data. Is this a problem that is
> as difficult to solve just as finding different input data for a hash that
> leads to the same digest?

There's no differentiation between a 'struct ima_file_id' that
*represents* the contents of some file, and a file whose contents are
*equal to* that 'struct ima_file_id' and that uses a full-file hash.  In
both cases the same key and message are used for signing and verifying.

This means that every time a file is signed using the ima_file_id
scheme, it also implicitly signs some other file contents, which an
attacker can freely replace the file with.  Similarly, every time a file
that happens to be a valid ima_file_id is signed using the older scheme,
it also implicitly signs the contents that the ima_file_id correspond
to, which the attacker can freely replace the file with.  In either
case, no collision in the cryptographic hash function is required.

It's simply a broken protocol.  To fix this, IMA must only support
signatures that use the ima_file_id scheme.

Of course, that will require making them support full-file hashes and
not just fsverity hashes.  If I recall correctly, this was actually part
of the original design of the ima_file_id-based signatures.  It's
unclear why the implementation is still incomplete.

- Eric

^ permalink raw reply

* Re: [PATCH v4 2/2] lib/crypto: tests: Add KUnit tests for ML-DSA verification
From: Eric Biggers @ 2026-02-26 19:21 UTC (permalink / raw)
  To: Geert Uytterhoeven, Brendan Higgins, David Gow
  Cc: linux-crypto, David Howells, Herbert Xu, Luis Chamberlain,
	Petr Pavlu, Daniel Gomez, Sami Tolvanen, Jason A . Donenfeld,
	Ard Biesheuvel, Stephan Mueller, Lukas Wunner, Ignat Korchagin,
	keyrings, linux-modules, linux-kernel,
	open list:KERNEL SELFTEST FRAMEWORK, KUnit Development
In-Reply-To: <20260226180538.GC2251@sol>

On Thu, Feb 26, 2026 at 10:05:38AM -0800, Eric Biggers wrote:
> On Thu, Feb 26, 2026 at 02:09:47PM +0100, Geert Uytterhoeven wrote:
> > On Sun, 14 Dec 2025 at 19:18, Eric Biggers <ebiggers@kernel.org> wrote:
> > > Add a KUnit test suite for ML-DSA verification, including the following
> > > for each ML-DSA parameter set (ML-DSA-44, ML-DSA-65, and ML-DSA-87):
> > >
> > > - Positive test (valid signature), using vector imported from leancrypto
> > > - Various negative tests:
> > >     - Wrong length for signature, message, or public key
> > >     - Out-of-range coefficients in z vector
> > >     - Invalid encoded hint vector
> > >     - Any bit flipped in signature, message, or public key
> > > - Unit test for the internal function use_hint()
> > > - A benchmark
> > >
> > > ML-DSA inputs and outputs are very large.  To keep the size of the tests
> > > down, use just one valid test vector per parameter set, and generate the
> > > negative tests at runtime by mutating the valid test vector.
> > >
> > > I also considered importing the test vectors from Wycheproof.  I've
> > > tested that mldsa_verify() indeed passes all of Wycheproof's ML-DSA test
> > > vectors that use an empty context string.  However, importing these
> > > permanently would add over 6 MB of source.  That's too much to be a
> > > reasonable addition to the Linux kernel tree for one algorithm.  It also
> > > wouldn't actually provide much better test coverage than this commit.
> > > Another potential issue is that Wycheproof uses the Apache license.
> > >
> > > Similarly, this also differs from the earlier proposal to import a long
> > > list of test vectors from leancrypto.  I retained only one valid
> > > signature for each algorithm, and I also added (runtime-generated)
> > > negative tests which were missing.  I think this is a better tradeoff.
> > >
> > > Reviewed-by: David Howells <dhowells@redhat.com>
> > > Tested-by: David Howells <dhowells@redhat.com>
> > > Signed-off-by: Eric Biggers <ebiggers@kernel.org>
> > 
> > Thanks for your patch, which is now commit ed894faccb8de55c
> > ("lib/crypto: tests: Add KUnit tests for ML-DSA verification")
> > in v7.0-rc1.
> > 
> > > --- a/lib/crypto/tests/Kconfig
> > > +++ b/lib/crypto/tests/Kconfig
> > > @@ -36,10 +36,19 @@ config CRYPTO_LIB_MD5_KUNIT_TEST
> > >         select CRYPTO_LIB_MD5
> > >         help
> > >           KUnit tests for the MD5 cryptographic hash function and its
> > >           corresponding HMAC.
> > >
> > > +config CRYPTO_LIB_MLDSA_KUNIT_TEST
> > > +       tristate "KUnit tests for ML-DSA" if !KUNIT_ALL_TESTS
> > > +       depends on KUNIT
> > > +       default KUNIT_ALL_TESTS || CRYPTO_SELFTESTS
> > > +       select CRYPTO_LIB_BENCHMARK_VISIBLE
> > > +       select CRYPTO_LIB_MLDSA
> > 
> > These two selects mean that enabling KUNIT_ALL_TESTS also enables
> > extra functionality, which may not be desirable in a production system.
> > Fortunately CRYPTO_LIB_MLDSA is tristate, so in the modular case
> > the extra functionality is a module, too, and not part of the running system
> > by default.  Unfortunately CRYPTO_LIB_MLDSA is invisible, so this cannot
> > just be changed from "select" to "depends on". But as CRYPTO_MLDSA
> > also selects it, perhaps the test can be made dependent on CRYPTO_MLDSA?
> 
> "depends on CRYPTO_MLDSA" doesn't make sense, since the test is for the
> code in CRYPTO_LIB_MLDSA, not CRYPTO_MLDSA.  CRYPTO_MLDSA just happens
> to be one of the users of CRYPTO_LIB_MLDSA.  In this case the names
> happen to be similar, but consider e.g. CRYPTO_LIB_AESGCM which is
> selected by AMD_MEM_ENCRYPT.  If we added a test for CRYPTO_LIB_AESGCM,
> it clearly shouldn't use "depends on AMD_MEM_ENCRYPT".
> 
> So, "depends on CRYPTO_LIB_MLDSA" would be the correct way to switch it
> from a selection to a dependency.
> 
> It's just a bit annoying to do this for hidden symbols, given that it
> makes it so that anyone who wants to unconditionally enable the test,
> like what I do to test all the crypto library code, has to find and
> enable some other random kconfig symbol that enables the code.
> 
> Also, the series that originally added CRYPTO_LIB_MLDSA and its test
> (https://lore.kernel.org/linux-crypto/20251214181712.29132-1-ebiggers@kernel.org/)
> didn't add any user of CRYPTO_LIB_MLDSA besides the test, as the real
> user came a bit later.  So if I had used "depends on CRYPTO_LIB_MLDSA",
> my series wouldn't have received any build bot coverage, and I'd have
> needed to temporarily carry local patches to build and test the code.
> 
> But if this is really the convention for KUnit, as it seems to be, I
> will follow it and work around it for my own testing.  So I'll plan to
> change the crypto library and CRC tests to use "depends on".
> 
> But any thoughts from the KUnit maintainers would also be appreciated.
> Is it indeed intended that the tests for library modules depend on those
> modules rather than selecting them, despite their symbols being hidden?

I sent the following patch to consider:
https://lore.kernel.org/linux-crypto/20260226191749.39397-1-ebiggers@kernel.org/

- Eric

^ permalink raw reply

* Re: IMA and PQC
From: Stefan Berger @ 2026-02-26 19:21 UTC (permalink / raw)
  To: Eric Biggers
  Cc: Simo Sorce, Coiby Xu, Johannes Wiesböck, dhowells,
	dmitry.kasatkin, eric.snowberg, keyrings, linux-crypto,
	linux-integrity, linux-kernel, linux-modules, roberto.sassu,
	zohar, michael.weiss
In-Reply-To: <20260226183248.GE2251@sol>



On 2/26/26 1:32 PM, Eric Biggers wrote:
> On Thu, Feb 26, 2026 at 12:22:32PM -0500, Stefan Berger wrote:
>>> I see that IMA indeed never upgraded full file hashes to use
>>> 'struct ima_file_id'.  Building a new feature that relies on this seems
>>> like a bad idea though, given that it's a security bug that makes the> IMA
>> protocol cryptographically ambiguous.  I.e., it means that in IMA,
>>> when the contents of some file are signed, that signature is sometimes
>>> also valid for some other file contents which the signer didn't intend.
>>
>> You mean IMA should not sign the digest in the ima_file_id structure but
>> hash the ima_file_id structure in which this file digest is written into
>> (that we currently sign) and sign/verify this digest? And we would do this
>> to avoid two different files (with presumably different content) from having
>> the same hashes leading to the same signature? Which hashes (besides the
>> non-recommended ones) are so weak now that you must not merely sign a file's
>> hash?
>>
>> The problem with this is that older kernels (without patching) won't be able
>> to handle newer signatures.
> 
> IMA needs to sign the entire ima_file_id structure, which is indeed what
> IMA already does when it uses that structure.  (Well, actually it signs
> a hash of the struct, but that's best thought of an implementation
> detail of legacy signature algorithms that can only sign hashes.  For a
> modern algorithm the whole struct should be passed instead.)  Just IMA
> uses that structure only for fsverity hashes, which is a bug that makes
> the IMA protocol ambiguous.  It needs to use ima_file_id consistently,
> otherwise a signed message sometimes corresponds to multiple unique file
> contents even without a break in the cryptographic hash function.

Before we jump into making changes on this old stuff I think it's good 
to understand the underlying problem and the likelyhood of signatures 
validating different data, such as a file and fsverity data. How likely 
is this?

Assuming a strong hash I suppose that is not a concern with RSA because 
here the digest is padded and then directly encrypted with the private 
key. Upon verification (pub key decrypt) we would unpad and memcmp the 
digests.

Again, assuming a strong hash: With ECDSA NIST P256 for example we have 
a 32 byte signature. With a SHA512 being used for hashing for example we 
would be doing a projection of a 64byte hash space to a 32byte signature 
space with. Just by this projection of a much larger space into a 
smaller space signatures that validate multiple input data could be a 
problem. One 'easy' case where signatures for different input data is 
the same (not exactly the same due to nonce involved the signature is 
verifyable), albeit unlikely, is that there could be different input 
data for the SHA512 that lead to the same 32bytes prefix, which is then 
used after truncating the sha512 to the first 32 bytes for the ECDSA 
signature, and this then leads to a signature that is verifyable for 
different input data. So that's the 'simple' case at least for this 
thought experiment for a non-expert.

Now what should still be difficult to do is given a file and a 
hash-to-use that you can create fsverity content that leads to a hash 
that in turn leads to a NIST-P256 signature that can be used for 
signature verification(s) of the file and the totally different fsverity 
data. Is this a problem that is as difficult to solve just as finding 
different input data for a hash that leads to the same digest?

> 
> Sure, when that bug is fixed, old kernels won't support the new
> signatures for files that use a full-file hash.  But the same applies to
> starting to use a new signature algorithm, such as ML-DSA.
> 
> - Eric
> 


^ permalink raw reply

* Re: IMA and PQC
From: Simo Sorce @ 2026-02-26 18:42 UTC (permalink / raw)
  To: Stefan Berger, Eric Biggers
  Cc: Coiby Xu, Johannes Wiesböck, dhowells, dmitry.kasatkin,
	eric.snowberg, keyrings, linux-crypto, linux-integrity,
	linux-kernel, linux-modules, roberto.sassu, zohar, michael.weiss
In-Reply-To: <969c74f3-81ed-442c-87dd-381274a642a7@linux.ibm.com>

On Thu, 2026-02-26 at 12:22 -0500, Stefan Berger wrote:
> 
> On 2/26/26 11:58 AM, Eric Biggers wrote:
> > On Thu, Feb 26, 2026 at 10:27:43AM -0500, Simo Sorce wrote:
> > > On Thu, 2026-02-26 at 09:16 -0500, Stefan Berger wrote:
> > > > On 2/26/26 7:42 AM, Stefan Berger wrote:
> > > > > On 2/25/26 7:10 PM, Eric Biggers wrote:
> > > > > > On Wed, Feb 25, 2026 at 09:25:43AM -0500, Stefan Berger wrote:
> > > > > > > To avoid duplicate work: Is either one of you planning on writing
> > > > > > > patches
> > > > > > > for IMA to use ML-DSA and convert the current ML-DSA to also support
> > > > > > > HashML?
> > > > > > > I had done the work on this before and could dig out the patches
> > > > > > > again...
> > > > > > 
> > > > > > IMA already had to add its own digest prefixing support, since it was
> > > > > > needed to disambiguate between full-file digests and fsverity digests.
> > > > > > See 'struct ima_file_id'.  Thus the message signed is at most 66 bytes.
> > > > > 
> > > > > The hash there is still only a hash over a file and that hash is signed,
> > > > > isn't it?
> > > > > 
> > > > > > 
> > > > > > With that being the case, HashML-DSA isn't necessary.  It's not even
> > > > > > possible to use here, since there are no OIDs assigned for the fsverity
> > > > > > digests, so it cannot replace the ima_file_id.
> > > > > 
> > > > > For non-fsverify IMA signatures it is 'possible' to use HashML-DSA and
> > > > > it's 'working' (recycled old patches yesterday):
> > > > > 
> > > > > Linux: https://github.com/stefanberger/linux/commits/
> > > > > dhmlsa%2Bima.202602025/
> > > > > 
> > > > > ima-evm-utils: https://github.com/linux-integrity/ima-evm-utils/pull/19/
> > > > > commits
> > > > > 
> > > > > > 
> > > > > > I'll also note that HashML-DSA is controversial (e.g. see
> > > > > > https://keymaterial.net/2024/11/05/hashml-dsa-considered-harmful/),
> > > > > 
> > > > > The problem with this is that NIST would have to react to these
> > > > > controversies as we race to support PQC. If something is wrong with the
> > > > > standard then it would be best for NIST to withdraw/modify HashML-DSA
> > > > > asap. Otherwise it's the best to follow the standard IMO because if you
> > > > > don't you get criticism otherwise.
> > > > 
> > > > What I am not clear about from FIPS-204 is whether availability of
> > > > HashML-DSA is a "must-use" or  a "may-use". What speaks against it for
> > > > our use case is performance. The lookup of a hash's ID (last digit of
> > > > OID) and the creation of the 11 byte encoding to prepend before every
> > > > digest for every signature takes cycles.
> > > 
> > > It is a recommendation, but there are plenty of protocols (TLS,
> > > OpenPGP, etc...) where the decision has been made to use "pure" ML-DSA
> > > only, even if what you are signing is not the full data, but something
> > > containing a hash.
> > > 
> > > Ideally you do not sign *just* a hash, but some structured data, like a
> > > context label that identifies the hash and some other related metadata
> > > for example. In order to make forgeries much harder should the hashing
> > > algorithm used to hash the data weaken over time. But it is not
> > > strictly necessary (NIST mentioned in some forum, sorry I do not have
> > > the message handy for quoting, that a structured packet is perfectly
> > > fine for use with pure ML-DSA, because it does enough to address the
> > > same issues that a separate internal context does with HashML-DSA).
> > > 
> > > If pure-ML-DSA works better for IMA, just use pure ML-DSA.
> > > 
> > > > Maybe it should explicitly state in FIPS-204 something along the lines
> > > > of "with a given hash either ML-DSA or HashML-DSA can be used (for as
> > > > long as you use it in the same way from then on)." At least this way
> > > > nobody can point out that HashML-DSA should have been used when you didn't.
> > > 
> > > NIST will not change the standard documents any time soon, but for FIPS
> > > certification there are Implementation Guidelines.
> > > 
> > > In any case a FIPS module cannot distinguish between data that happens
> > > to be 32 bytes long and a hash of larger data, so the point is kind of
> > > moot. From the FIPS perspective HashML-DSA is just an available
> > > algorithm that protocol implementations can use, or not.
> > > 
> > > There are additional guidelines on what this may be useful for, but so
> > > far NIST has not objected to the use of pure ML-DSA even where
> > > theoretically HashML-DSA could be used.
> > 
> > I see that IMA indeed never upgraded full file hashes to use
> > 'struct ima_file_id'.  Building a new feature that relies on this seems
>  > like a bad idea though, given that it's a security bug that makes 
> the> IMA protocol cryptographically ambiguous.  I.e., it means that in IMA,
> > when the contents of some file are signed, that signature is sometimes
> > also valid for some other file contents which the signer didn't intend.
> 
> You mean IMA should not sign the digest in the ima_file_id structure but 
> hash the ima_file_id structure in which this file digest is written into 
> (that we currently sign) and sign/verify this digest?

You should not (need to) hash it, just format it and the use ML-DSA to
sign it.

> And we would do 
> this to avoid two different files (with presumably different content) 
> from having the same hashes leading to the same signature? Which hashes 
> (besides the non-recommended ones) are so weak now that you must not 
> merely sign a file's hash?
> 
> The problem with this is that older kernels (without patching) won't be 
> able to handle newer signatures.

Ad a kernel option to use the new format? Old kernels won't be able to
deal with ML-DSA IMA either.

> > 
> > Just fix that bug first, which has to be done anyway.  Then just use
> > pure ML-DSA to sign and verify the 'struct ima_file_id'.
>  > > As Simo mentioned, FIPS 204 doesn't require HashML-DSA when signing a
> > hash.  It's there as an *option* to solve a perceived problem, which is
> > actually solvable in better ways.
> > 
> > NIST doesn't plan to update FIPS 204 until 2029, and most likely the
> > updates will just be errata in the text (such as the ones I reported to
> > them), not changes or withdrawals in the algorithms themselves.  But
> > it's irrelevant: just because HashML-DSA is an option doesn't mean it
> > has to be used.  Pure ML-DSA supports arbitrary data, which includes
> 
> And I was sure whether it was merely an 'option'. Who would use it then 
> if it takes more cycles to hash the prepended 11 byte oid in HashML-DSA?

Nobody is using HashML-DSA at the moment.

-- 
Simo Sorce
Distinguished Engineer
RHEL Crypto Team
Red Hat, Inc


^ permalink raw reply

* Re: [PATCH v2] module: print version for external modules in print_modules()
From: Sami Tolvanen @ 2026-02-26 18:39 UTC (permalink / raw)
  To: Yafang Shao; +Cc: mcgrof, petr.pavlu, da.gomez, atomlin, linux-modules
In-Reply-To: <CALOAHbA5fkK8XSqo62=k1hVX+krx58eUWMLNwMdwncAp+Xn4=w@mail.gmail.com>

On Wed, Feb 25, 2026 at 6:19 PM Yafang Shao <laoar.shao@gmail.com> wrote:
> Just checking in on this patch. It looks like it hasn't been merged
> yet. Is it good to go, or does it need something else?

The patch looks reasonable to me. I have it in my queue for v7.1.

Sami

^ permalink raw reply

* Re: IMA and PQC
From: Eric Biggers @ 2026-02-26 18:32 UTC (permalink / raw)
  To: Stefan Berger
  Cc: Simo Sorce, Coiby Xu, Johannes Wiesböck, dhowells,
	dmitry.kasatkin, eric.snowberg, keyrings, linux-crypto,
	linux-integrity, linux-kernel, linux-modules, roberto.sassu,
	zohar, michael.weiss
In-Reply-To: <969c74f3-81ed-442c-87dd-381274a642a7@linux.ibm.com>

On Thu, Feb 26, 2026 at 12:22:32PM -0500, Stefan Berger wrote:
> > I see that IMA indeed never upgraded full file hashes to use
> > 'struct ima_file_id'.  Building a new feature that relies on this seems
> > like a bad idea though, given that it's a security bug that makes the> IMA
> protocol cryptographically ambiguous.  I.e., it means that in IMA,
> > when the contents of some file are signed, that signature is sometimes
> > also valid for some other file contents which the signer didn't intend.
> 
> You mean IMA should not sign the digest in the ima_file_id structure but
> hash the ima_file_id structure in which this file digest is written into
> (that we currently sign) and sign/verify this digest? And we would do this
> to avoid two different files (with presumably different content) from having
> the same hashes leading to the same signature? Which hashes (besides the
> non-recommended ones) are so weak now that you must not merely sign a file's
> hash?
> 
> The problem with this is that older kernels (without patching) won't be able
> to handle newer signatures.

IMA needs to sign the entire ima_file_id structure, which is indeed what
IMA already does when it uses that structure.  (Well, actually it signs
a hash of the struct, but that's best thought of an implementation
detail of legacy signature algorithms that can only sign hashes.  For a
modern algorithm the whole struct should be passed instead.)  Just IMA
uses that structure only for fsverity hashes, which is a bug that makes
the IMA protocol ambiguous.  It needs to use ima_file_id consistently,
otherwise a signed message sometimes corresponds to multiple unique file
contents even without a break in the cryptographic hash function.

Sure, when that bug is fixed, old kernels won't support the new
signatures for files that use a full-file hash.  But the same applies to
starting to use a new signature algorithm, such as ML-DSA.

- Eric

^ permalink raw reply

* Re: [PATCH v4 2/2] lib/crypto: tests: Add KUnit tests for ML-DSA verification
From: Eric Biggers @ 2026-02-26 18:05 UTC (permalink / raw)
  To: Geert Uytterhoeven, Brendan Higgins, David Gow
  Cc: linux-crypto, David Howells, Herbert Xu, Luis Chamberlain,
	Petr Pavlu, Daniel Gomez, Sami Tolvanen, Jason A . Donenfeld,
	Ard Biesheuvel, Stephan Mueller, Lukas Wunner, Ignat Korchagin,
	keyrings, linux-modules, linux-kernel,
	open list:KERNEL SELFTEST FRAMEWORK, KUnit Development
In-Reply-To: <CAMuHMdULzMdxuTVfg8_4jdgzbzjfx-PHkcgbGSthcUx_sHRNMg@mail.gmail.com>

On Thu, Feb 26, 2026 at 02:09:47PM +0100, Geert Uytterhoeven wrote:
> On Sun, 14 Dec 2025 at 19:18, Eric Biggers <ebiggers@kernel.org> wrote:
> > Add a KUnit test suite for ML-DSA verification, including the following
> > for each ML-DSA parameter set (ML-DSA-44, ML-DSA-65, and ML-DSA-87):
> >
> > - Positive test (valid signature), using vector imported from leancrypto
> > - Various negative tests:
> >     - Wrong length for signature, message, or public key
> >     - Out-of-range coefficients in z vector
> >     - Invalid encoded hint vector
> >     - Any bit flipped in signature, message, or public key
> > - Unit test for the internal function use_hint()
> > - A benchmark
> >
> > ML-DSA inputs and outputs are very large.  To keep the size of the tests
> > down, use just one valid test vector per parameter set, and generate the
> > negative tests at runtime by mutating the valid test vector.
> >
> > I also considered importing the test vectors from Wycheproof.  I've
> > tested that mldsa_verify() indeed passes all of Wycheproof's ML-DSA test
> > vectors that use an empty context string.  However, importing these
> > permanently would add over 6 MB of source.  That's too much to be a
> > reasonable addition to the Linux kernel tree for one algorithm.  It also
> > wouldn't actually provide much better test coverage than this commit.
> > Another potential issue is that Wycheproof uses the Apache license.
> >
> > Similarly, this also differs from the earlier proposal to import a long
> > list of test vectors from leancrypto.  I retained only one valid
> > signature for each algorithm, and I also added (runtime-generated)
> > negative tests which were missing.  I think this is a better tradeoff.
> >
> > Reviewed-by: David Howells <dhowells@redhat.com>
> > Tested-by: David Howells <dhowells@redhat.com>
> > Signed-off-by: Eric Biggers <ebiggers@kernel.org>
> 
> Thanks for your patch, which is now commit ed894faccb8de55c
> ("lib/crypto: tests: Add KUnit tests for ML-DSA verification")
> in v7.0-rc1.
> 
> > --- a/lib/crypto/tests/Kconfig
> > +++ b/lib/crypto/tests/Kconfig
> > @@ -36,10 +36,19 @@ config CRYPTO_LIB_MD5_KUNIT_TEST
> >         select CRYPTO_LIB_MD5
> >         help
> >           KUnit tests for the MD5 cryptographic hash function and its
> >           corresponding HMAC.
> >
> > +config CRYPTO_LIB_MLDSA_KUNIT_TEST
> > +       tristate "KUnit tests for ML-DSA" if !KUNIT_ALL_TESTS
> > +       depends on KUNIT
> > +       default KUNIT_ALL_TESTS || CRYPTO_SELFTESTS
> > +       select CRYPTO_LIB_BENCHMARK_VISIBLE
> > +       select CRYPTO_LIB_MLDSA
> 
> These two selects mean that enabling KUNIT_ALL_TESTS also enables
> extra functionality, which may not be desirable in a production system.
> Fortunately CRYPTO_LIB_MLDSA is tristate, so in the modular case
> the extra functionality is a module, too, and not part of the running system
> by default.  Unfortunately CRYPTO_LIB_MLDSA is invisible, so this cannot
> just be changed from "select" to "depends on". But as CRYPTO_MLDSA
> also selects it, perhaps the test can be made dependent on CRYPTO_MLDSA?

"depends on CRYPTO_MLDSA" doesn't make sense, since the test is for the
code in CRYPTO_LIB_MLDSA, not CRYPTO_MLDSA.  CRYPTO_MLDSA just happens
to be one of the users of CRYPTO_LIB_MLDSA.  In this case the names
happen to be similar, but consider e.g. CRYPTO_LIB_AESGCM which is
selected by AMD_MEM_ENCRYPT.  If we added a test for CRYPTO_LIB_AESGCM,
it clearly shouldn't use "depends on AMD_MEM_ENCRYPT".

So, "depends on CRYPTO_LIB_MLDSA" would be the correct way to switch it
from a selection to a dependency.

It's just a bit annoying to do this for hidden symbols, given that it
makes it so that anyone who wants to unconditionally enable the test,
like what I do to test all the crypto library code, has to find and
enable some other random kconfig symbol that enables the code.

Also, the series that originally added CRYPTO_LIB_MLDSA and its test
(https://lore.kernel.org/linux-crypto/20251214181712.29132-1-ebiggers@kernel.org/)
didn't add any user of CRYPTO_LIB_MLDSA besides the test, as the real
user came a bit later.  So if I had used "depends on CRYPTO_LIB_MLDSA",
my series wouldn't have received any build bot coverage, and I'd have
needed to temporarily carry local patches to build and test the code.

But if this is really the convention for KUnit, as it seems to be, I
will follow it and work around it for my own testing.  So I'll plan to
change the crypto library and CRC tests to use "depends on".

But any thoughts from the KUnit maintainers would also be appreciated.
Is it indeed intended that the tests for library modules depend on those
modules rather than selecting them, despite their symbols being hidden?

- Eric

^ permalink raw reply

* Re: [PATCH] kernel/trace/ftrace: introduce ftrace module notifier
From: Steven Rostedt @ 2026-02-26 17:30 UTC (permalink / raw)
  To: Miroslav Benes
  Cc: Song Chen, mcgrof, petr.pavlu, da.gomez, samitolvanen, atomlin,
	mhiramat, mark.rutland, mathieu.desnoyers, linux-modules,
	linux-kernel, linux-trace-kernel, live-patching
In-Reply-To: <alpine.LSU.2.21.2602261147150.5739@pobox.suse.cz>

On Thu, 26 Feb 2026 11:51:53 +0100 (CET)
Miroslav Benes <mbenes@suse.cz> wrote:

> > Let me see if there is any way to use notifier and remain below calling
> > sequence:
> > 
> > ftrace_module_enable
> > klp_module_coming
> > blocking_notifier_call_chain_robust(MODULE_STATE_COMING)
> > 
> > blocking_notifier_call_chain(MODULE_STATE_GOING)
> > klp_module_going
> > ftrace_release_mod  
> 
> Both klp and ftrace used module notifiers in the past. We abandoned that 
> and opted for direct calls due to issues with ordering at the time. I do 
> not have the list of problems at hand but I remember it was very fragile.
> 
> See commits 7dcd182bec27 ("ftrace/module: remove ftrace module 
> notifier"), 7e545d6eca20 ("livepatch/module: remove livepatch module 
> notifier") and their surroundings.
> 
> So unless there is a reason for the change (which should be then carefully 
> reviewed and properly tested), I would prefer to keep it as is. What is 
> the motivation? I am failing to find it in the commit log.

Honestly, I do think just decoupling ftrace and live kernel patching from
modules is rationale enough, as it makes the code a bit cleaner. But to do
so, we really need to make sure there is absolutely no regressions.

Thus, to allow such a change, I would ask those that are proposing it, show
a full work flow of how ftrace, live kernel patching, and modules work with
each other and why those functions are currently injected in the module code.

As Miroslav stated, we tried to do it via notifiers in the past and it
failed. I don't want to find out why they failed by just adding them back
to notifiers again. Instead, the reasons must be fully understood and
updates made to make sure they will not fail in the future.

-- Steve

^ permalink raw reply

* Re: IMA and PQC
From: Stefan Berger @ 2026-02-26 17:22 UTC (permalink / raw)
  To: Eric Biggers, Simo Sorce
  Cc: Coiby Xu, Johannes Wiesböck, dhowells, dmitry.kasatkin,
	eric.snowberg, keyrings, linux-crypto, linux-integrity,
	linux-kernel, linux-modules, roberto.sassu, zohar, michael.weiss
In-Reply-To: <20260226165819.GA2251@sol>



On 2/26/26 11:58 AM, Eric Biggers wrote:
> On Thu, Feb 26, 2026 at 10:27:43AM -0500, Simo Sorce wrote:
>> On Thu, 2026-02-26 at 09:16 -0500, Stefan Berger wrote:
>>> On 2/26/26 7:42 AM, Stefan Berger wrote:
>>>> On 2/25/26 7:10 PM, Eric Biggers wrote:
>>>>> On Wed, Feb 25, 2026 at 09:25:43AM -0500, Stefan Berger wrote:
>>>>>> To avoid duplicate work: Is either one of you planning on writing
>>>>>> patches
>>>>>> for IMA to use ML-DSA and convert the current ML-DSA to also support
>>>>>> HashML?
>>>>>> I had done the work on this before and could dig out the patches
>>>>>> again...
>>>>>
>>>>> IMA already had to add its own digest prefixing support, since it was
>>>>> needed to disambiguate between full-file digests and fsverity digests.
>>>>> See 'struct ima_file_id'.  Thus the message signed is at most 66 bytes.
>>>>
>>>> The hash there is still only a hash over a file and that hash is signed,
>>>> isn't it?
>>>>
>>>>>
>>>>> With that being the case, HashML-DSA isn't necessary.  It's not even
>>>>> possible to use here, since there are no OIDs assigned for the fsverity
>>>>> digests, so it cannot replace the ima_file_id.
>>>>
>>>> For non-fsverify IMA signatures it is 'possible' to use HashML-DSA and
>>>> it's 'working' (recycled old patches yesterday):
>>>>
>>>> Linux: https://github.com/stefanberger/linux/commits/
>>>> dhmlsa%2Bima.202602025/
>>>>
>>>> ima-evm-utils: https://github.com/linux-integrity/ima-evm-utils/pull/19/
>>>> commits
>>>>
>>>>>
>>>>> I'll also note that HashML-DSA is controversial (e.g. see
>>>>> https://keymaterial.net/2024/11/05/hashml-dsa-considered-harmful/),
>>>>
>>>> The problem with this is that NIST would have to react to these
>>>> controversies as we race to support PQC. If something is wrong with the
>>>> standard then it would be best for NIST to withdraw/modify HashML-DSA
>>>> asap. Otherwise it's the best to follow the standard IMO because if you
>>>> don't you get criticism otherwise.
>>>
>>> What I am not clear about from FIPS-204 is whether availability of
>>> HashML-DSA is a "must-use" or  a "may-use". What speaks against it for
>>> our use case is performance. The lookup of a hash's ID (last digit of
>>> OID) and the creation of the 11 byte encoding to prepend before every
>>> digest for every signature takes cycles.
>>
>> It is a recommendation, but there are plenty of protocols (TLS,
>> OpenPGP, etc...) where the decision has been made to use "pure" ML-DSA
>> only, even if what you are signing is not the full data, but something
>> containing a hash.
>>
>> Ideally you do not sign *just* a hash, but some structured data, like a
>> context label that identifies the hash and some other related metadata
>> for example. In order to make forgeries much harder should the hashing
>> algorithm used to hash the data weaken over time. But it is not
>> strictly necessary (NIST mentioned in some forum, sorry I do not have
>> the message handy for quoting, that a structured packet is perfectly
>> fine for use with pure ML-DSA, because it does enough to address the
>> same issues that a separate internal context does with HashML-DSA).
>>
>> If pure-ML-DSA works better for IMA, just use pure ML-DSA.
>>
>>> Maybe it should explicitly state in FIPS-204 something along the lines
>>> of "with a given hash either ML-DSA or HashML-DSA can be used (for as
>>> long as you use it in the same way from then on)." At least this way
>>> nobody can point out that HashML-DSA should have been used when you didn't.
>>
>> NIST will not change the standard documents any time soon, but for FIPS
>> certification there are Implementation Guidelines.
>>
>> In any case a FIPS module cannot distinguish between data that happens
>> to be 32 bytes long and a hash of larger data, so the point is kind of
>> moot. From the FIPS perspective HashML-DSA is just an available
>> algorithm that protocol implementations can use, or not.
>>
>> There are additional guidelines on what this may be useful for, but so
>> far NIST has not objected to the use of pure ML-DSA even where
>> theoretically HashML-DSA could be used.
> 
> I see that IMA indeed never upgraded full file hashes to use
> 'struct ima_file_id'.  Building a new feature that relies on this seems
 > like a bad idea though, given that it's a security bug that makes 
the> IMA protocol cryptographically ambiguous.  I.e., it means that in IMA,
> when the contents of some file are signed, that signature is sometimes
> also valid for some other file contents which the signer didn't intend.

You mean IMA should not sign the digest in the ima_file_id structure but 
hash the ima_file_id structure in which this file digest is written into 
(that we currently sign) and sign/verify this digest? And we would do 
this to avoid two different files (with presumably different content) 
from having the same hashes leading to the same signature? Which hashes 
(besides the non-recommended ones) are so weak now that you must not 
merely sign a file's hash?

The problem with this is that older kernels (without patching) won't be 
able to handle newer signatures.

> 
> Just fix that bug first, which has to be done anyway.  Then just use
> pure ML-DSA to sign and verify the 'struct ima_file_id'.
 > > As Simo mentioned, FIPS 204 doesn't require HashML-DSA when signing a
> hash.  It's there as an *option* to solve a perceived problem, which is
> actually solvable in better ways.
> 
> NIST doesn't plan to update FIPS 204 until 2029, and most likely the
> updates will just be errata in the text (such as the ones I reported to
> them), not changes or withdrawals in the algorithms themselves.  But
> it's irrelevant: just because HashML-DSA is an option doesn't mean it
> has to be used.  Pure ML-DSA supports arbitrary data, which includes

And I was sure whether it was merely an 'option'. Who would use it then 
if it takes more cycles to hash the prepended 11 byte oid in HashML-DSA?

> formatted hashes which allow for pre-hashing perfectly well.
> 
> - Eric


^ permalink raw reply

* Re: IMA and PQC
From: Eric Biggers @ 2026-02-26 16:58 UTC (permalink / raw)
  To: Simo Sorce
  Cc: Stefan Berger, Coiby Xu, Johannes Wiesböck, dhowells,
	dmitry.kasatkin, eric.snowberg, keyrings, linux-crypto,
	linux-integrity, linux-kernel, linux-modules, roberto.sassu,
	zohar, michael.weiss
In-Reply-To: <da190dbbc692b9da8464bbbfffdde7bab26b3f1c.camel@redhat.com>

On Thu, Feb 26, 2026 at 10:27:43AM -0500, Simo Sorce wrote:
> On Thu, 2026-02-26 at 09:16 -0500, Stefan Berger wrote:
> > On 2/26/26 7:42 AM, Stefan Berger wrote:
> > > On 2/25/26 7:10 PM, Eric Biggers wrote:
> > > > On Wed, Feb 25, 2026 at 09:25:43AM -0500, Stefan Berger wrote:
> > > > > To avoid duplicate work: Is either one of you planning on writing 
> > > > > patches
> > > > > for IMA to use ML-DSA and convert the current ML-DSA to also support 
> > > > > HashML?
> > > > > I had done the work on this before and could dig out the patches 
> > > > > again...
> > > > 
> > > > IMA already had to add its own digest prefixing support, since it was
> > > > needed to disambiguate between full-file digests and fsverity digests.
> > > > See 'struct ima_file_id'.  Thus the message signed is at most 66 bytes.
> > > 
> > > The hash there is still only a hash over a file and that hash is signed, 
> > > isn't it?
> > > 
> > > > 
> > > > With that being the case, HashML-DSA isn't necessary.  It's not even
> > > > possible to use here, since there are no OIDs assigned for the fsverity
> > > > digests, so it cannot replace the ima_file_id.
> > > 
> > > For non-fsverify IMA signatures it is 'possible' to use HashML-DSA and 
> > > it's 'working' (recycled old patches yesterday):
> > > 
> > > Linux: https://github.com/stefanberger/linux/commits/ 
> > > dhmlsa%2Bima.202602025/
> > > 
> > > ima-evm-utils: https://github.com/linux-integrity/ima-evm-utils/pull/19/ 
> > > commits
> > > 
> > > > 
> > > > I'll also note that HashML-DSA is controversial (e.g. see
> > > > https://keymaterial.net/2024/11/05/hashml-dsa-considered-harmful/),
> > > 
> > > The problem with this is that NIST would have to react to these 
> > > controversies as we race to support PQC. If something is wrong with the 
> > > standard then it would be best for NIST to withdraw/modify HashML-DSA 
> > > asap. Otherwise it's the best to follow the standard IMO because if you 
> > > don't you get criticism otherwise.
> > 
> > What I am not clear about from FIPS-204 is whether availability of 
> > HashML-DSA is a "must-use" or  a "may-use". What speaks against it for 
> > our use case is performance. The lookup of a hash's ID (last digit of 
> > OID) and the creation of the 11 byte encoding to prepend before every 
> > digest for every signature takes cycles.
> 
> It is a recommendation, but there are plenty of protocols (TLS,
> OpenPGP, etc...) where the decision has been made to use "pure" ML-DSA
> only, even if what you are signing is not the full data, but something
> containing a hash.
> 
> Ideally you do not sign *just* a hash, but some structured data, like a
> context label that identifies the hash and some other related metadata
> for example. In order to make forgeries much harder should the hashing
> algorithm used to hash the data weaken over time. But it is not
> strictly necessary (NIST mentioned in some forum, sorry I do not have
> the message handy for quoting, that a structured packet is perfectly
> fine for use with pure ML-DSA, because it does enough to address the
> same issues that a separate internal context does with HashML-DSA).
> 
> If pure-ML-DSA works better for IMA, just use pure ML-DSA.
> 
> > Maybe it should explicitly state in FIPS-204 something along the lines 
> > of "with a given hash either ML-DSA or HashML-DSA can be used (for as 
> > long as you use it in the same way from then on)." At least this way 
> > nobody can point out that HashML-DSA should have been used when you didn't.
> 
> NIST will not change the standard documents any time soon, but for FIPS
> certification there are Implementation Guidelines.
> 
> In any case a FIPS module cannot distinguish between data that happens
> to be 32 bytes long and a hash of larger data, so the point is kind of
> moot. From the FIPS perspective HashML-DSA is just an available
> algorithm that protocol implementations can use, or not.
> 
> There are additional guidelines on what this may be useful for, but so
> far NIST has not objected to the use of pure ML-DSA even where
> theoretically HashML-DSA could be used.

I see that IMA indeed never upgraded full file hashes to use
'struct ima_file_id'.  Building a new feature that relies on this seems
like a bad idea though, given that it's a security bug that makes the
IMA protocol cryptographically ambiguous.  I.e., it means that in IMA,
when the contents of some file are signed, that signature is sometimes
also valid for some other file contents which the signer didn't intend.

Just fix that bug first, which has to be done anyway.  Then just use
pure ML-DSA to sign and verify the 'struct ima_file_id'.

As Simo mentioned, FIPS 204 doesn't require HashML-DSA when signing a
hash.  It's there as an *option* to solve a perceived problem, which is
actually solvable in better ways.

NIST doesn't plan to update FIPS 204 until 2029, and most likely the
updates will just be errata in the text (such as the ones I reported to
them), not changes or withdrawals in the algorithms themselves.  But
it's irrelevant: just because HashML-DSA is an option doesn't mean it
has to be used.  Pure ML-DSA supports arbitrary data, which includes
formatted hashes which allow for pre-hashing perfectly well.

- Eric

^ permalink raw reply

* Re: IMA and PQC
From: Simo Sorce @ 2026-02-26 15:27 UTC (permalink / raw)
  To: Stefan Berger, Eric Biggers
  Cc: Coiby Xu, Johannes Wiesböck, dhowells, dmitry.kasatkin,
	eric.snowberg, keyrings, linux-crypto, linux-integrity,
	linux-kernel, linux-modules, roberto.sassu, zohar, michael.weiss
In-Reply-To: <dc09be79-5efe-4756-a295-5b0428985525@linux.ibm.com>

On Thu, 2026-02-26 at 09:16 -0500, Stefan Berger wrote:
> 
> On 2/26/26 7:42 AM, Stefan Berger wrote:
> > 
> > 
> > On 2/25/26 7:10 PM, Eric Biggers wrote:
> > > On Wed, Feb 25, 2026 at 09:25:43AM -0500, Stefan Berger wrote:
> > > > To avoid duplicate work: Is either one of you planning on writing 
> > > > patches
> > > > for IMA to use ML-DSA and convert the current ML-DSA to also support 
> > > > HashML?
> > > > I had done the work on this before and could dig out the patches 
> > > > again...
> > > 
> > > IMA already had to add its own digest prefixing support, since it was
> > > needed to disambiguate between full-file digests and fsverity digests.
> > > See 'struct ima_file_id'.  Thus the message signed is at most 66 bytes.
> > 
> > The hash there is still only a hash over a file and that hash is signed, 
> > isn't it?
> > 
> > > 
> > > With that being the case, HashML-DSA isn't necessary.  It's not even
> > > possible to use here, since there are no OIDs assigned for the fsverity
> > > digests, so it cannot replace the ima_file_id.
> > 
> > For non-fsverify IMA signatures it is 'possible' to use HashML-DSA and 
> > it's 'working' (recycled old patches yesterday):
> > 
> > Linux: https://github.com/stefanberger/linux/commits/ 
> > dhmlsa%2Bima.202602025/
> > 
> > ima-evm-utils: https://github.com/linux-integrity/ima-evm-utils/pull/19/ 
> > commits
> > 
> > > 
> > > I'll also note that HashML-DSA is controversial (e.g. see
> > > https://keymaterial.net/2024/11/05/hashml-dsa-considered-harmful/),
> > 
> > The problem with this is that NIST would have to react to these 
> > controversies as we race to support PQC. If something is wrong with the 
> > standard then it would be best for NIST to withdraw/modify HashML-DSA 
> > asap. Otherwise it's the best to follow the standard IMO because if you 
> > don't you get criticism otherwise.
> 
> What I am not clear about from FIPS-204 is whether availability of 
> HashML-DSA is a "must-use" or  a "may-use". What speaks against it for 
> our use case is performance. The lookup of a hash's ID (last digit of 
> OID) and the creation of the 11 byte encoding to prepend before every 
> digest for every signature takes cycles.

It is a recommendation, but there are plenty of protocols (TLS,
OpenPGP, etc...) where the decision has been made to use "pure" ML-DSA
only, even if what you are signing is not the full data, but something
containing a hash.

Ideally you do not sign *just* a hash, but some structured data, like a
context label that identifies the hash and some other related metadata
for example. In order to make forgeries much harder should the hashing
algorithm used to hash the data weaken over time. But it is not
strictly necessary (NIST mentioned in some forum, sorry I do not have
the message handy for quoting, that a structured packet is perfectly
fine for use with pure ML-DSA, because it does enough to address the
same issues that a separate internal context does with HashML-DSA).

If pure-ML-DSA works better for IMA, just use pure ML-DSA.

> Maybe it should explicitly state in FIPS-204 something along the lines 
> of "with a given hash either ML-DSA or HashML-DSA can be used (for as 
> long as you use it in the same way from then on)." At least this way 
> nobody can point out that HashML-DSA should have been used when you didn't.

NIST will not change the standard documents any time soon, but for FIPS
certification there are Implementation Guidelines.

In any case a FIPS module cannot distinguish between data that happens
to be 32 bytes long and a hash of larger data, so the point is kind of
moot. From the FIPS perspective HashML-DSA is just an available
algorithm that protocol implementations can use, or not.

There are additional guidelines on what this may be useful for, but so
far NIST has not objected to the use of pure ML-DSA even where
theoretically HashML-DSA could be used.

> > 
> > > since it was added to the ML-DSA specification at a late stage without
> > > sufficient review, and what it does can be achieved in better ways.
> > 
> > In case of doubt I would use the standard, though. It's probably not a 
> > good idea for everyone to implement their own (bad) solution.
> > 
> > > Which is exactly what we are seeing here, since again, IMA needs to do
> > > the digest calculation and prefixing itself anyway.
> > 
> > Use the standard...
> > 
> >     Stefan
> > 
> > > 
> > > - Eric
> > 
> > 
> 

-- 
Simo Sorce
Distinguished Engineer
RHEL Crypto Team
Red Hat, Inc


^ permalink raw reply

* Re: IMA and PQC
From: Stefan Berger @ 2026-02-26 14:16 UTC (permalink / raw)
  To: Eric Biggers
  Cc: Coiby Xu, Johannes Wiesböck, dhowells, dmitry.kasatkin,
	eric.snowberg, keyrings, linux-crypto, linux-integrity,
	linux-kernel, linux-modules, roberto.sassu, simo, zohar,
	michael.weiss
In-Reply-To: <cba10ac6-3557-4fc1-9b86-55361d14156d@linux.ibm.com>



On 2/26/26 7:42 AM, Stefan Berger wrote:
> 
> 
> On 2/25/26 7:10 PM, Eric Biggers wrote:
>> On Wed, Feb 25, 2026 at 09:25:43AM -0500, Stefan Berger wrote:
>>> To avoid duplicate work: Is either one of you planning on writing 
>>> patches
>>> for IMA to use ML-DSA and convert the current ML-DSA to also support 
>>> HashML?
>>> I had done the work on this before and could dig out the patches 
>>> again...
>>
>> IMA already had to add its own digest prefixing support, since it was
>> needed to disambiguate between full-file digests and fsverity digests.
>> See 'struct ima_file_id'.  Thus the message signed is at most 66 bytes.
> 
> The hash there is still only a hash over a file and that hash is signed, 
> isn't it?
> 
>>
>> With that being the case, HashML-DSA isn't necessary.  It's not even
>> possible to use here, since there are no OIDs assigned for the fsverity
>> digests, so it cannot replace the ima_file_id.
> 
> For non-fsverify IMA signatures it is 'possible' to use HashML-DSA and 
> it's 'working' (recycled old patches yesterday):
> 
> Linux: https://github.com/stefanberger/linux/commits/ 
> dhmlsa%2Bima.202602025/
>
> ima-evm-utils: https://github.com/linux-integrity/ima-evm-utils/pull/19/ 
> commits
> 
>>
>> I'll also note that HashML-DSA is controversial (e.g. see
>> https://keymaterial.net/2024/11/05/hashml-dsa-considered-harmful/),
> 
> The problem with this is that NIST would have to react to these 
> controversies as we race to support PQC. If something is wrong with the 
> standard then it would be best for NIST to withdraw/modify HashML-DSA 
> asap. Otherwise it's the best to follow the standard IMO because if you 
> don't you get criticism otherwise.

What I am not clear about from FIPS-204 is whether availability of 
HashML-DSA is a "must-use" or  a "may-use". What speaks against it for 
our use case is performance. The lookup of a hash's ID (last digit of 
OID) and the creation of the 11 byte encoding to prepend before every 
digest for every signature takes cycles.

Maybe it should explicitly state in FIPS-204 something along the lines 
of "with a given hash either ML-DSA or HashML-DSA can be used (for as 
long as you use it in the same way from then on)." At least this way 
nobody can point out that HashML-DSA should have been used when you didn't.

> 
>> since it was added to the ML-DSA specification at a late stage without
>> sufficient review, and what it does can be achieved in better ways.
> 
> In case of doubt I would use the standard, though. It's probably not a 
> good idea for everyone to implement their own (bad) solution.
> 
>> Which is exactly what we are seeing here, since again, IMA needs to do
>> the digest calculation and prefixing itself anyway.
> 
> Use the standard...
> 
>     Stefan
> 
>>
>> - Eric
> 
> 



^ permalink raw reply

* Re: [PATCH v4 2/2] lib/crypto: tests: Add KUnit tests for ML-DSA verification
From: Geert Uytterhoeven @ 2026-02-26 13:09 UTC (permalink / raw)
  To: Eric Biggers
  Cc: linux-crypto, David Howells, Herbert Xu, Luis Chamberlain,
	Petr Pavlu, Daniel Gomez, Sami Tolvanen, Jason A . Donenfeld,
	Ard Biesheuvel, Stephan Mueller, Lukas Wunner, Ignat Korchagin,
	keyrings, linux-modules, linux-kernel,
	open list:KERNEL SELFTEST FRAMEWORK, KUnit Development
In-Reply-To: <20251214181712.29132-3-ebiggers@kernel.org>

Hi Eric,

CC kunit

On Sun, 14 Dec 2025 at 19:18, Eric Biggers <ebiggers@kernel.org> wrote:
> Add a KUnit test suite for ML-DSA verification, including the following
> for each ML-DSA parameter set (ML-DSA-44, ML-DSA-65, and ML-DSA-87):
>
> - Positive test (valid signature), using vector imported from leancrypto
> - Various negative tests:
>     - Wrong length for signature, message, or public key
>     - Out-of-range coefficients in z vector
>     - Invalid encoded hint vector
>     - Any bit flipped in signature, message, or public key
> - Unit test for the internal function use_hint()
> - A benchmark
>
> ML-DSA inputs and outputs are very large.  To keep the size of the tests
> down, use just one valid test vector per parameter set, and generate the
> negative tests at runtime by mutating the valid test vector.
>
> I also considered importing the test vectors from Wycheproof.  I've
> tested that mldsa_verify() indeed passes all of Wycheproof's ML-DSA test
> vectors that use an empty context string.  However, importing these
> permanently would add over 6 MB of source.  That's too much to be a
> reasonable addition to the Linux kernel tree for one algorithm.  It also
> wouldn't actually provide much better test coverage than this commit.
> Another potential issue is that Wycheproof uses the Apache license.
>
> Similarly, this also differs from the earlier proposal to import a long
> list of test vectors from leancrypto.  I retained only one valid
> signature for each algorithm, and I also added (runtime-generated)
> negative tests which were missing.  I think this is a better tradeoff.
>
> Reviewed-by: David Howells <dhowells@redhat.com>
> Tested-by: David Howells <dhowells@redhat.com>
> Signed-off-by: Eric Biggers <ebiggers@kernel.org>

Thanks for your patch, which is now commit ed894faccb8de55c
("lib/crypto: tests: Add KUnit tests for ML-DSA verification")
in v7.0-rc1.

> --- a/lib/crypto/tests/Kconfig
> +++ b/lib/crypto/tests/Kconfig
> @@ -36,10 +36,19 @@ config CRYPTO_LIB_MD5_KUNIT_TEST
>         select CRYPTO_LIB_MD5
>         help
>           KUnit tests for the MD5 cryptographic hash function and its
>           corresponding HMAC.
>
> +config CRYPTO_LIB_MLDSA_KUNIT_TEST
> +       tristate "KUnit tests for ML-DSA" if !KUNIT_ALL_TESTS
> +       depends on KUNIT
> +       default KUNIT_ALL_TESTS || CRYPTO_SELFTESTS
> +       select CRYPTO_LIB_BENCHMARK_VISIBLE
> +       select CRYPTO_LIB_MLDSA

These two selects mean that enabling KUNIT_ALL_TESTS also enables
extra functionality, which may not be desirable in a production system.
Fortunately CRYPTO_LIB_MLDSA is tristate, so in the modular case
the extra functionality is a module, too, and not part of the running system
by default.  Unfortunately CRYPTO_LIB_MLDSA is invisible, so this cannot
just be changed from "select" to "depends on". But as CRYPTO_MLDSA
also selects it, perhaps the test can be made dependent on CRYPTO_MLDSA?

> +       help
> +         KUnit tests for the ML-DSA digital signature algorithm.
> +
>  config CRYPTO_LIB_POLY1305_KUNIT_TEST
>         tristate "KUnit tests for Poly1305" if !KUNIT_ALL_TESTS
>         depends on KUNIT
>         default KUNIT_ALL_TESTS || CRYPTO_SELFTESTS
>         select CRYPTO_LIB_BENCHMARK_VISIBLE

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* Re: IMA and PQC
From: Stefan Berger @ 2026-02-26 12:42 UTC (permalink / raw)
  To: Eric Biggers
  Cc: Coiby Xu, Johannes Wiesböck, dhowells, dmitry.kasatkin,
	eric.snowberg, keyrings, linux-crypto, linux-integrity,
	linux-kernel, linux-modules, roberto.sassu, simo, zohar,
	michael.weiss
In-Reply-To: <20260226001049.GA3135@quark>



On 2/25/26 7:10 PM, Eric Biggers wrote:
> On Wed, Feb 25, 2026 at 09:25:43AM -0500, Stefan Berger wrote:
>> To avoid duplicate work: Is either one of you planning on writing patches
>> for IMA to use ML-DSA and convert the current ML-DSA to also support HashML?
>> I had done the work on this before and could dig out the patches again...
> 
> IMA already had to add its own digest prefixing support, since it was
> needed to disambiguate between full-file digests and fsverity digests.
> See 'struct ima_file_id'.  Thus the message signed is at most 66 bytes.

The hash there is still only a hash over a file and that hash is signed, 
isn't it?

> 
> With that being the case, HashML-DSA isn't necessary.  It's not even
> possible to use here, since there are no OIDs assigned for the fsverity
> digests, so it cannot replace the ima_file_id.

For non-fsverify IMA signatures it is 'possible' to use HashML-DSA and 
it's 'working' (recycled old patches yesterday):

Linux: https://github.com/stefanberger/linux/commits/dhmlsa%2Bima.202602025/

ima-evm-utils: 
https://github.com/linux-integrity/ima-evm-utils/pull/19/commits

> 
> I'll also note that HashML-DSA is controversial (e.g. see
> https://keymaterial.net/2024/11/05/hashml-dsa-considered-harmful/),

The problem with this is that NIST would have to react to these 
controversies as we race to support PQC. If something is wrong with the 
standard then it would be best for NIST to withdraw/modify HashML-DSA 
asap. Otherwise it's the best to follow the standard IMO because if you 
don't you get criticism otherwise.

> since it was added to the ML-DSA specification at a late stage without
> sufficient review, and what it does can be achieved in better ways.

In case of doubt I would use the standard, though. It's probably not a 
good idea for everyone to implement their own (bad) solution.

> Which is exactly what we are seeing here, since again, IMA needs to do
> the digest calculation and prefixing itself anyway.

Use the standard...

    Stefan

> 
> - Eric


^ permalink raw reply

* Re: [PATCH] kernel/trace/ftrace: introduce ftrace module notifier
From: Miroslav Benes @ 2026-02-26 10:51 UTC (permalink / raw)
  To: Song Chen
  Cc: Steven Rostedt, mcgrof, petr.pavlu, da.gomez, samitolvanen,
	atomlin, mhiramat, mark.rutland, mathieu.desnoyers, linux-modules,
	linux-kernel, linux-trace-kernel, live-patching
In-Reply-To: <e18ed5f4-3917-46e7-bca9-78063e6e4457@189.cn>

[-- Attachment #1: Type: text/plain, Size: 3620 bytes --]

Hi,

+ Cc: live-patching@vger.kernel.org

On Thu, 26 Feb 2026, Song Chen wrote:

> Hi,
> 
> 在 2026/2/26 08:27, Steven Rostedt 写道:
> > On Wed, 25 Feb 2026 13:46:39 +0800
> > chensong_2000@189.cn wrote:
> > 
> >> From: Song Chen <chensong_2000@189.cn>
> >>
> >> Like kprobe, fprobe and btf, this patch attempts to introduce
> >> a notifier_block for ftrace to decouple its initialization from
> >> load_module.
> >>
> >> Below is the table of ftrace fucntions calls in different
> >> module state:
> >>
> >>  MODULE_STATE_UNFORMED	ftrace_module_init
> >>  MODULE_STATE_COMING	ftrace_module_enable
> >>  MODULE_STATE_LIVE	ftrace_free_mem
> >>  MODULE_STATE_GOING	ftrace_release_mod
> >>
> >> Unlike others, ftrace module notifier must take care of state
> >> MODULE_STATE_UNFORMED to ensure calling ftrace_module_init
> >> before complete_formation which changes module's text property.
> >>
> >> That pretty much remains same logic with its original design,
> >> the only thing that changes is blocking_notifier_call_chain
> >> (MODULE_STATE_GOING) has to be moved from coming_cleanup to
> >> ddebug_cleanup in function load_module to ensure
> >> ftrace_release_mod is invoked in case complete_formation fails.
> >>
> >> Signed-off-by: Song Chen <chensong_2000@189.cn>
> >> ---
> >>   kernel/module/main.c  | 14 ++++----------
> >>   kernel/trace/ftrace.c | 37 +++++++++++++++++++++++++++++++++++++
> >>   2 files changed, 41 insertions(+), 10 deletions(-)
> >>
> >> diff --git a/kernel/module/main.c b/kernel/module/main.c
> >> index 710ee30b3bea..5dc0a980e9bd 100644
> >> --- a/kernel/module/main.c
> >> +++ b/kernel/module/main.c
> >> @@ -45,7 +45,6 @@
> >>   #include <linux/license.h>
> >>   #include <asm/sections.h>
> >>   #include <linux/tracepoint.h>
> >> -#include <linux/ftrace.h>
> >>   #include <linux/livepatch.h>
> >>   #include <linux/async.h>
> >>   #include <linux/percpu.h>
> >> @@ -836,7 +835,6 @@ SYSCALL_DEFINE2(delete_module, const char __user *,
> >> name_user,
> >>    blocking_notifier_call_chain(&module_notify_list,
> >>    			     MODULE_STATE_GOING, mod);
> >>   	klp_module_going(mod);
> >> -	ftrace_release_mod(mod);
> > 
> > Is the above safe? klp uses ftrace. That means klp_module_going() may
> > need to be called before ftrace_release_mod(). That said, I wonder if
> > klp_module_going() could be moved into ftrace_release_mod()?
> > 
> >>   
> 
> I didn't test with klp, so i'm not sure if it's safe. But i consider klp is
> the other part which should be decoupled after ftrace and klp should introduce
> its own notifier.
> 
> If klp_module_going must be running before ftrace_release_mod, i can try to
> use priority in notifier_block to ensure their order.
> 
> Let me see if there is any way to use notifier and remain below calling
> sequence:
> 
> ftrace_module_enable
> klp_module_coming
> blocking_notifier_call_chain_robust(MODULE_STATE_COMING)
> 
> blocking_notifier_call_chain(MODULE_STATE_GOING)
> klp_module_going
> ftrace_release_mod

Both klp and ftrace used module notifiers in the past. We abandoned that 
and opted for direct calls due to issues with ordering at the time. I do 
not have the list of problems at hand but I remember it was very fragile.

See commits 7dcd182bec27 ("ftrace/module: remove ftrace module 
notifier"), 7e545d6eca20 ("livepatch/module: remove livepatch module 
notifier") and their surroundings.

So unless there is a reason for the change (which should be then carefully 
reviewed and properly tested), I would prefer to keep it as is. What is 
the motivation? I am failing to find it in the commit log.

Regards,
Miroslav

^ permalink raw reply

* Re: [PATCH] kernel/trace/ftrace: introduce ftrace module notifier
From: Song Chen @ 2026-02-26 10:12 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: mcgrof, petr.pavlu, da.gomez, samitolvanen, atomlin, mhiramat,
	mark.rutland, mathieu.desnoyers, linux-modules, linux-kernel,
	linux-trace-kernel
In-Reply-To: <20260225192724.48ed165e@fedora>

Hi,

在 2026/2/26 08:27, Steven Rostedt 写道:
> On Wed, 25 Feb 2026 13:46:39 +0800
> chensong_2000@189.cn wrote:
> 
>> From: Song Chen <chensong_2000@189.cn>
>>
>> Like kprobe, fprobe and btf, this patch attempts to introduce
>> a notifier_block for ftrace to decouple its initialization from
>> load_module.
>>
>> Below is the table of ftrace fucntions calls in different
>> module state:
>>
>> 	MODULE_STATE_UNFORMED	ftrace_module_init
>> 	MODULE_STATE_COMING	ftrace_module_enable
>> 	MODULE_STATE_LIVE	ftrace_free_mem
>> 	MODULE_STATE_GOING	ftrace_release_mod
>>
>> Unlike others, ftrace module notifier must take care of state
>> MODULE_STATE_UNFORMED to ensure calling ftrace_module_init
>> before complete_formation which changes module's text property.
>>
>> That pretty much remains same logic with its original design,
>> the only thing that changes is blocking_notifier_call_chain
>> (MODULE_STATE_GOING) has to be moved from coming_cleanup to
>> ddebug_cleanup in function load_module to ensure
>> ftrace_release_mod is invoked in case complete_formation fails.
>>
>> Signed-off-by: Song Chen <chensong_2000@189.cn>
>> ---
>>   kernel/module/main.c  | 14 ++++----------
>>   kernel/trace/ftrace.c | 37 +++++++++++++++++++++++++++++++++++++
>>   2 files changed, 41 insertions(+), 10 deletions(-)
>>
>> diff --git a/kernel/module/main.c b/kernel/module/main.c
>> index 710ee30b3bea..5dc0a980e9bd 100644
>> --- a/kernel/module/main.c
>> +++ b/kernel/module/main.c
>> @@ -45,7 +45,6 @@
>>   #include <linux/license.h>
>>   #include <asm/sections.h>
>>   #include <linux/tracepoint.h>
>> -#include <linux/ftrace.h>
>>   #include <linux/livepatch.h>
>>   #include <linux/async.h>
>>   #include <linux/percpu.h>
>> @@ -836,7 +835,6 @@ SYSCALL_DEFINE2(delete_module, const char __user *, name_user,
>>   	blocking_notifier_call_chain(&module_notify_list,
>>   				     MODULE_STATE_GOING, mod);
>>   	klp_module_going(mod);
>> -	ftrace_release_mod(mod);
> 
> Is the above safe? klp uses ftrace. That means klp_module_going() may
> need to be called before ftrace_release_mod(). That said, I wonder if
> klp_module_going() could be moved into ftrace_release_mod()?
> 
>>   

I didn't test with klp, so i'm not sure if it's safe. But i consider klp 
is the other part which should be decoupled after ftrace and klp should 
introduce its own notifier.

If klp_module_going must be running before ftrace_release_mod, i can try 
to use priority in notifier_block to ensure their order.

Let me see if there is any way to use notifier and remain below calling 
sequence:

ftrace_module_enable
klp_module_coming
blocking_notifier_call_chain_robust(MODULE_STATE_COMING)

blocking_notifier_call_chain(MODULE_STATE_GOING)
klp_module_going
ftrace_release_mod


>>   	async_synchronize_full();
>>   
>> @@ -3067,8 +3065,6 @@ static noinline int do_init_module(struct module *mod)
>>   	if (!mod->async_probe_requested)
>>   		async_synchronize_full();
>>   
>> -	ftrace_free_mem(mod, mod->mem[MOD_INIT_TEXT].base,
>> -			mod->mem[MOD_INIT_TEXT].base + mod->mem[MOD_INIT_TEXT].size);
> 
> Have you tested the case for why this is called? It has to be called
> before the module frees the kallsyms. It's for tracing the module's
> init functions.
> 
>    cd /sys/kernel/tracing
>    echo :mod:<module> > set_ftrace_filter
>    echo function > current_tracer
>    modprobe <module>
>    cat trace
> 
> You should see the init functions of the module loaded. If
> ftrace_free_mem() is called after the module frees the kallsyms of the
> module init functions, you'll just get garbage for the init function
> names.
> 
> 

Yes, after applying this patch, i tested it with your above commands, 
result is:

cat trace_pipe
            <...>-4027    [004] .....   103.171161: mem_blkdev_init 
<-do_one_initcall
           insmod-4027    [004] .....   103.249854: mem_blkdev_queue_rq 
<-blk_mq_dispatch_rq_list
           insmod-4027    [004] .....   103.249865: mem_blkdev_queue_rq 
<-blk_mq_dispatch_rq_list
....
module init function can be seen when module is being loaded.

As far as my understanding, ftrace_free_mem is called right after 
blocking_notifier_call_chain(MODULE_STATE_LIVE) originally in 
do_init_module, after this patch, it's called by notifier, almost 
nothing changed, so no impact to current calling sequence.

Correct me if I'm wrong.

> 
>>   	mutex_lock(&module_mutex);
>>   	/* Drop initial reference. */
>>   	module_put(mod);
>> @@ -3131,7 +3127,6 @@ static noinline int do_init_module(struct module *mod)
>>   	blocking_notifier_call_chain(&module_notify_list,
>>   				     MODULE_STATE_GOING, mod);
>>   	klp_module_going(mod);
>> -	ftrace_release_mod(mod);
>>   	free_module(mod);
>>   	wake_up_all(&module_wq);
>>   
>> @@ -3278,7 +3273,6 @@ static int prepare_coming_module(struct module *mod)
>>   {
>>   	int err;
>>   
>> -	ftrace_module_enable(mod);
>>   	err = klp_module_coming(mod);
> 
> Same issue with ftrace and klp here.
> 
>>   	if (err)
>>   		return err;
>> @@ -3461,7 +3455,8 @@ static int load_module(struct load_info *info, const char __user *uargs,
>>   	init_build_id(mod, info);
>>   
>>   	/* Ftrace init must be called in the MODULE_STATE_UNFORMED state */
>> -	ftrace_module_init(mod);
>> +	blocking_notifier_call_chain(&module_notify_list,
>> +				MODULE_STATE_UNFORMED, mod);
>>   
>>   	/* Finally it's fully formed, ready to start executing. */
>>   	err = complete_formation(mod, info);
>> @@ -3513,8 +3508,6 @@ static int load_module(struct load_info *info, const char __user *uargs,
>>    coming_cleanup:
>>   	mod->state = MODULE_STATE_GOING;
>>   	destroy_params(mod->kp, mod->num_kp);
>> -	blocking_notifier_call_chain(&module_notify_list,
>> -				     MODULE_STATE_GOING, mod);
>>   	klp_module_going(mod);
> 
> Now klp_module_going() may need to be called *after* the
> MODULE_STATE_GOING callbacks and *before* ftrace_release_mod(). But
> again, if that's moved into ftrace_release_mod() it may be fine.
> 
>>    bug_cleanup:
>>   	mod->state = MODULE_STATE_GOING;
>> @@ -3524,7 +3517,8 @@ static int load_module(struct load_info *info, const char __user *uargs,
>>   	mutex_unlock(&module_mutex);
>>   
>>    ddebug_cleanup:
>> -	ftrace_release_mod(mod);
>> +	blocking_notifier_call_chain(&module_notify_list,
>> +				     MODULE_STATE_GOING, mod);
>>   	synchronize_rcu();
>>   	kfree(mod->args);
>>    free_arch_cleanup:
>> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> 
> -- Steve
> 


^ permalink raw reply

* Re: [PATCH v2] module: print version for external modules in print_modules()
From: Yafang Shao @ 2026-02-26  2:18 UTC (permalink / raw)
  To: mcgrof, petr.pavlu, da.gomez, samitolvanen, atomlin; +Cc: linux-modules
In-Reply-To: <20251231094004.37851-1-laoar.shao@gmail.com>

On Wed, Dec 31, 2025 at 5:40 PM Yafang Shao <laoar.shao@gmail.com> wrote:
>
> We maintain a vmcore analysis script on each server that automatically
> parses /var/crash/XXXX/vmcore-dmesg.txt to categorize vmcores. This helps
> us save considerable effort by avoiding analysis of known bugs.
>
> For vmcores triggered by a driver bug, the system calls print_modules() to
> list the loaded modules. However, print_modules() does not output module
> version information. Across a large fleet of servers, there are often many
> different module versions running simultaneously, and we need to know which
> driver version caused a given vmcore.
>
> Currently, the only reliable way to obtain the module version associated
> with a vmcore is to analyze the /var/crash/XXXX/vmcore file itself—an
> operation that is resource-intensive. Therefore, we propose printing the
> driver version directly in the log, which is far more efficient.
>
> The motivation behind this change is that the external NVIDIA driver
> [0] frequently causes kernel panics across our server fleet.
> While we continuously upgrade to newer NVIDIA driver versions,
> upgrading the entire fleet is time-consuming. Therefore, we need to
> identify which driver version is responsible for each panic.
>
> In-tree modules are tied to the specific kernel version already, so
> printing their versions is redundant. However, for external drivers (like
> proprietary networking or GPU stacks), the version is the single most
> critical piece of metadata for triage. Therefore, to avoid bloating the
> information about loaded modules, we only print the version for external
> modules.
>
> - Before this patch
>
>   Modules linked in: mlx5_core(O) nvidia(PO) nvme_core
>
> - After this patch
>
>   Modules linked in: mlx5_core-5.8-2.0.3(O) nvidia-535.274.02(PO) nvme_core
>                               ^^^^^^^^^^          ^^^^^^^^^^^
>
>   Note: nvme_core is a in-tree module[1], so its version isn't printed.
>
> Link: https://github.com/NVIDIA/open-gpu-kernel-modules/tags [0]
> Link: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/nvme/host/core.c?h=v6.19-rc3#n5448 [1]
> Suggested-by: Petr Pavlu <petr.pavlu@suse.com>
> Reviewed-by: Aaron Tomlin <atomlin@atomlin.com>
> Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> ---
>  kernel/module/main.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> ---
> v1->v2:
> - print it for external module only (Petr, Aaron)
> - add comment for it (Aaron)
>
> diff --git a/kernel/module/main.c b/kernel/module/main.c
> index 710ee30b3bea..16263ce23e92 100644
> --- a/kernel/module/main.c
> +++ b/kernel/module/main.c
> @@ -3901,7 +3901,11 @@ void print_modules(void)
>         list_for_each_entry_rcu(mod, &modules, list) {
>                 if (mod->state == MODULE_STATE_UNFORMED)
>                         continue;
> -               pr_cont(" %s%s", mod->name, module_flags(mod, buf, true));
> +               pr_cont(" %s", mod->name);
> +               /* Only append version for out-of-tree modules */
> +               if (mod->version && test_bit(TAINT_OOT_MODULE, &mod->taints))
> +                       pr_cont("-%s", mod->version);
> +               pr_cont("%s", module_flags(mod, buf, true));
>         }
>
>         print_unloaded_tainted_modules();
> --
> 2.43.5
>

Just checking in on this patch. It looks like it hasn't been merged
yet. Is it good to go, or does it need something else?

-- 
Regards
Yafang

^ 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