Re: [PATCH 1/1] rust: module_param: Fix potentially incorrect access of `SetOnce<T>`
From: Andreas Hindborg <a.hindborg@kernel.org>
Date: 2026-09-22 12:38:54
Also in:
rust-for-linux
Subsystem:
module support, rust, the rest · Maintainers:
Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen, Miguel Ojeda, Linus Torvalds
"Priya Bala Govindasamy" [off-list ref] writes:
quoted hunk ↗ jump to hunk
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 <redacted> Assisted-by: LLM Signed-off-by: Priya Bala Govindasamy <redacted> --- 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 = ¶m_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 = ¶m_access.value; container ---
Please also remember to run `make rustfmt` to format the code properly. Best regards, Andreas Hindborg