[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <aBTMMHWNXS7wK7zS@google.com>
Date: Fri, 2 May 2025 13:44:16 +0000
From: Alice Ryhl <aliceryhl@...gle.com>
To: Andreas Hindborg <a.hindborg@...nel.org>
Cc: Miguel Ojeda <ojeda@...nel.org>, Alex Gaynor <alex.gaynor@...il.com>,
Boqun Feng <boqun.feng@...il.com>, Gary Guo <gary@...yguo.net>,
"Björn Roy Baron" <bjorn3_gh@...tonmail.com>, Benno Lossin <benno.lossin@...ton.me>,
Masahiro Yamada <masahiroy@...nel.org>, Nathan Chancellor <nathan@...nel.org>,
Luis Chamberlain <mcgrof@...nel.org>, Danilo Krummrich <dakr@...nel.org>,
Nicolas Schier <nicolas.schier@...ux.dev>, Trevor Gross <tmgross@...ch.edu>,
Adam Bratschi-Kaye <ark.email@...il.com>, rust-for-linux@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-kbuild@...r.kernel.org,
Petr Pavlu <petr.pavlu@...e.com>, Sami Tolvanen <samitolvanen@...gle.com>,
Daniel Gomez <da.gomez@...sung.com>, Simona Vetter <simona.vetter@...ll.ch>,
Greg KH <gregkh@...uxfoundation.org>, Fiona Behrens <me@...enk.dev>,
Daniel Almeida <daniel.almeida@...labora.com>, linux-modules@...r.kernel.org
Subject: Re: [PATCH v11 2/3] rust: add parameter support to the `module!` macro
On Fri, May 02, 2025 at 02:16:35PM +0200, Andreas Hindborg wrote:
> Add support for module parameters to the `module!` macro. Implement read
> only support for integer types without `sysfs` support.
>
> Acked-by: Petr Pavlu <petr.pavlu@...e.com> # from modules perspective
> Tested-by: Daniel Gomez <da.gomez@...sung.com>
> Signed-off-by: Andreas Hindborg <a.hindborg@...nel.org>
> +unsafe extern "C" fn set_param<T>(
> + val: *const kernel::ffi::c_char,
> + param: *const crate::bindings::kernel_param,
> +) -> core::ffi::c_int
> +where
> + T: ModuleParam,
> +{
> + // NOTE: If we start supporting arguments without values, val _is_ allowed
> + // to be null here.
> + if val.is_null() {
> + // TODO: Use pr_warn_once available.
> + crate::pr_warn!("Null pointer passed to `module_param::set_param`");
> + return EINVAL.to_errno();
> + }
> +
> + // SAFETY: By function safety requirement, val is non-null and
> + // null-terminated. By C API contract, `val` is live and valid for reads
> + // for the duration of this function.
> + let arg = unsafe { CStr::from_char_ptr(val) };
> +
> + crate::error::from_result(|| {
> + let new_value = T::try_from_param_arg(arg)?;
> +
> + // SAFETY: `param` is guaranteed to be valid by C API contract
> + // and `arg` is guaranteed to point to an instance of `T`.
> + let old_value = unsafe { (*param).__bindgen_anon_1.arg as *mut T };
> +
> + // SAFETY: `old_value` is valid for writes, as we have exclusive
> + // access. `old_value` is pointing to an initialized static, and
> + // so it is properly initialized.
> + unsafe { core::ptr::replace(old_value, new_value) };
You don't use the return value of this, so this is equivalent to
unsafe { *old_value = new_value };
> +macro_rules! make_param_ops {
> + ($ops:ident, $ty:ty) => {
> + ///
> + /// Static [`kernel_param_ops`](srctree/include/linux/moduleparam.h)
> + /// struct generated by `make_param_ops`
> + #[doc = concat!("for [`", stringify!($ty), "`].")]
> + pub static $ops: $crate::bindings::kernel_param_ops = $crate::bindings::kernel_param_ops {
> + flags: 0,
> + set: Some(set_param::<$ty>),
> + get: None,
> + free: Some(free::<$ty>),
You could potentially only include `free` if
`core::mem::needs_drop::<T>()` as an optimization.
> + fn emit_params(&mut self, info: &ModuleInfo) {
> + let Some(params) = &info.params else {
> + return;
> + };
> +
> + for param in params {
> + let ops = param_ops_path(¶m.ptype);
> +
> + // Note: The spelling of these fields is dictated by the user space
> + // tool `modinfo`.
> + self.emit_param("parmtype", ¶m.name, ¶m.ptype);
> + self.emit_param("parm", ¶m.name, ¶m.description);
> +
> + write!(
> + self.param_buffer,
> + "
> + pub(crate) static {param_name}:
> + ::kernel::module_param::ModuleParamAccess<{param_type}> =
> + ::kernel::module_param::ModuleParamAccess::new({param_default});
Is this global accessible to the user? It would be a use-after-free to
access it during module teardown. For example, what if I access this
static during its own destructor? Or during the destructor of another
module parameter?
Alice
Powered by blists - more mailing lists