From: Greg KH <gregkh@linuxfoundation.org>
To: Gary Guo <gary@garyguo.net>
Cc: "Benno Lossin" <lossin@kernel.org>,
"Miguel Ojeda" <ojeda@kernel.org>,
"Boqun Feng" <boqun.feng@gmail.com>,
"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
"Andreas Hindborg" <a.hindborg@kernel.org>,
"Alice Ryhl" <aliceryhl@google.com>,
"Trevor Gross" <tmgross@umich.edu>,
"Danilo Krummrich" <dakr@kernel.org>,
"Tim Chirananthavat" <theemathas@gmail.com>,
stable@vger.kernel.org, rust-for-linux@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH 6.19.y 2/2] rust: pin-init: replace shadowed return token by `unsafe`-to-create token
Date: Tue, 31 Mar 2026 15:53:14 +0200 [thread overview]
Message-ID: <2026033159-jaundice-crayon-aecb@gregkh> (raw)
In-Reply-To: <DHH0AG3OW3RO.O79RKFY0VR85@garyguo.net>
On Tue, Mar 31, 2026 at 02:36:21PM +0100, Gary Guo wrote:
> On Tue Mar 31, 2026 at 11:17 AM BST, Greg KH wrote:
> > On Wed, Mar 25, 2026 at 01:57:32PM +0100, Benno Lossin wrote:
> >> [ Upstream commit fdbaa9d2b78e0da9e1aeb303bbdc3adfe6d8e749 ]
> >>
> >> We use a unit struct `__InitOk` in the closure generated by the
> >> initializer macros as the return value. We shadow it by creating a
> >> struct with the same name again inside of the closure, preventing early
> >> returns of `Ok` in the initializer (before all fields have been
> >> initialized).
> >>
> >> In the face of Type Alias Impl Trait (TAIT) and the next trait solver,
> >> this solution no longer works [1]. The shadowed struct can be named
> >> through type inference. In addition, there is an RFC proposing to add
> >> the feature of path inference to Rust, which would similarly allow [2].
> >>
> >> Thus remove the shadowed token and replace it with an `unsafe` to create
> >> token.
> >>
> >> The reason we initially used the shadowing solution was because an
> >> alternative solution used a builder pattern. Gary writes [3]:
> >>
> >> In the early builder-pattern based InitOk, having a single InitOk
> >> type for token is unsound because one can launder an InitOk token
> >> used for one place to another initializer. I used a branded lifetime
> >> solution, and then you figured out that using a shadowed type would
> >> work better because nobody could construct it at all.
> >>
> >> The laundering issue does not apply to the approach we ended up with
> >> today.
> >>
> >> With this change, the example by Tim Chirananthavat in [1] no longer
> >> compiles and results in this error:
> >>
> >> error: cannot construct `pin_init::__internal::InitOk` with struct literal syntax due to private fields
> >> --> src/main.rs:26:17
> >> |
> >> 26 | InferredType {}
> >> | ^^^^^^^^^^^^
> >> |
> >> = note: private field `0` that was not provided
> >> help: you might have meant to use the `new` associated function
> >> |
> >> 26 - InferredType {}
> >> 26 + InferredType::new()
> >> |
> >>
> >> Applying the suggestion of using the `::new()` function, results in
> >> another expected error:
> >>
> >> error[E0133]: call to unsafe function `pin_init::__internal::InitOk::new` is unsafe and requires unsafe block
> >> --> src/main.rs:26:17
> >> |
> >> 26 | InferredType::new()
> >> | ^^^^^^^^^^^^^^^^^^^ call to unsafe function
> >> |
> >> = note: consult the function's documentation for information on how to avoid undefined behavior
> >>
> >> Reported-by: Tim Chirananthavat <theemathas@gmail.com>
> >> Link: https://github.com/rust-lang/rust/issues/153535 [1]
> >> Link: https://github.com/rust-lang/rfcs/pull/3444#issuecomment-4016145373 [2]
> >> Link: https://github.com/rust-lang/rust/issues/153535#issuecomment-4017620804 [3]
> >> Fixes: fc6c6baa1f40 ("rust: init: add initialization macros")
> >> Cc: stable@vger.kernel.org
> >> Signed-off-by: Benno Lossin <lossin@kernel.org>
> >> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
> >> Reviewed-by: Gary Guo <gary@garyguo.net>
> >> Link: https://patch.msgid.link/20260311105056.1425041-1-lossin@kernel.org
> >> [ Added period as mentioned. - Miguel ]
> >> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
> >> [ Moved to declarative macro, because 6.19.y and earlier do not have
> >> `syn`. - Benno ]
> >> Signed-off-by: Benno Lossin <lossin@kernel.org>
> >
> > This commit blows up the build for me with lots of errors like:
> >
> > error[E0433]: failed to resolve: could not find `init` in `$crate`
> > --> rust/kernel/maple_tree.rs:393:20
> > |
> > 393 | let tree = pin_init!(MapleTree {
> > | ____________________^
> > 394 | | // SAFETY: This initializes a maple tree into a pinned slot. The maple tree will be
> > 395 | | // destroyed in Drop before the memory location becomes invalid.
> > 396 | | tree <- Opaque::ffi_init(|slot| unsafe {
> > ... |
> > 399 | | _p: PhantomData,
> > 400 | | });
> > | |__________^ could not find `init` in `$crate`
> > |
> > = note: this error originates in the macro `$crate::__init_internal` which comes from the expansion of the macro `pin_init` (in Nightly builds, run with -Z macro-backtrace for more info)
> >
> >
> >
> > So I'll take patch 1 here, but not this one :(
>
> This was dicussed in a RfL call, and we think it's fine to not backport this
> specific patch.
>
> The soundness issue that this patch is fixing relies on code deliberately
> triggering this, a future Rust compiler that enables path inference without
> feature gates, and people using that compiler to build an old LTS/stable kernel.
>
> On the other hand, the other two (or 1 for recent enough stable versions)
> patches have known broken out-of-tree users. So it's good that they're
> backported as code could have relied on them by accident.
Ok, then we are all set, thanks for the backports.
greg k-h
prev parent reply other threads:[~2026-03-31 13:53 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-03-25 12:57 [PATCH 6.19.y 1/2] rust: pin-init: internal: init: document load-bearing fact of field accessors Benno Lossin
2026-03-25 12:57 ` [PATCH 6.19.y 2/2] rust: pin-init: replace shadowed return token by `unsafe`-to-create token Benno Lossin
2026-03-31 10:17 ` Greg KH
2026-03-31 13:36 ` Gary Guo
2026-03-31 13:53 ` Greg KH [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=2026033159-jaundice-crayon-aecb@gregkh \
--to=gregkh@linuxfoundation.org \
--cc=a.hindborg@kernel.org \
--cc=aliceryhl@google.com \
--cc=bjorn3_gh@protonmail.com \
--cc=boqun.feng@gmail.com \
--cc=dakr@kernel.org \
--cc=gary@garyguo.net \
--cc=linux-kernel@vger.kernel.org \
--cc=lossin@kernel.org \
--cc=ojeda@kernel.org \
--cc=rust-for-linux@vger.kernel.org \
--cc=stable@vger.kernel.org \
--cc=theemathas@gmail.com \
--cc=tmgross@umich.edu \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.