linux-kernel - Re: [PATCH v3 4/4] rust: add parameter support to the `module!` macro

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: <87ldvlbd5a.fsf@kernel.org>
Date: Wed, 08 Jan 2025 15:20:33 +0100
From: Andreas Hindborg <a.hindborg@...nel.org>
To: "Miguel Ojeda" <miguel.ojeda.sandonis@...il.com>
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>,  "Alice Ryhl" <aliceryhl@...gle.com>,
  "Masahiro Yamada" <masahiroy@...nel.org>,  "Nathan Chancellor"
 <nathan@...nel.org>,  "Nicolas Schier" <nicolas@...sle.eu>,  "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>
Subject: Re: [PATCH v3 4/4] rust: add parameter support to the `module!` macro

"Miguel Ojeda" <miguel.ojeda.sandonis@...il.com> writes:

> On Wed, Jan 8, 2025 at 1:45 PM Andreas Hindborg <a.hindborg@...nel.org> wrote:
>>
>> I don't think we ever discussed this?
>
> I don't think so -- we discussed other things related to 2025H1 the
> last meeting. Perhaps you/we can bring it up in the next one?
>
>> Version 3 of this patch enabled the unstable feature `sync_unsafe_cell` [1] to
>> avoid `static mut` variables as suggested by Trevor Tross and Benno Lossin [2].
>>
>> As we are moving closer to a new edition, it is now clear that `static mut` is
>> not being deprecated in the 2024 edition as first anticipated [3].
>>
>> Still, `SyncUnsafeCell` allows us to use safe code when referring to the
>> parameter value:
>
>> What do you think?
>
> The justification seems fairly weak... Unless we are fairly confident
> the API will be stable (even if not stabilized right now), I am not
> sure why we would want to do this right now.
>
> Can we provide our own `SyncUnsafeCell` instead in the meantime, if
> you want to keep the advantages?

I like having the shared `ModuleParamAccess` struct to encapsulate the access
to the parameters rather than emitting an enum for every parameter
instance. For that to work without unsafe code when the user accesses
the parameter value, we need a non-mutable static that provides interior
mutability.

I could implement `Sync` for `ModuleParamAccess` or I could add
`kernel::types::SyncUnsafeCell`. Either way is fine for me.

>
>> Returning an error and `pr_warn!` is doable. As far as I can tell, we do
>> not have `WARN_ON_ONCE` yet?
>
> Please see https://lore.kernel.org/rust-for-linux/20241126-pr_once_macros-v4-0-410b8ca9643e@tuta.io/
> in case it helps.

Cool, I can list that as a prerequisite.


Best regards,
Andreas Hindborg