Commit 68bf1022 authored by Gary Guo's avatar Gary Guo Committed by Miguel Ojeda
Browse files

rust: pin-init: fix incorrect accessor reference lifetime 



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 in exactly what we want for these accessors. The safety and
invariant comments of `DropGuard` have been reworked; instead of reasoning
about what caller can do with the guard, express it in a way that the
ownership is transferred to the guard and `forget` takes it back, so the
unsafe operations within the `DropGuard` can be more easily justified.

Fixes: 42415d16 ("rust: pin-init: add references to previously initialized fields")
Cc: stable@vger.kernel.org
Signed-off-by: default avatarGary Guo <gary@garyguo.net>
Link: https://patch.msgid.link/20260427-pin-init-fix-v3-2-496a699674dd@garyguo.net


[ Reworded for missing word. - Miguel ]
Signed-off-by: default avatarMiguel Ojeda <ojeda@kernel.org>
parent 83ac2870

rust/pin-init/internal/src/init.rs

+47 −59
Original line number Diff line number Diff line
@@ -249,18 +249,6 @@ fn init_fields( 
                });

                // Again span for better diagnostics

                let write = quote_spanned!(ident.span()=> ::core::ptr::write);

                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)*

                    {
@@ -268,21 +256,12 @@ 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}");

                    (

                let value_init = if pinned {

                    quote! {

                        // SAFETY:

                        // - `slot` is valid, because we are inside of an initializer closure, we
@@ -290,14 +269,8 @@ fn init_fields( 
                        // - 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) }

                        },

                    )

                    }

                } else {

                    (

                    quote! {

                        // SAFETY: `slot` is valid, because we are inside of an initializer

                        // closure, we return when an error/panic occurs.
@@ -307,12 +280,7 @@ fn init_fields( 
                                &raw mut (*#slot).#ident,

                            )?

                        };

                        },

                        quote! {

                            // SAFETY: TODO

                            unsafe { &mut (*#slot).#ident }

                        },

                    )

                    }

                };

                quote! {

                    #(#attrs)*
@@ -320,9 +288,6 @@ fn init_fields( 
                        let #init = #value;

                        #value_init

                    }

                    #(#cfgs)*

                    #[allow(unused_variables)]

                    let #ident = #accessor;

                }

            }

            InitializerKind::Code { block: value, .. } => quote! {
@@ -335,18 +300,41 @@ 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(#guard.let_binding()) }

                }

            } else {

                quote! {

                    #guard.let_binding()

                }

            };



            res.extend(quote! {

                #(#cfgs)*

                // Create the drop guard:

                // 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 {

                // SAFETY:

                // - `&raw mut (*slot).#ident` is valid.

                // - `make_field_check` checks that `&raw mut (*slot).#ident` is properly aligned.

                // - `(*slot).#ident` has been initialized above.

                // - We only need the ownership to the pointee back when initialization has

                //   succeeded, where we `forget` the guard.

                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);

rust/pin-init/src/__internal.rs

+19 −9
Original line number Diff line number Diff line
@@ -238,32 +238,42 @@ struct Foo { 
/// When a value of this type is dropped, it drops a `T`.

///

/// Can be forgotten to prevent the drop.

///

/// # Invariants

///

/// - `ptr` is valid and properly aligned.

/// - `*ptr` is initialized and owned by this guard.

pub struct DropGuard<T: ?Sized> {

    ptr: *mut T,

}



impl<T: ?Sized> DropGuard<T> {

    /// Creates a new [`DropGuard<T>`]. It will [`ptr::drop_in_place`] `ptr` when it gets dropped.

    /// Creates a drop guard and transfer the ownership of the pointer content.

    ///

    /// # Safety

    /// The ownership is only relinguished if the guard is forgotten via [`core::mem::forget`].

    ///

    /// `ptr` must be a valid pointer.

    /// # Safety

    ///

    /// 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` is valid and properly aligned.

    /// - `*ptr` is initialized, and the ownership is transferred to this guard.

    #[inline]

    pub unsafe fn new(ptr: *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 {

        // SAFETY: Per type invariant.

        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: `self.ptr` is valid, properly aligned and `*self.ptr` is owned by this guard.

        unsafe { ptr::drop_in_place(self.ptr) }

    }

}