From adaa2ab0c30fb75e2e43dc50e4450b0cdc5a033c Mon Sep 17 00:00:00 2001 From: Mirko Adzic Date: Sun, 26 Apr 2026 13:07:02 +0200 Subject: [PATCH 1/3] make `[pin_]init_array_from_fn` unwind safe The previous code only ran cleanup on the explicit error path. If the per- element initializer panicked partway through, the elements already written into the array would be leaked: their `Drop` impls would never run. This violates the pinning requirement. Fix the unwind safety issue by adding a guard type that drops element on both error and panic path. To avoid having to duplicate code between `pin_init_array_from_fn` and the non-pin variant, extract the code to a shared `ArrayInit` type; this type is internal and not visible via API. Reported-by: Gary Guo Closes: https://github.com/Rust-for-Linux/pin-init/issues/136 Signed-off-by: Mirko Adzic [ Split guard type and the initializer type, move the guard type to be within __pinned_init. - Gary ] Co-developed-by: Gary Guo Signed-off-by: Gary Guo --- src/lib.rs | 122 +++++++++++++++++++++++++++++++++++------------------ 1 file changed, 80 insertions(+), 42 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index fd40c8f2..ca2c5ca6 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1193,6 +1193,82 @@ pub fn uninit() -> impl Init, E> { unsafe { init_from_closure(|_| Ok(())) } } +/// Array initializer from element initializer. +struct ArrayInit(F, __internal::PhantomInvariant); + +// SAFETY: On success, all `N` elements of the array have been initialized. On error or panic, the +// elements that have been initialized so far are dropped, thus leaving the array uninitialized and +// ready to deallocate. +unsafe impl PinInit<[T; N], E> for ArrayInit +where + F: FnMut(usize) -> I, + I: PinInit, +{ + unsafe fn __pinned_init(mut self, slot: *mut [T; N]) -> Result<(), E> { + /// # Invariants + /// + /// - `ptr[..num_init]` contains initialized elements of type `T` + /// - `ptr[num_init..N]` (where N is the size of the array) contains uninitialized memory + struct ArrayInitGuard { + /// A pointer to the first element of the array. + ptr: *mut T, + /// The number of initialized elements in the array. + num_init: usize, + } + + impl Drop for ArrayInitGuard { + #[inline] + fn drop(&mut self) { + // SAFETY: Per type invariant, `self.ptr[..self.num_init]` are initialized. + unsafe { + core::ptr::drop_in_place(core::ptr::slice_from_raw_parts_mut( + self.ptr, + self.num_init, + )) + }; + } + } + + // INVARIANT: nothing is initialized yet. + let mut guard = ArrayInitGuard { + ptr: slot.cast::(), + num_init: 0, + }; + + for i in 0..N { + // INVARIANT: Elements `self.ptr[..self.num_init]` have been initialized + // thus far. This holds true for every `self.num_init = i`. + guard.num_init = i; + + let init = (self.0)(i); + // SAFETY: + // - The subslot is derived from `slot` with a valid offset. + // - If `Err` is touched, the subslot is not touched further, the guard will drop + // previously initialized elements only. + // - `slot` is pinned so is the subslot. + unsafe { init.__pinned_init(&raw mut (*slot)[i]) }?; + } + + // Dismiss the drop guard now that all elements are initialized. + core::mem::forget(guard); + Ok(()) + } +} + +// SAFETY: Follows the `PinInit` impl. `__init` executes the same code as `__pinned_init`. +unsafe impl Init<[T; N], E> for ArrayInit +where + F: FnMut(usize) -> I, + I: Init, +{ + #[inline(always)] + unsafe fn __init(self, slot: *mut [T; N]) -> Result<(), E> { + // SAFETY: `I: Init` cancels out the pinning requirement on subslots. The other safety + // requirements follow that of `__init`. + unsafe { self.__pinned_init(slot) } + } +} + /// Initializes an array by initializing each element via the provided initializer. /// /// # Examples @@ -1204,31 +1280,12 @@ pub fn uninit() -> impl Init, E> { /// assert_eq!(array.len(), 1_000); /// ``` pub fn init_array_from_fn( - mut make_init: impl FnMut(usize) -> I, + make_init: impl FnMut(usize) -> I, ) -> impl Init<[T; N], E> where I: Init, { - let init = move |slot: *mut [T; N]| { - let slot = slot.cast::(); - for i in 0..N { - let init = make_init(i); - // SAFETY: Since 0 <= `i` < N, it is still in bounds of `[T; N]`. - let ptr = unsafe { slot.add(i) }; - // SAFETY: The pointer is derived from `slot` and thus satisfies the `__init` - // requirements. - if let Err(e) = unsafe { init.__init(ptr) } { - // SAFETY: The loop has initialized the elements `slot[0..i]` and since we return - // `Err` below, `slot` will be considered uninitialized memory. - unsafe { ptr::drop_in_place(ptr::slice_from_raw_parts_mut(slot, i)) }; - return Err(e); - } - } - Ok(()) - }; - // SAFETY: The initializer above initializes every element of the array. On failure it drops - // any initialized elements and returns `Err`. - unsafe { init_from_closure(init) } + ArrayInit(make_init, __internal::PhantomInvariant::new()) } /// Initializes an array by initializing each element via the provided initializer. @@ -1247,31 +1304,12 @@ where /// assert_eq!(array.len(), 1_000); /// ``` pub fn pin_init_array_from_fn( - mut make_init: impl FnMut(usize) -> I, + make_init: impl FnMut(usize) -> I, ) -> impl PinInit<[T; N], E> where I: PinInit, { - let init = move |slot: *mut [T; N]| { - let slot = slot.cast::(); - for i in 0..N { - let init = make_init(i); - // SAFETY: Since 0 <= `i` < N, it is still in bounds of `[T; N]`. - let ptr = unsafe { slot.add(i) }; - // SAFETY: The pointer is derived from `slot` and thus satisfies the `__init` - // requirements. - if let Err(e) = unsafe { init.__pinned_init(ptr) } { - // SAFETY: The loop has initialized the elements `slot[0..i]` and since we return - // `Err` below, `slot` will be considered uninitialized memory. - unsafe { ptr::drop_in_place(ptr::slice_from_raw_parts_mut(slot, i)) }; - return Err(e); - } - } - Ok(()) - }; - // SAFETY: The initializer above initializes every element of the array. On failure it drops - // any initialized elements and returns `Err`. - unsafe { pin_init_from_closure(init) } + ArrayInit(make_init, __internal::PhantomInvariant::new()) } /// Construct an initializer in a closure and run it. From 48a8903b447e00f249f5709a3059a14849002de9 Mon Sep 17 00:00:00 2001 From: Mirko Adzic Date: Sun, 26 Apr 2026 20:26:37 +0200 Subject: [PATCH 2/3] make `[pin_]chain` unwind safe Adds a drop guard before the call to the chained closure so that the value initialized by the first stage is dropped if the closure errors or panics; `mem::forget` the guard on success. The previous code only ran cleanup on the explicit error path, leaking the first-stage value if the chained closure panicked. Reported-by: Gary Guo Closes: https://github.com/Rust-for-Linux/pin-init/issues/136 Suggested-by: Gary Guo Signed-off-by: Mirko Adzic [ Fix Clippy safety comment false positive when `slot` and `guard` creation are merged in a single line. - Gary ] Signed-off-by: Gary Guo --- src/lib.rs | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index ca2c5ca6..d68eb0a8 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -965,13 +965,11 @@ where { unsafe fn __pinned_init(self, slot: *mut T) -> Result<(), E> { // SAFETY: All requirements fulfilled since this function is `__pinned_init`. - unsafe { self.0.__pinned_init(slot)? }; - // SAFETY: The above call initialized `slot` and we still have unique access. - let val = unsafe { &mut *slot }; - // SAFETY: `slot` is considered pinned. - let val = unsafe { Pin::new_unchecked(val) }; - // SAFETY: `slot` was initialized above. - (self.1)(val).inspect_err(|_| unsafe { core::ptr::drop_in_place(slot) }) + let slot = unsafe { __internal::Slot::<__internal::Pinned, _>::new(slot) }; + let mut guard = slot.init(self.0)?; + (self.1)(guard.let_binding())?; + core::mem::forget(guard); + Ok(()) } } @@ -1072,11 +1070,11 @@ where { unsafe fn __init(self, slot: *mut T) -> Result<(), E> { // SAFETY: All requirements fulfilled since this function is `__init`. - unsafe { self.0.__pinned_init(slot)? }; - // SAFETY: The above call initialized `slot` and we still have unique access. - (self.1)(unsafe { &mut *slot }).inspect_err(|_| - // SAFETY: `slot` was initialized above. - unsafe { core::ptr::drop_in_place(slot) }) + let slot = unsafe { __internal::Slot::<__internal::Unpinned, _>::new(slot) }; + let mut guard = slot.init(self.0)?; + (self.1)(guard.let_binding())?; + core::mem::forget(guard); + Ok(()) } } From 48f3139bf555804f2534c7984f720e183d9b2bde Mon Sep 17 00:00:00 2001 From: Mirko Adzic Date: Sun, 26 Apr 2026 20:14:15 +0200 Subject: [PATCH 3/3] test: regression tests for unwind safety Add test for `[pin_]init_array_from_fn` and `[pin_]chain` that tests various error and panicking cases to ensure safety. Also assert no double-drop on the success paths. Signed-off-by: Mirko Adzic [ Code cleanups. - Gary ] Signed-off-by: Gary Guo --- tests/unwind_safety.rs | 246 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 246 insertions(+) create mode 100644 tests/unwind_safety.rs diff --git a/tests/unwind_safety.rs b/tests/unwind_safety.rs new file mode 100644 index 00000000..75976afe --- /dev/null +++ b/tests/unwind_safety.rs @@ -0,0 +1,246 @@ +//! Regression tests for unwind safety of `[pin_]init_array_from_fn` and `[pin_]chain`. See +//! https://github.com/Rust-for-Linux/pin-init/issues/136. + +use core::{ + mem::MaybeUninit, + sync::atomic::{AtomicUsize, Ordering}, +}; +use std::pin::Pin; + +use pin_init::*; + +fn try_stack_init(init: impl Init) -> Result { + let mut value = MaybeUninit::uninit(); + // SAFETY: `value` provides a valid uninitialized slot, and on error we return without touching + // the slot further. + unsafe { init.__init(value.as_mut_ptr())? }; + // SAFETY: `value` is initialized. + Ok(unsafe { value.assume_init() }) +} + +fn stack_init(init: impl Init) -> T { + match try_stack_init(init) { + Ok(value) => value, + Err(err) => match err {}, + } +} + +#[pin_data(PinnedDrop)] +struct CountDrop<'a> { + counter: &'a AtomicUsize, +} + +#[pinned_drop] +impl<'a> PinnedDrop for CountDrop<'a> { + fn drop(self: Pin<&mut Self>) { + self.counter.fetch_add(1, Ordering::Relaxed); + } +} + +fn maybe_panicking_init(counter: &AtomicUsize, should_panic: bool) -> impl Init> { + init!(CountDrop { + _: { + assert!(!should_panic); + }, + counter, + }) +} + +fn maybe_panicking_pin_init( + counter: &AtomicUsize, + should_panic: bool, +) -> impl PinInit> { + pin_init!(CountDrop { + _: { + assert!(!should_panic); + }, + counter, + }) +} + +fn maybe_error_init(counter: &AtomicUsize, should_error: bool) -> impl Init, ()> { + // SAFETY: on `Ok(())` we have written a valid `CountDrop` into `slot`; + // on `Err(())` we never wrote, so `slot` is left uninitialized as required. + unsafe { + init_from_closure(move |slot: *mut CountDrop| { + if should_error { + Err(()) + } else { + slot.write(CountDrop { counter }); + Ok(()) + } + }) + } +} + +fn maybe_error_pin_init( + counter: &AtomicUsize, + should_error: bool, +) -> impl PinInit, ()> { + // SAFETY: on `Ok(())` we have written a valid `CountDrop` into `slot`; + // on `Err(())` we never wrote, so `slot` is left uninitialized as required. + // + // `CountDrop: Unpin`, so pinning invariants are trivial. + unsafe { + pin_init_from_closure(move |slot: *mut CountDrop| { + if should_error { + Err(()) + } else { + slot.write(CountDrop { counter }); + Ok(()) + } + }) + } +} + +#[test] +fn init_array_from_fn_drops_initialized_prefix_on_panic() { + const N: usize = 10; + + for panic_at in [0, 5, N - 1] { + let drops = AtomicUsize::new(0); + let result = std::panic::catch_unwind(|| { + let _array: [CountDrop<'_>; N] = stack_init(init_array_from_fn(|i| { + maybe_panicking_init(&drops, i == panic_at) + })); + }); + assert!(result.is_err()); + assert_eq!(drops.load(Ordering::Relaxed), panic_at); + } +} + +#[test] +fn pin_init_array_from_fn_drops_initialized_prefix_on_panic() { + const N: usize = 10; + + for panic_at in [0, 5, N - 1] { + let drops = AtomicUsize::new(0); + let result = std::panic::catch_unwind(|| { + stack_pin_init!(let _array: [CountDrop; N] = pin_init_array_from_fn(|i| { + maybe_panicking_pin_init(&drops, i == panic_at) + })); + }); + assert!(result.is_err()); + assert_eq!(drops.load(Ordering::Relaxed), panic_at); + } +} + +#[test] +fn init_array_from_fn_drops_initialized_prefix_on_error() { + const N: usize = 10; + + for error_at in [0, 5, N - 1] { + let drops = AtomicUsize::new(0); + let result: Result<[CountDrop<'_>; N], ()> = try_stack_init(init_array_from_fn(|i| { + maybe_error_init(&drops, i == error_at) + })); + assert!(result.is_err()); + assert_eq!(drops.load(Ordering::Relaxed), error_at); + } +} + +#[test] +fn pin_init_array_from_fn_drops_initialized_prefix_on_error() { + const N: usize = 10; + + for error_at in [0, 5, N - 1] { + let drops = AtomicUsize::new(0); + stack_try_pin_init!(let result: [CountDrop; N] = pin_init_array_from_fn(|i| { + maybe_error_pin_init(&drops, i == error_at) + })); + assert!(result.is_err()); + assert_eq!(drops.load(Ordering::Relaxed), error_at); + } +} + +#[test] +fn init_array_from_fn_no_double_drop_on_success() { + const N: usize = 8; + + let drops = AtomicUsize::new(0); + { + let _array: [CountDrop<'_>; N] = + stack_init(init_array_from_fn(|_| maybe_panicking_init(&drops, false))); + assert_eq!(drops.load(Ordering::Relaxed), 0); + } + assert_eq!(drops.load(Ordering::Relaxed), N); +} + +#[test] +fn pin_init_array_from_fn_no_double_drop_on_success() { + const N: usize = 8; + + let drops = AtomicUsize::new(0); + { + stack_pin_init!(let _array: [CountDrop; N] = + pin_init_array_from_fn(|_| maybe_panicking_pin_init(&drops, false)) + ); + assert_eq!(drops.load(Ordering::Relaxed), 0); + } + assert_eq!(drops.load(Ordering::Relaxed), N); +} + +#[test] +fn chain_init_drops_on_panic() { + let drops = AtomicUsize::new(0); + let result = std::panic::catch_unwind(|| { + let _count_drop: CountDrop<'_> = + stack_init(maybe_panicking_init(&drops, false).chain(|_| panic!())); + }); + assert!(result.is_err()); + assert_eq!(drops.load(Ordering::Relaxed), 1); +} + +#[test] +fn chain_pin_init_drops_on_panic() { + let drops = AtomicUsize::new(0); + let result = std::panic::catch_unwind(|| { + stack_pin_init!(let _count_drop: CountDrop = + maybe_panicking_pin_init(&drops, false).pin_chain(|_| panic!()) + ); + }); + assert!(result.is_err()); + assert_eq!(drops.load(Ordering::Relaxed), 1); +} + +#[test] +fn chain_init_drops_on_error() { + let drops = AtomicUsize::new(0); + let result: Result, ()> = + try_stack_init(maybe_error_init(&drops, false).chain(|_| Err(()))); + assert!(result.is_err()); + assert_eq!(drops.load(Ordering::Relaxed), 1); +} + +#[test] +fn chain_pin_init_drops_on_error() { + let drops = AtomicUsize::new(0); + stack_try_pin_init!(let result: CountDrop = + maybe_error_init(&drops, false).pin_chain(|_| Err(())) + ); + assert!(result.is_err()); + assert_eq!(drops.load(Ordering::Relaxed), 1); +} + +#[test] +fn chain_init_no_double_drop_on_success() { + let drops = AtomicUsize::new(0); + { + let _count_drop: CountDrop<'_> = + stack_init(maybe_panicking_init(&drops, false).chain(|_| Ok(()))); + assert_eq!(drops.load(Ordering::Relaxed), 0); + } + assert_eq!(drops.load(Ordering::Relaxed), 1); +} + +#[test] +fn chain_pin_init_no_double_drop_on_success() { + let drops = AtomicUsize::new(0); + { + stack_pin_init!(let _count_drop: CountDrop = + maybe_panicking_init(&drops, false).pin_chain(|_| Ok(())) + ); + assert_eq!(drops.load(Ordering::Relaxed), 0); + } + assert_eq!(drops.load(Ordering::Relaxed), 1); +}