public inbox for rust-for-linux@vger.kernel.org
 help / color / mirror / Atom feed
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

      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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox