Rust for Linux List
 help / color / mirror / Atom feed
* [PATCH 0/1] rust: module_param: Fix potentially incorrect access  of `SetOnce<T>`
@ 2026-09-17 17:43 Priya Bala Govindasamy
  2026-09-17 17:43 ` [PATCH 1/1] " Priya Bala Govindasamy
  2026-09-23 20:31 ` [PATCH v2] " Priya Bala Govindasamy
  0 siblings, 2 replies; 6+ messages in thread
From: Priya Bala Govindasamy @ 2026-09-17 17:43 UTC (permalink / raw)
  To: mcgrof, petr.pavlu, da.gomez, samitolvanen, ojeda
  Cc: atomlin, boqun, gary, bjorn3_gh, lossin, a.hindborg, aliceryhl,
	tmgross, dakr, daniel.almeida, tamird, acourbot, work,
	linux-modules, rust-for-linux, ardalan, zhiyunq, dzueck, pgovind2

Dear Linux kernel maintainers,

We are developing a tool called FerroLens to detect potential 
unsound behavior in Rust code in the Linux kernel. The tool 
internally uses an LLM to detect bugs.
We then perform manual analysis to verify these reports. 
FerroLens reported the following bug in rust/kernel/module_param.rs:

The `set_param` function casts `kernel_param.arg` to `*const SetOnce<T>`
But the Rust module macro in rust/macros/module.rs initializes `arg` with 
`#param_name.as_void_ptr()`, and `#param_name` is a `ModuleParamAccess<T>`,
not a `SetOnce<T>`.

ModuleParamAccess<T> has default Rust layout but `set_param` accesses its 
first field SetOnce<T> assuming it to be at offset 0. This is not 
guaranteed by Rust and if SetOnce<T> is not at offset 0 it could cause 
type confusion leading to data corruption.

In particular, if the kernel is built with a randomized layout 
(e.g with KRUSTFLAGS='-Zrandomize-layout=yes -Zlayout-seed=1'), 
the first field of `ModuleParamAccess<T>` may not be at offset 0. 
This leads to incorrect behavior when loading the rust_minimal module 
from samples.

$ sudo insmod samples/rust/rust_minimal.ko test_parameter=1 test_bool_parameter=false
insmod: ERROR: could not insert module samples/rust/rust_minimal.ko: File exists

[  854.638251] rust_minimal: `1' invalid for parameter `test_parameter'

Priya Bala Govindasamy (1):
  rust: module_param: Fix potentially incorrect layout for 
    `ModuleParamAccess<T>`

 rust/kernel/module_param.rs | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

-- 
2.34.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/1] rust: module_param: Fix potentially incorrect access of `SetOnce<T>`
  2026-09-17 17:43 [PATCH 0/1] rust: module_param: Fix potentially incorrect access of `SetOnce<T>` Priya Bala Govindasamy
@ 2026-09-17 17:43 ` Priya Bala Govindasamy
  2026-09-20 20:12   ` Miguel Ojeda
                     ` (2 more replies)
  2026-09-23 20:31 ` [PATCH v2] " Priya Bala Govindasamy
  1 sibling, 3 replies; 6+ messages in thread
From: Priya Bala Govindasamy @ 2026-09-17 17:43 UTC (permalink / raw)
  To: mcgrof, petr.pavlu, da.gomez, samitolvanen, ojeda
  Cc: atomlin, boqun, gary, bjorn3_gh, lossin, a.hindborg, aliceryhl,
	tmgross, dakr, daniel.almeida, tamird, acourbot, work,
	linux-modules, rust-for-linux, ardalan, zhiyunq, dzueck, pgovind2

The `set_param` function casts `kernel_param.arg` to `*const SetOnce<T>`
But the Rust module macro in rust/macros/module.rs initializes `arg` with
`#param_name.as_void_ptr()`, and `#param_name` is a `ModuleParamAccess<T>`,
not a `SetOnce<T>`.

ModuleParamAccess<T> has default Rust layout but `set_param` accesses its
first field SetOnce<T> assuming it to be at offset 0. This is not
guaranteed by Rust and could cause type confusion leading to data
corruption.

Fix this by casting `kernel_param.arg` to `ModuleParamAccess<T>` and
then accessing the `value: SetOnce<T>` field.

Fixes: 0b08fc292842 ("rust: introduce module_param module")
Reported-by: Dylan Zueck <dzueck@uci.edu>
Assisted-by: LLM
Signed-off-by: Priya Bala Govindasamy <pgovind2@uci.edu>
---
 rust/kernel/module_param.rs | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/rust/kernel/module_param.rs b/rust/kernel/module_param.rs
index f9a14765a926..7ae87ffe1d6b 100644
--- a/rust/kernel/module_param.rs
+++ b/rust/kernel/module_param.rs
@@ -75,7 +75,9 @@ pub trait ModuleParam: Sized + Copy {
         let new_value = T::try_from_param_arg(arg)?;
 
         // SAFETY: By function safety requirements, this access is safe.
-        let container = unsafe { &*((*param).__bindgen_anon_1.arg.cast::<SetOnce<T>>()) };
+        let param_access = unsafe { &*((*param)
+            .__bindgen_anon_1.arg.cast::<ModuleParamAccess<T>>()) };
+        let container = &param_access.value;
 
         container
             .populate(new_value)
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/1] rust: module_param: Fix potentially incorrect access of `SetOnce<T>`
  2026-09-17 17:43 ` [PATCH 1/1] " Priya Bala Govindasamy
@ 2026-09-20 20:12   ` Miguel Ojeda
  2026-09-22 12:16   ` Petr Pavlu
  2026-09-22 12:38   ` Andreas Hindborg
  2 siblings, 0 replies; 6+ messages in thread
From: Miguel Ojeda @ 2026-09-20 20:12 UTC (permalink / raw)
  To: Priya Bala Govindasamy
  Cc: mcgrof, petr.pavlu, da.gomez, samitolvanen, ojeda, atomlin, boqun,
	gary, bjorn3_gh, lossin, a.hindborg, aliceryhl, tmgross, dakr,
	daniel.almeida, tamird, acourbot, work, linux-modules,
	rust-for-linux, ardalan, zhiyunq, dzueck

On Thu, Sep 17, 2026 at 7:43 PM Priya Bala Govindasamy <pgovind2@uci.edu> wrote:
>
> Fixes: 0b08fc292842 ("rust: introduce module_param module")

Before it is forgotten:

Cc: stable@vger.kernel.org

Without taking a proper look reproducing etc., it is indeed true that
Rust's default representation may reorder fields (even without the
flags).

Thanks!

Cheers,
Miguel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/1] rust: module_param: Fix potentially incorrect access of `SetOnce<T>`
  2026-09-17 17:43 ` [PATCH 1/1] " Priya Bala Govindasamy
  2026-09-20 20:12   ` Miguel Ojeda
@ 2026-09-22 12:16   ` Petr Pavlu
  2026-09-22 12:38   ` Andreas Hindborg
  2 siblings, 0 replies; 6+ messages in thread
From: Petr Pavlu @ 2026-09-22 12:16 UTC (permalink / raw)
  To: Priya Bala Govindasamy
  Cc: mcgrof, da.gomez, samitolvanen, ojeda, atomlin, boqun, gary,
	bjorn3_gh, lossin, a.hindborg, aliceryhl, tmgross, dakr,
	daniel.almeida, tamird, acourbot, work, linux-modules,
	rust-for-linux, ardalan, zhiyunq, dzueck

On 9/17/26 7:43 PM, Priya Bala Govindasamy wrote:
> The `set_param` function casts `kernel_param.arg` to `*const SetOnce<T>`
> But the Rust module macro in rust/macros/module.rs initializes `arg` with
> `#param_name.as_void_ptr()`, and `#param_name` is a `ModuleParamAccess<T>`,
> not a `SetOnce<T>`.
> 
> ModuleParamAccess<T> has default Rust layout but `set_param` accesses its
> first field SetOnce<T> assuming it to be at offset 0. This is not
> guaranteed by Rust and could cause type confusion leading to data
> corruption.
> 
> Fix this by casting `kernel_param.arg` to `ModuleParamAccess<T>` and
> then accessing the `value: SetOnce<T>` field.
> 
> Fixes: 0b08fc292842 ("rust: introduce module_param module")
> Reported-by: Dylan Zueck <dzueck@uci.edu>
> Assisted-by: LLM
> Signed-off-by: Priya Bala Govindasamy <pgovind2@uci.edu>
> ---
>  rust/kernel/module_param.rs | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/rust/kernel/module_param.rs b/rust/kernel/module_param.rs
> index f9a14765a926..7ae87ffe1d6b 100644
> --- a/rust/kernel/module_param.rs
> +++ b/rust/kernel/module_param.rs
> @@ -75,7 +75,9 @@ pub trait ModuleParam: Sized + Copy {
>          let new_value = T::try_from_param_arg(arg)?;
>  
>          // SAFETY: By function safety requirements, this access is safe.
> -        let container = unsafe { &*((*param).__bindgen_anon_1.arg.cast::<SetOnce<T>>()) };
> +        let param_access = unsafe { &*((*param)
> +            .__bindgen_anon_1.arg.cast::<ModuleParamAccess<T>>()) };
> +        let container = &param_access.value;
>  
>          container
>              .populate(new_value)

This looks ok to me, except for the formatting. I plan to take it on the
modules tree, after giving others a bit more time to potentially
comment.

Reviewed-by: Petr Pavlu <petr.pavlu@suse.com>

-- 
Thanks,
Petr

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/1] rust: module_param: Fix potentially incorrect access of `SetOnce<T>`
  2026-09-17 17:43 ` [PATCH 1/1] " Priya Bala Govindasamy
  2026-09-20 20:12   ` Miguel Ojeda
  2026-09-22 12:16   ` Petr Pavlu
@ 2026-09-22 12:38   ` Andreas Hindborg
  2 siblings, 0 replies; 6+ messages in thread
From: Andreas Hindborg @ 2026-09-22 12:38 UTC (permalink / raw)
  To: Priya Bala Govindasamy, mcgrof, petr.pavlu, da.gomez,
	samitolvanen, ojeda
  Cc: atomlin, boqun, gary, bjorn3_gh, lossin, aliceryhl, tmgross, dakr,
	daniel.almeida, tamird, acourbot, work, linux-modules,
	rust-for-linux, ardalan, zhiyunq, dzueck, pgovind2

"Priya Bala Govindasamy" <pgovind2@uci.edu> writes:

> The `set_param` function casts `kernel_param.arg` to `*const SetOnce<T>`
> But the Rust module macro in rust/macros/module.rs initializes `arg` with
> `#param_name.as_void_ptr()`, and `#param_name` is a `ModuleParamAccess<T>`,
> not a `SetOnce<T>`.
>
> ModuleParamAccess<T> has default Rust layout but `set_param` accesses its
> first field SetOnce<T> assuming it to be at offset 0. This is not
> guaranteed by Rust and could cause type confusion leading to data
> corruption.
>
> Fix this by casting `kernel_param.arg` to `ModuleParamAccess<T>` and
> then accessing the `value: SetOnce<T>` field.
>
> Fixes: 0b08fc292842 ("rust: introduce module_param module")
> Reported-by: Dylan Zueck <dzueck@uci.edu>
> Assisted-by: LLM
> Signed-off-by: Priya Bala Govindasamy <pgovind2@uci.edu>
> ---
>  rust/kernel/module_param.rs | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/rust/kernel/module_param.rs b/rust/kernel/module_param.rs
> index f9a14765a926..7ae87ffe1d6b 100644
> --- a/rust/kernel/module_param.rs
> +++ b/rust/kernel/module_param.rs
> @@ -75,7 +75,9 @@ pub trait ModuleParam: Sized + Copy {
>          let new_value = T::try_from_param_arg(arg)?;
>
>          // SAFETY: By function safety requirements, this access is safe.
> -        let container = unsafe { &*((*param).__bindgen_anon_1.arg.cast::<SetOnce<T>>()) };
> +        let param_access = unsafe { &*((*param)
> +            .__bindgen_anon_1.arg.cast::<ModuleParamAccess<T>>()) };
> +        let container = &param_access.value;
>
>          container
>              .populate(new_value)
> --
> 2.34.1

This is indeed a bug. Nice catch.

I think the primary reason for this error is that the safety comment is
lacking. It is also covering two distinct unsafe operations. I think we
should rephrase as so:

diff --git a/rust/kernel/module_param.rs b/rust/kernel/module_param.rs
index 7ae87ffe1d6b..1a2fcbf4a5b3 100644
--- a/rust/kernel/module_param.rs
+++ b/rust/kernel/module_param.rs
@@ -74,9 +74,12 @@ pub trait ModuleParam: Sized + Copy {
     crate::error::from_result(|| {
         let new_value = T::try_from_param_arg(arg)?;
 
-        // SAFETY: By function safety requirements, this access is safe.
-        let param_access = unsafe { &*((*param)
-            .__bindgen_anon_1.arg.cast::<ModuleParamAccess<T>>()) };
+        // SAFETY: By function safety requirements, `param` is valid for read.
+        let arg = unsafe { (*param).__bindgen_anon_1.arg };
+        let param_access_ptr = arg.cast::<ModuleParamAccess<T>>();
+        // SAFETY: The `arg` field is initialized with a pointer to a `static ModuleAccessParam` by
+        // the rust module macro. Thus, the pointer is valid for use as a reference.
+        let param_access = unsafe { &*(param_access_ptr) };
         let container = &param_access.value;
 
         container
---

Please also remember to run `make rustfmt` to format the code properly.


Best regards,
Andreas Hindborg



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v2] rust: module_param: Fix potentially incorrect access of `SetOnce<T>`
  2026-09-17 17:43 [PATCH 0/1] rust: module_param: Fix potentially incorrect access of `SetOnce<T>` Priya Bala Govindasamy
  2026-09-17 17:43 ` [PATCH 1/1] " Priya Bala Govindasamy
@ 2026-09-23 20:31 ` Priya Bala Govindasamy
  1 sibling, 0 replies; 6+ messages in thread
From: Priya Bala Govindasamy @ 2026-09-23 20:31 UTC (permalink / raw)
  To: mcgrof, petr.pavlu, da.gomez, samitolvanen, ojeda
  Cc: atomlin, boqun, gary, bjorn3_gh, lossin, a.hindborg, aliceryhl,
	tmgross, dakr, daniel.almeida, tamird, acourbot, work,
	linux-modules, rust-for-linux, stable, ardalan, zhiyunq, dzueck,
	pgovind2

The `set_param` function casts `kernel_param.arg` to `*const SetOnce<T>`
But the Rust module macro in rust/macros/module.rs initializes `arg` with
`#param_name.as_void_ptr()`, and `#param_name` is a `ModuleParamAccess<T>`,
not a `SetOnce<T>`.

ModuleParamAccess<T> has default Rust layout but `set_param` accesses its
first field SetOnce<T> assuming it to be at offset 0. This is not
guaranteed by Rust and could cause type confusion leading to data
corruption.

Fix this by casting `kernel_param.arg` to `ModuleParamAccess<T>` and
then accessing the `value: SetOnce<T>` field.

Fixes: 0b08fc292842 ("rust: introduce module_param module")
Cc: stable@vger.kernel.org
Reported-by: Dylan Zueck <dzueck@uci.edu>
Assisted-by: LLM
Suggested-by: Andreas Hindborg <a.hindborg@kernel.org>
Signed-off-by: Priya Bala Govindasamy <pgovind2@uci.edu>
---
Changes in v2:
 - Split unsafe block into two separate unsafe operations
 - Add safety comments explaining the unsafe operations

 rust/kernel/module_param.rs | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/rust/kernel/module_param.rs b/rust/kernel/module_param.rs
index f9a14765a926..1a2fcbf4a5b3 100644
--- a/rust/kernel/module_param.rs
+++ b/rust/kernel/module_param.rs
@@ -74,8 +74,13 @@ pub trait ModuleParam: Sized + Copy {
     crate::error::from_result(|| {
         let new_value = T::try_from_param_arg(arg)?;
 
-        // SAFETY: By function safety requirements, this access is safe.
-        let container = unsafe { &*((*param).__bindgen_anon_1.arg.cast::<SetOnce<T>>()) };
+        // SAFETY: By function safety requirements, `param` is valid for read.
+        let arg = unsafe { (*param).__bindgen_anon_1.arg };
+        let param_access_ptr = arg.cast::<ModuleParamAccess<T>>();
+        // SAFETY: The `arg` field is initialized with a pointer to a `static ModuleAccessParam` by
+        // the rust module macro. Thus, the pointer is valid for use as a reference.
+        let param_access = unsafe { &*(param_access_ptr) };
+        let container = &param_access.value;
 
         container
             .populate(new_value)
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-09-23 20:31 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-17 17:43 [PATCH 0/1] rust: module_param: Fix potentially incorrect access of `SetOnce<T>` Priya Bala Govindasamy
2026-09-17 17:43 ` [PATCH 1/1] " Priya Bala Govindasamy
2026-09-20 20:12   ` Miguel Ojeda
2026-09-22 12:16   ` Petr Pavlu
2026-09-22 12:38   ` Andreas Hindborg
2026-09-23 20:31 ` [PATCH v2] " Priya Bala Govindasamy

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox