All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] rust: pin-init: fix incorrect accessor reference lifetime
@ 2026-04-20 17:23 Gary Guo
  0 siblings, 0 replies; only message in thread
From: Gary Guo @ 2026-04-20 17:23 UTC (permalink / raw)
  To: Benno Lossin, Gary Guo, Miguel Ojeda, Boqun Feng,
	Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Danilo Krummrich
  Cc: stable, rust-for-linux, linux-kernel

From: Gary Guo <gary@garyguo.net>

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 `Deref`/`DerefMut` implementation to `DropGuard` and
derive the reference from dereferencing `DropGuard` instead. The lifetime
of `DropGuard` is exactly what we want for these accessors.

The old accessor creation code also has a purpose of preventing unaligned
fields. So a `let _ = &(*#slot).#ident` still needs to be present, but this
no longer has to care about pinning, so it is moved to just before the
guard generation.

Fixes: 42415d163e5d ("rust: pin-init: add references to previously initialized fields")
Cc: stable@vger.kernel.org
Signed-off-by: Gary Guo <gary@garyguo.net>
---
The patch is also available at https://github.com/Rust-for-Linux/pin-init/pull/132
which has regression tests added and runs through pin-init's test suite.
---
 rust/pin-init/internal/src/init.rs | 107 +++++++++++++----------------
 rust/pin-init/src/__internal.rs    |  27 +++++++-
 2 files changed, 73 insertions(+), 61 deletions(-)

diff --git a/rust/pin-init/internal/src/init.rs b/rust/pin-init/internal/src/init.rs
index daa3f1c6466e..c5f848103c92 100644
--- a/rust/pin-init/internal/src/init.rs
+++ b/rust/pin-init/internal/src/init.rs
@@ -249,22 +249,6 @@ fn init_fields(
                 });
                 // Again span for better diagnostics
                 let write = quote_spanned!(ident.span()=> ::core::ptr::write);
-                // NOTE: the field accessor ensures that the initialized field is properly aligned.
-                // Unaligned fields will cause the compiler to emit E0793. We do not support
-                // unaligned fields since `Init::__init` requires an aligned pointer; the call to
-                // `ptr::write` below has the same requirement.
-                let accessor = if pinned {
-                    let project_ident = format_ident!("__project_{ident}");
-                    quote! {
-                        // SAFETY: TODO
-                        unsafe { #data.#project_ident(&mut (*#slot).#ident) }
-                    }
-                } else {
-                    quote! {
-                        // SAFETY: TODO
-                        unsafe { &mut (*#slot).#ident }
-                    }
-                };
                 quote! {
                     #(#attrs)*
                     {
@@ -272,51 +256,31 @@ fn init_fields(
                         // SAFETY: TODO
                         unsafe { #write(&raw mut (*#slot).#ident, #value_ident) };
                     }
-                    #(#cfgs)*
-                    #[allow(unused_variables)]
-                    let #ident = #accessor;
                 }
             }
             InitializerKind::Init { ident, value, .. } => {
                 // Again span for better diagnostics
                 let init = format_ident!("init", span = value.span());
-                // NOTE: the field accessor ensures that the initialized field is properly aligned.
-                // Unaligned fields will cause the compiler to emit E0793. We do not support
-                // unaligned fields since `Init::__init` requires an aligned pointer; the call to
-                // `ptr::write` below has the same requirement.
-                let (value_init, accessor) = if pinned {
-                    let project_ident = format_ident!("__project_{ident}");
-                    (
-                        quote! {
-                            // SAFETY:
-                            // - `slot` is valid, because we are inside of an initializer closure, we
-                            //   return when an error/panic occurs.
-                            // - We also use `#data` to require the correct trait (`Init` or `PinInit`)
-                            //   for `#ident`.
-                            unsafe { #data.#ident(&raw mut (*#slot).#ident, #init)? };
-                        },
-                        quote! {
-                            // SAFETY: TODO
-                            unsafe { #data.#project_ident(&mut (*#slot).#ident) }
-                        },
-                    )
+                let value_init = if pinned {
+                    quote! {
+                        // SAFETY:
+                        // - `slot` is valid, because we are inside of an initializer closure, we
+                        //   return when an error/panic occurs.
+                        // - We also use `#data` to require the correct trait (`Init` or `PinInit`)
+                        //   for `#ident`.
+                        unsafe { #data.#ident(&raw mut (*#slot).#ident, #init)? };
+                    }
                 } else {
-                    (
-                        quote! {
-                            // SAFETY: `slot` is valid, because we are inside of an initializer
-                            // closure, we return when an error/panic occurs.
-                            unsafe {
-                                ::pin_init::Init::__init(
-                                    #init,
-                                    &raw mut (*#slot).#ident,
-                                )?
-                            };
-                        },
-                        quote! {
-                            // SAFETY: TODO
-                            unsafe { &mut (*#slot).#ident }
-                        },
-                    )
+                    quote! {
+                        // SAFETY: `slot` is valid, because we are inside of an initializer
+                        // closure, we return when an error/panic occurs.
+                        unsafe {
+                            ::pin_init::Init::__init(
+                                #init,
+                                &raw mut (*#slot).#ident,
+                            )?
+                        };
+                    }
                 };
                 quote! {
                     #(#attrs)*
@@ -324,9 +288,6 @@ fn init_fields(
                         let #init = #value;
                         #value_init
                     }
-                    #(#cfgs)*
-                    #[allow(unused_variables)]
-                    let #ident = #accessor;
                 }
             }
             InitializerKind::Code { block: value, .. } => quote! {
@@ -339,18 +300,46 @@ fn init_fields(
         if let Some(ident) = kind.ident() {
             // `mixed_site` ensures that the guard is not accessible to the user-controlled code.
             let guard = format_ident!("__{ident}_guard", span = Span::mixed_site());
+
+            // NOTE: The reference is derived from the guard so that it only lives as long as the
+            // guard does and cannot escape the scope. If it's created via `&mut (*#slot).#ident`
+            // like the unaligned field guard, it will become effectively `'static`.
+            let accessor = if pinned {
+                let project_ident = format_ident!("__project_{ident}");
+                quote! {
+                    // SAFETY: the initialization is pinned.
+                    unsafe { #data.#project_ident(&mut *#guard) }
+                }
+            } else {
+                quote! {
+                    &mut *#guard
+                }
+            };
+
             res.extend(quote! {
+                // Take a reference of the field to ensure that the initialized field is
+                // properly aligned. Unaligned fields will cause the compiler to emit E0793. We
+                // do not support unaligned fields since `Init::__init` requires an aligned
+                // pointer; the call to `ptr::write` above has the same requirement.
+                // SAFETY: `slot` is valid.
+                #(#cfgs)*
+                let _ = unsafe { &(*#slot).#ident };
+
                 #(#cfgs)*
                 // Create the drop guard:
                 //
                 // We rely on macro hygiene to make it impossible for users to access this local
                 // variable.
                 // SAFETY: We forget the guard later when initialization has succeeded.
-                let #guard = unsafe {
+                let mut #guard = unsafe {
                     ::pin_init::__internal::DropGuard::new(
                         &raw mut (*slot).#ident
                     )
                 };
+
+                #(#cfgs)*
+                #[allow(unused_variables)]
+                let #ident = #accessor;
             });
             guards.push(guard);
             guard_attrs.push(cfgs);
diff --git a/rust/pin-init/src/__internal.rs b/rust/pin-init/src/__internal.rs
index 90adbdc1893b..d122ab840c44 100644
--- a/rust/pin-init/src/__internal.rs
+++ b/rust/pin-init/src/__internal.rs
@@ -5,6 +5,8 @@
 //! These items must not be used outside of this crate and the pin-init-internal crate located at
 //! `../internal`.
 
+use core::ops::{Deref, DerefMut};
+
 use super::*;
 
 /// See the [nomicon] for what subtyping is. See also [this table].
@@ -238,6 +240,10 @@ struct Foo {
 /// When a value of this type is dropped, it drops a `T`.
 ///
 /// Can be forgotten to prevent the drop.
+///
+/// # Invariants
+///
+/// `ptr` is convertible to a mutable reference and `*ptr` is owned by `DropGuard`.
 pub struct DropGuard<T: ?Sized> {
     ptr: *mut T,
 }
@@ -259,11 +265,28 @@ pub unsafe fn new(ptr: *mut T) -> Self {
     }
 }
 
+impl<T: ?Sized> Deref for DropGuard<T> {
+    type Target = T;
+
+    #[inline]
+    fn deref(&self) -> &T {
+        // SAFETY: `ptr` is convertible to reference
+        unsafe { &*self.ptr }
+    }
+}
+
+impl<T: ?Sized> DerefMut for DropGuard<T> {
+    #[inline]
+    fn deref_mut(&mut self) -> &mut T {
+        // SAFETY: `ptr` is convertible to a mutable reference
+        unsafe { &mut *self.ptr }
+    }
+}
+
 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: `DropGuard` owns the `*ptr`.
         unsafe { ptr::drop_in_place(self.ptr) }
     }
 }

base-commit: 1c7cc4904160c6fc6377564140062d68a3dc93a0
-- 
2.51.2


^ permalink raw reply related	[flat|nested] only message in thread

only message in thread, other threads:[~2026-04-20 17:23 UTC | newest]

Thread overview: (only message) (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-20 17:23 [PATCH] rust: pin-init: fix incorrect accessor reference lifetime Gary Guo

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.