From: Benno Lossin <benno.lossin@proton.me>
To: Wedson Almeida Filho <wedsonaf@gmail.com>
Cc: "Miguel Ojeda" <ojeda@kernel.org>,
"Alex Gaynor" <alex.gaynor@gmail.com>,
"Boqun Feng" <boqun.feng@gmail.com>,
"Gary Guo" <gary@garyguo.net>,
"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
"Andreas Hindborg" <a.hindborg@samsung.com>,
"Alice Ryhl" <aliceryhl@google.com>,
"Martin Rodriguez Reboredo" <yakoyoku@gmail.com>,
"Asahi Lina" <lina@asahilina.net>,
"Eric Curtin" <ecurtin@redhat.com>, "Neal Gompa" <neal@gompa.dev>,
"Thomas Bertschinger" <tahbertschinger@gmail.com>,
"Andrea Righi" <andrea.righi@canonical.com>,
"Sumera Priyadarsini" <sylphrenadin@gmail.com>,
"Finn Behrens" <me@kloenk.dev>,
"Adam Bratschi-Kaye" <ark.email@gmail.com>,
stable@vger.kernel.org, "Daniel Xu" <dxu@dxuuu.xyz>,
rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] rust: macros: fix soundness issue in `module!` macro
Date: Mon, 01 Apr 2024 19:27:52 +0000 [thread overview]
Message-ID: <06d5ccdb-a5c3-4630-9f97-9a7bdf7b7a48@proton.me> (raw)
In-Reply-To: <CANeycqr_AkxTj2iNdnjRFrC-C8npsBtS34V4hNy35RpQHszG9w@mail.gmail.com>
On 01.04.24 21:10, Wedson Almeida Filho wrote:
> On Sun, 31 Mar 2024 at 07:27, Benno Lossin <benno.lossin@proton.me> wrote:
>>
>> On 31.03.24 03:00, Wedson Almeida Filho wrote:
>>> On Wed, 27 Mar 2024 at 13:04, Benno Lossin <benno.lossin@proton.me> wrote:
>>>> - fn __init() -> core::ffi::c_int {{
>>>> - match <{type_} as kernel::Module>::init(&THIS_MODULE) {{
>>>> - Ok(m) => {{
>>>> + /// # Safety
>>>> + ///
>>>> + /// This function must
>>>> + /// - only be called once,
>>>> + /// - be called after `__init`,
>>>> + /// - not be called concurrently with `__init`.
>>>
>>> The second item is incomplete: it must be called after `__init` *succeeds*.
>>
>> Indeed.
>>
>>>
>>> With that added (which is a different precondition), I think the third
>>> item can be dropped because if you have to wait to see whether
>>> `__init` succeeded or failed before you can call `__exit`, then
>>> certainly you cannot call it concurrently with `__init`.
>>
>> I would love to drop that requirement, but I am not sure we can. With
>> that requirement, I wanted to ensure that no data race on `__MOD` can
>> happen. If you need to verify that `__init` succeeded, one might think
>> that it is not possible to call `__exit` such that a data race occurs,
>> but I think it could theoretically be done if the concrete `Module`
>> implementation never failed.
>
> I see. If you're concerned about compiler reordering, then we need
> compiler barriers.
>
>> Do you have any suggestion for what I could add to the "be called after
>> `__init` was called and returned `0`" requirement to make a data race
>> impossible?
>
> If you're concerned with reordering from the processor as well, then
> we need cpu barriers. You'd have to say that the cpu/thread executing
> `__init` must have a release barrier after `__init` completes, and the
> thread/cpu doing `__exit` must have an acquire barrier before starting
> `__exit`.
>
> But I'm not sure we need to go that far. Mostly because C is going to
> guarantee that ordering for us, so I'd say we can just omit this or
> perhaps say "This function must only be called from the exit module
> implementation".
Yeah, though I do not exactly know where or what the "exit module
implementation" is. If you are happy with v2, then I think we can go
with that. This piece of code is also not really something people will
need to read.
--
Cheers,
Benno
prev parent reply other threads:[~2024-04-01 19:28 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-27 16:04 [PATCH] rust: macros: fix soundness issue in `module!` macro Benno Lossin
2024-03-31 1:00 ` Wedson Almeida Filho
2024-03-31 10:27 ` Benno Lossin
2024-04-01 19:10 ` Wedson Almeida Filho
2024-04-01 19:27 ` Benno Lossin [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=06d5ccdb-a5c3-4630-9f97-9a7bdf7b7a48@proton.me \
--to=benno.lossin@proton.me \
--cc=a.hindborg@samsung.com \
--cc=alex.gaynor@gmail.com \
--cc=aliceryhl@google.com \
--cc=andrea.righi@canonical.com \
--cc=ark.email@gmail.com \
--cc=bjorn3_gh@protonmail.com \
--cc=boqun.feng@gmail.com \
--cc=dxu@dxuuu.xyz \
--cc=ecurtin@redhat.com \
--cc=gary@garyguo.net \
--cc=lina@asahilina.net \
--cc=linux-kernel@vger.kernel.org \
--cc=me@kloenk.dev \
--cc=neal@gompa.dev \
--cc=ojeda@kernel.org \
--cc=rust-for-linux@vger.kernel.org \
--cc=stable@vger.kernel.org \
--cc=sylphrenadin@gmail.com \
--cc=tahbertschinger@gmail.com \
--cc=wedsonaf@gmail.com \
--cc=yakoyoku@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).