Re: [PATCH v2 2/2] rust: pin-init: fix incorrect accessor reference lifetime

[Alice Ryhl <aliceryhl@google.com>]

On Fri, Apr 24, 2026 at 08:10:06PM +0100, Gary Guo wrote:
> On Thu Apr 23, 2026 at 3:51 PM BST, Gary Guo wrote:
> > When a field has been initialized, `init!`/`pin_init!` create a reference
> > or pinned reference to the field so it can be accessed later during the
> > initialization of other fields. However, the reference it created is
> > incorrectly `&'static` rather than just the scope of the initializer.
> >
> > This means that you can do
> >
> >     init!(Foo {
> >         a: 1,
> >         _: {
> >             let b: &'static u32 = a;
> >         }
> >     })
> >
> > which is unsound.
> >
> > This is caused by `&mut (*#slot).#ident`, which actually allows arbitrary
> > lifetime, so this is effectively `'static`. Somewhat ironically, the safety
> > justification of creating the accessor is.. "SAFETY: TODO".
> >
> > Fix it by adding `let_binding` method on `DropGuard` to shorten lifetime.
> > This results exactly what we want for these accessors.
> >
> > Fixes: 42415d163e5d ("rust: pin-init: add references to previously initialized fields")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Gary Guo <gary@garyguo.net>
> > ---
> >  rust/pin-init/internal/src/init.rs | 104 ++++++++++++++++---------------------
> >  rust/pin-init/src/__internal.rs    |  31 ++++++-----
> >  2 files changed, 62 insertions(+), 73 deletions(-)
> 
> > diff --git a/rust/pin-init/src/__internal.rs b/rust/pin-init/src/__internal.rs
> > index 90adbdc1893b..c3fd7589fd82 100644
> > --- a/rust/pin-init/src/__internal.rs
> > +++ b/rust/pin-init/src/__internal.rs
> > @@ -238,32 +238,37 @@ struct Foo {
> >  /// When a value of this type is dropped, it drops a `T`.
> >  ///
> >  /// Can be forgotten to prevent the drop.
> > -pub struct DropGuard<T: ?Sized> {
> > -    ptr: *mut T,
> > +///
> > +/// # Invariants
> > +///
> > +/// `ptr` will not be accessed or dropped after `DropGuard` is dropped.
> > +pub struct DropGuard<'a, T: ?Sized> {
> > +    ptr: &'a mut T,
> >  }
> >  
> > -impl<T: ?Sized> DropGuard<T> {
> > +impl<'a, T: ?Sized> DropGuard<'a, T> {
> >      /// Creates a new [`DropGuard<T>`]. It will [`ptr::drop_in_place`] `ptr` when it gets dropped.
> >      ///
> >      /// # Safety
> >      ///
> > -    /// `ptr` must be a valid pointer.
> > -    ///
> > -    /// It is the callers responsibility that `self` will only get dropped if the pointee of `ptr`:
> > -    /// - has not been dropped,
> > -    /// - is not accessible by any other means,
> > -    /// - will not be dropped by any other means.
> > +    /// `ptr` must not be accessed or dropped after `DropGuard` is dropped.
> >      #[inline]
> > -    pub unsafe fn new(ptr: *mut T) -> Self {
> > +    pub unsafe fn new(ptr: &'a mut T) -> Self {
> > +        // INVARIANT: By safety requirement.
> >          Self { ptr }
> >      }
> > +
> > +    /// Create a let binding for accessor use.
> > +    #[inline]
> > +    pub fn let_binding(&mut self) -> &mut T {
> > +        self.ptr
> > +    }
> >  }
> >  
> > -impl<T: ?Sized> Drop for DropGuard<T> {
> > +impl<T: ?Sized> Drop for DropGuard<'_, T> {
> >      #[inline]
> >      fn drop(&mut self) {
> > -        // SAFETY: A `DropGuard` can only be constructed using the unsafe `new` function
> > -        // ensuring that this operation is safe.
> > +        // SAFETY: `self.ptr` is not going to be accessed or dropped later.
> >          unsafe { ptr::drop_in_place(self.ptr) }
> >      }
> >  }
> 
> Sashiko mentions that:
> 
> > When ptr::drop_in_place(self.ptr) is called here, the value is dropped,
> > but the DropGuard struct still holds the &'a mut T field until the
> > drop method completely returns.
> > 
> > Would it be better to revert DropGuard to store a raw pointer and use
> > unsafe { &mut *self.ptr } in let_binding instead?
> > 
> > The lifetime-shortening effect is fully achieved by the let_binding
> > signature taking &mut self and returning &mut T, which ties the returned
> > reference to the local borrow of the guard variable. This avoids the
> > potential validity issues while fully preserving the bug fix.
> 
> which has a point but not totally correct as the code is not violating the
> validity invariants of references, just the safety invariants. And since no code
> executed can observe the violation, the code is not undefined. The code passes
> all Miri checks which pin-init CI runs with both aliasing models.
> 
> I only used reference here because it's more convenient to do so (less safety
> comments to write), but if the effect is that it's harder to justify the
> correctness (and apparently Sashiko got confused here), then it's not worth
> doing and I should just spell out all safety comments repetitively.
> 
> I'll send a new version with the approach reverted to pointers. PATCH 1/2 will
> be kept as is.

Sounds reasonable to me.

Alice