linux-kernel - Re: [PATCH] rust: pin-init: add references to previously initialized fields

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: <DCMQVH09L1Y5.3A842FC1NGG5H@kernel.org>
Date: Sun, 07 Sep 2025 19:29:45 +0200
From: "Danilo Krummrich" <dakr@...nel.org>
To: "Benno Lossin" <lossin@...nel.org>
Cc: "Boqun Feng" <boqun.feng@...il.com>, "Miguel Ojeda" <ojeda@...nel.org>,
 "Alex Gaynor" <alex.gaynor@...il.com>, "Gary Guo" <gary@...yguo.net>,
 Björn Roy Baron <bjorn3_gh@...tonmail.com>, "Andreas
 Hindborg" <a.hindborg@...nel.org>, "Alice Ryhl" <aliceryhl@...gle.com>,
 "Trevor Gross" <tmgross@...ch.edu>, "Fiona Behrens" <me@...enk.dev>, "Alban
 Kurti" <kurti@...icto.ai>, "Greg Kroah-Hartman"
 <gregkh@...uxfoundation.org>, "Rafael J. Wysocki" <rafael@...nel.org>,
 "Bjorn Helgaas" <bhelgaas@...gle.com>,
 Krzysztof Wilczy´nski <kwilczynski@...nel.org>,
 <rust-for-linux@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] rust: pin-init: add references to previously
 initialized fields

On Sun Sep 7, 2025 at 10:41 AM CEST, Benno Lossin wrote:
> On Sun Sep 7, 2025 at 4:07 AM CEST, Boqun Feng wrote:
>> On Sat, Sep 06, 2025 at 06:57:04PM -0700, Boqun Feng wrote:
>>> On Sat, Sep 06, 2025 at 12:52:22PM +0200, Danilo Krummrich wrote:
>>> > On Fri Sep 5, 2025 at 7:44 PM CEST, Boqun Feng wrote:
>>> > > On Fri, Sep 05, 2025 at 07:18:25PM +0200, Benno Lossin wrote:
>>> > > [...]
>>> > >> index 606946ff4d7f..1ac0b06fa3b3 100644
>>> > >> --- a/samples/rust/rust_driver_pci.rs
>>> > >> +++ b/samples/rust/rust_driver_pci.rs
>>> > >> @@ -78,8 +78,8 @@ fn probe(pdev: &pci::Device<Core>, info: &Self::IdInfo) -> Result<Pin<KBox<Self>
>>> > >>  
>>> > >>          let drvdata = KBox::pin_init(
>>> > >>              try_pin_init!(Self {
>>> > >> -                pdev: pdev.into(),
>>> > >>                  bar <- pdev.iomap_region_sized::<{ Regs::END }>(0, c_str!("rust_driver_pci")),
>>> > >> +                pdev: pdev.into(),
>>> > >
>>> > > Ok, this example is good enough for me to express the concern here: the
>>> > > variable shadowing behavior seems not straightforward (maybe because in
>>> > > normal Rust initalization expression, no binding is created for
>>> > > previous variables, neither do we have a `let` here).
>>> > >
>>> > > Would the future inplace initialization have the similar behavior? I
>>> > > asked because a natural resolution is adding a special syntax like:
>>> > >
>>> > >     let a = ..;
>>> > >
>>> > >     try_pin_init!(Self {
>>> > >         b: a,
>>> > > 	let a = a.into(); // create the new binding here.
>>> > > 	c: a, // <- use the previous initalized `a`.
>>> > >     }
>>> > 
>>> > Can you please clarify the example? I'm a bit confused that this is not a field
>>> > of Self, so currently this can just be written as:
>>> > 
>>> 
>>> Oh, I could have been more clear: `a` is a field of `Self`, and the
>>> `let` part initalizes it.
>>> 
>>> > 	try_pin_init!(Self {
>>> > 	   b: a,
>>> > 	   c: a.into,
>>> > 	})
>>> > 
>>> > Of course assuming that a is Clone, as the code above does as well.
>>> > 
>>> > So, if we are concerned by the variable shadowing, which I'm less concerned
>>> > about, maybe we can do this:
>>> 
>>> I'm not that concerned to block this, but it does look to me like we are
>>> inventing a new way (and even a different syntax because normal Rust
>>> initialization doesn't create new bindings) to create binding, so I
>>> think I should bring it up.
>>> 
>>> > 
>>> > 	// The "original" `a` and `b`.
>>> > 	let a: A = ...;
>>> > 	let b: B = ...;
>>> > 
>>> > 	try_pin_init!(Self {
>>> > 	   a,                   // Initialize the field only.
>>> > 	   let b <- b,          // Initialize the field and create a `&B` named `b`.
>>> > 	   c: a.into(),         // That's the "original" `a`.
>>> > 	   d <- D::new(b),      // Not the original `b`, but the pin-init one.
>>> > 	})
>>
>> Another idea is using `&this`:
>>
>>  	try_pin_init!(&this in Self {
>>  	   a,                   // Initialize the field only.
>>  	   b <- b,              // Initialize the field only.
>>  	   c: a.into(),         // That's the "original" `a`.
>>  	   d <- D::new(this->b),      // Not the original `b`, but the pin-init one.
>>  	})
>>
>> , like a special field projection during initialization.
>
> The main issue with new syntax is the difficulty of implementing it. The
> let one is fine, but it's pretty jarring & doesn't get formatted by
> rustfmt (which I want to eventually have). Using `this` does look better
> IMO, but it's near impossible to implement using declarative macros
> (even using `syn` it seems difficult to me). So either we find some way
> to express it in existing rust syntax (maybe use an attribute?), or we
> just keep it this way.
>
> Maybe Gary has some ideas on how to implement it.

I also thought about reusing `this`, but I think we should not reuse it. We
still need it to get pointers to uninitialized fields.

Surely, we could say that we provide this.as_ptr() to get the NonNull `this`
is currently defined to be and otherwise make it expose only the initialized
fields for a certain scope.

But as you say, that sounds tricky to implement and is probably not very
intuitive either. I'd rather say keep it as it is, if we don't want something
like the `let b <- b` syntax I proposed for formatting reasons.