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] [thread-next>] [day] [month] [year] [list]

Message-Id: <DB0LKI8BO3HZ.3FF03JN4364RM@kernel.org>
Date: Tue, 01 Jul 2025 11:05:43 +0200
From: "Benno Lossin" <lossin@...nel.org>
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>, "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>, "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 Tue Jul 1, 2025 at 10:43 AM CEST, Andreas Hindborg wrote:
> "Benno Lossin" <lossin@...nel.org> writes:
>> On Mon Jun 30, 2025 at 3:15 PM CEST, Andreas Hindborg wrote:
>>> "Benno Lossin" <lossin@...nel.org> writes:
>>>> On Mon Jun 30, 2025 at 1:18 PM CEST, Andreas Hindborg wrote:
>>>>> "Benno Lossin" <lossin@...nel.org> writes:
>>>>>> (no idea if the orderings are correct, I always have to think way to
>>>>>> much about that... especially since our atomics seem to only take one
>>>>>> ordering in compare_exchange?)
>>>>>>
>>>>>>> As far as I can tell, atomics may not land in v6.17, so this series
>>>>>>> will probably not be ready for merge until v6.18 at the earliest.
>>>>>>
>>>>>> Yeah, sorry about that :(
>>>>>
>>>>> Actually, perhaps we could aim at merging this code without this
>>>>> synchronization?
>>>>
>>>> I won't remember this issue in a few weeks and I fear that it will just
>>>> get buried. In fact, I already had to re-read now what the actual issue
>>>> was...
>>>>
>>>>> The lack of synchronization is only a problem if we
>>>>> support custom parsing. This patch set does not allow custom parsing
>>>>> code, so it does not suffer this issue.
>>>>
>>>> ... In doing that, I saw my original example of UB:
>>>>
>>>>     module! {
>>>>         // ...
>>>>         params: {
>>>>             my_param: i64 {
>>>>                 default: 0,
>>>>                 description: "",
>>>>             },
>>>>         },
>>>>     }
>>>>
>>>>     static BAD: &'static i64 = module_parameters::my_param.get();
>>>>
>>>> That can happen without custom parsing, so it's still a problem...
>>>
>>> Ah, got it. Thanks.
>>
>> On second thought, we *could* just make the accessor function `unsafe`.
>> Of course with a pinky promise to make the implementation safe once
>> atomics land. But I think if it helps you get your driver faster along,
>> then we should do it.
>
> No, I am OK for now with configfs.
>
> But, progress is still great. How about if we add a copy accessor
> instead for now, I think you proposed that a few million emails ago:
>
>     pub fn get(&self) -> T;
>
> or maybe rename:
>
>     pub fn copy(&self) -> T;
>
> Then we are fine safety wise for now, right? It is even sensible for
> these `T: Copy` types.

That is better than getting a reference, but still someone could read at
the same time that a write is happening (though we need some new
abstractions AFAIK?). But I fear that we forget about this issue,
because it'll be some time until we land parameters that are `!Copy` (if
at all...)

---
Cheers,
Benno