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

Alice Ryhl
2026-04-27T07:44:00Z

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