linux-kernel - Re: [PATCH v13 2/6] rust: introduce module

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-Id: <DAQJCUE1C2JE.204A8IS7LBIVZ@kernel.org>
Date: Thu, 19 Jun 2025 15:15:00 +0200
From: "Benno Lossin" <lossin@...nel.org>
To: "Andreas Hindborg" <a.hindborg@...nel.org>, "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>, "Alice Ryhl"
 <aliceryhl@...gle.com>, "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>
Cc: "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 v13 2/6] rust: introduce module_param module

On Thu Jun 12, 2025 at 3:40 PM CEST, Andreas Hindborg wrote:
> +/// A wrapper for kernel parameters.
> +///
> +/// This type is instantiated by the [`module!`] macro when module parameters are
> +/// defined. You should never need to instantiate this type directly.
> +///
> +/// Note: This type is `pub` because it is used by module crates to access
> +/// parameter values.
> +#[repr(transparent)]
> +pub struct ModuleParamAccess<T> {
> +    data: core::cell::UnsafeCell<T>,
> +}
> +
> +// SAFETY: We only create shared references to the contents of this container,
> +// so if `T` is `Sync`, so is `ModuleParamAccess`.
> +unsafe impl<T: Sync> Sync for ModuleParamAccess<T> {}
> +
> +impl<T> ModuleParamAccess<T> {
> +    #[doc(hidden)]
> +    pub const fn new(value: T) -> Self {
> +        Self {
> +            data: core::cell::UnsafeCell::new(value),
> +        }
> +    }
> +
> +    /// Get a shared reference to the parameter value.
> +    // Note: When sysfs access to parameters are enabled, we have to pass in a
> +    // held lock guard here.
> +    pub fn get(&self) -> &T {
> +        // SAFETY: As we only support read only parameters with no sysfs
> +        // exposure, the kernel will not touch the parameter data after module
> +        // initialization.

This should be a type invariant. But I'm having difficulty defining one
that's actually correct: after parsing the parameter, this is written
to, but when is that actually? Would we eventually execute other Rust
code during that time? (for example when we allow custom parameter
parsing)

This function also must never be `const` because of the following:

    module! {
        // ...
        params: {
            my_param: i64 {
                default: 0,
                description: "",
            },
        },
    }

    static BAD: &'static i64 = module_parameters::my_param.get();

AFAIK, this static will be executed before loading module parameters and
thus it makes writing to the parameter UB.

So maybe we should just use some sort of synchronization tool here...

---
Cheers,
Benno

> +        unsafe { &*self.data.get() }
> +    }
> +
> +    /// Get a mutable pointer to the parameter value.
> +    pub const fn as_mut_ptr(&self) -> *mut T {
> +        self.data.get()
> +    }
> +}