Re: [PATCH v2] rust: module_param: return value by copy from `value`

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Casting a pointer of `ModuleParamAccess<T>` to `SetOnce<T>` invokes undefined behavior due to missing `#[repr(C)]` on `ModuleParamAccess`.
--

commit 9dc35dfddf4037b5b314ed7586fa775219811371
Author: Andreas Hindborg <a.hindborg@kernel.org>

rust: module_param: return value by copy from value

This commit updates parameter access to be more ergonomic for Copy types
by returning the parameter value by copy rather than through a shared
reference. It changes ModuleParamAccess::value to return T by copy when
T: Copy, and renames the previous reference-returning accessor to
value_ref. The in-tree caller in rust_minimal is updated to reflect this
change.

> diff --git a/rust/kernel/module_param.rs b/rust/kernel/module_param.rs
> index 6a8a7a875643a..a33302d1b8a36 100644
> --- a/rust/kernel/module_param.rs
> +++ b/rust/kernel/module_param.rs

[ ... ]

[Severity: High]
This isn't a bug introduced by this patch, but casting the pointer to
SetOnce<T> appears to rely on an unspecified memory layout.

In rust/kernel/module_param.rs:ModuleParamAccess:

pub struct ModuleParamAccess<T> {
    value: SetOnce<T>,
    default: T,
}

Since ModuleParamAccess lacks a #[repr(C)] annotation, the compiler is
free to reorder its fields. If the default field is placed before value
to minimize padding, would the cast in set_param() overwrite the default
field instead?

rust/kernel/module_param.rs:set_param() {
    ...
    // SAFETY: By function safety requirements, this access is safe.
    let container = unsafe {
        &*((*param).__bindgen_anon_1.arg.cast::<SetOnce<T>>())
    };

    container
        .populate(new_value)
    ...
}

Should ModuleParamAccess be marked with #[repr(C)], or should the pointer
be cast back to ModuleParamAccess<T> before accessing the value field?

> @@ -131,10 +131,26 @@ pub const fn new(default: T) -> Self {
>          }
>      }
>  
> +    /// Get a copy of the parameter value.
> +    ///

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260601-modules-value-ref-v2-1-12ebbf0510c9@kernel.org?part=1