From 31585e3de6e557a8e91b2fe01b7e1d1f4ceb5b5d 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 Adds a guard type that safely initializes an array by running an initializer on each element, keeping track of the number of initialized elements. In the case of a panic or error in the per-element initializer, the guard drops the already-initialized portion of the array; nothing is dropped on success. 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. Link: https://github.com/Rust-for-Linux/pin-init/issues/136 Reported-by: Gary Guo Suggested-by: Gary Guo Signed-off-by: Mirko Adzic --- src/lib.rs | 156 ++++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 114 insertions(+), 42 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 80c476e6..42e94674 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1163,6 +1163,116 @@ pub fn uninit() -> impl Init, E> { unsafe { init_from_closure(|_| Ok(())) } } +/// Allows safe (pinned and non-pinned) initialization of an array. +/// +/// Drops the already initialized elements of the array if an error or panic occurs +/// partway through the initialization process. +/// +/// # Invariants +/// +/// If `ptr` is not null: +/// - `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 ArrayInit { + /// A pointer to the first element of the array. + ptr: *mut T, + /// The number of initialized elements in the array. + num_init: usize, + /// Initialization function factory. + make_init: F, +} + +impl ArrayInit { + #[inline] + fn new(make_init: F) -> Self { + Self { + // INVARIANT: `ptr` is null prior to any initialization. + ptr: core::ptr::null_mut(), + num_init: 0, + make_init, + } + } +} + +/// SAFETY: On success, all `N` elements of the array have been initialized through +/// `I: Init`. On error or panic, the elements that have been initialized so far are +/// dropped, thus leaving the array uninitialized and ready to deallocate. The `Init` +/// implementation executes the same code as that of `PinInit`. +unsafe impl Init<[T; N], E> for ArrayInit +where + F: FnMut(usize) -> I, + I: Init, +{ + unsafe fn __init(mut self, slot: *mut [T; N]) -> Result<(), E> { + debug_assert!(!slot.is_null()); + // INVARIANT: `self.ptr` is non-null once initialization starts. + self.ptr = slot.cast::(); + for i in 0..N { + // INVARIANT: Elements `self.ptr[..self.num_init]` have been initialized + // thus far. This hold true for every `self.num_init = i`. + self.num_init = i; + let init = (self.make_init)(i); + // SAFETY: `self.ptr.add(i)` is in bounds. + let ptr = unsafe { self.ptr.add(i) }; + // SAFETY: The pointer is derived from `slot` with a valid offset. It is + // thus valid for writes and points uninitialized memory. + unsafe { init.__init(ptr) }?; + } + // INVARIANT: `self.ptr` is null after the array is initialized. + self.ptr = core::ptr::null_mut(); + Ok(()) + } +} + +/// SAFETY: On success, all `N` elements of the array have been initialized through +/// `I`. Since `I: PinInit` guarantees that the pinning invariants of `T` are upheld, +/// the guarantees of `[T; N]` are also upheld. 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> { + debug_assert!(!slot.is_null()); + // INVARIANT: `self.ptr` is non-null once initialization starts. + self.ptr = slot.cast::(); + for i in 0..N { + // INVARIANT: Elements `self.ptr[..self.num_init]` have been initialized + // thus far. This hold true for every `self.num_init = i`. + self.num_init = i; + let init = (self.make_init)(i); + // SAFETY: `self.ptr.add(i)` is in bounds. + let ptr = unsafe { self.ptr.add(i) }; + // SAFETY: The pointer is derived from `slot` with a valid offset. It is + // thus valid for writes and points uninitialized memory. + unsafe { init.__pinned_init(ptr) }?; + } + // INVARIANT: `self.ptr` is null after the array is initialized. + self.ptr = core::ptr::null_mut(); + Ok(()) + } +} + +impl Drop for ArrayInit { + fn drop(&mut self) { + if self.ptr.is_null() { + // Since `self.ptr` is null - either no initialization has been attempted + // or the array was successfully initialized, per type invariant. Nothing + // to drop. + return; + } + + // SAFETY: Since `self.ptr` is not null - the initialization has failed + // partway. Drop `self.ptr[..self.num_init]` which are initialized per + // type invariant. + unsafe { + core::ptr::drop_in_place(core::ptr::slice_from_raw_parts_mut(self.ptr, self.num_init)) + }; + } +} + /// Initializes an array by initializing each element via the provided initializer. /// /// # Examples @@ -1174,31 +1284,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::new(make_init) } /// Initializes an array by initializing each element via the provided initializer. @@ -1217,31 +1308,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::new(make_init) } /// Construct an initializer in a closure and run it. From 76d9d65e6ecb0221483c2342b637be95ed8b351c 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. Link: https://github.com/Rust-for-Linux/pin-init/issues/136 Reported-by: Gary Guo Suggested-by: Gary Guo Signed-off-by: Mirko Adzic --- src/lib.rs | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 42e94674..55388464 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -966,12 +966,17 @@ 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. + // SAFETY: The above call initialized `slot`. The guard drops it if `self.1` returns + // `Err` or panics. + let guard = unsafe { __internal::DropGuard::new(slot) }; + // SAFETY: `slot` was initialized above. Excluding the guard (which only drops) we + // have unique access to it. 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) }) + (self.1)(val)?; + core::mem::forget(guard); + Ok(()) } } @@ -1073,10 +1078,14 @@ 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) }) + // SAFETY: The above call initialized `slot`. The guard drops it if `self.1` returns + // `Err` or panics. + let guard = unsafe { __internal::DropGuard::new(slot) }; + // SAFETY: `slot` was initialized above. Excluding the guard (which only drops) we + // have unique access to it. + (self.1)(unsafe { &mut *slot })?; + core::mem::forget(guard); + Ok(()) } } From 59950fffe2f31b604a96dfac9684c95fc275751d 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 Cover both fixes added in the series: - `[pin_]init_array_from_fn`: a panic or error from element `i`'s initializer drops the previously initialized elements `0..i`. - `[pin_]chain`: a panic or error from the chained closure drops the value initialized by the first stage. Also assert no double-drop on the success paths. Signed-off-by: Mirko Adzic --- tests/unwind_safety.rs | 266 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 266 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..20040629 --- /dev/null +++ b/tests/unwind_safety.rs @@ -0,0 +1,266 @@ +//! Regression tests for: +//! +//! 1. the unwind-safety fix in `[pin_]init_array_from_fn`: a panic or error during +//! element `i` initialization must drop the previously initialized elements `0..i`. +//! 2. the unwind-safety fix in `[pin_]chain`: a panic or error from the chained +//! closure must drop the value initialized by the first stage. +//! +//! For more information, see: https://github.com/Rust-for-Linux/pin-init/issues/136. + +#![cfg(any(feature = "std", feature = "alloc"))] +#![cfg_attr(feature = "alloc", feature(allocator_api))] + +use core::pin::Pin; +use core::sync::atomic::{AtomicUsize, Ordering}; +use std::panic::{catch_unwind, AssertUnwindSafe}; + +use pin_init::*; + +#[allow(unused_attributes)] +#[path = "../examples/error.rs"] +mod error; +use error::Error; + +struct Counted<'a>(&'a AtomicUsize); + +impl<'a> Drop for Counted<'a> { + fn drop(&mut self) { + self.0.fetch_add(1, Ordering::Relaxed); + } +} + +fn maybe_panicking_init( + count: &AtomicUsize, + should_panic: bool, +) -> impl Init, core::convert::Infallible> { + // SAFETY: on `Ok(())` we have written a valid `Counted` into `slot`; + // on panic we never wrote, so `slot` is left uninitialized as required. + unsafe { + init_from_closure(move |slot: *mut Counted| { + assert!(!should_panic); + slot.write(Counted(count)); + Ok(()) + }) + } +} + +fn maybe_panicking_pin_init( + count: &AtomicUsize, + should_panic: bool, +) -> impl PinInit, core::convert::Infallible> { + // SAFETY: on `Ok(())` we have written a valid `Counted` into `slot`; + // on panic we never wrote, so `slot` is left uninitialized as required. + // + // `Counted: Unpin`, so pinning invariants are trivial. + unsafe { + pin_init_from_closure(move |slot: *mut Counted| { + assert!(!should_panic); + slot.write(Counted(count)); + Ok(()) + }) + } +} + +fn fallible_init(count: &AtomicUsize, should_error: bool) -> impl Init, Error> { + // SAFETY: on `Ok(())` we have written a valid `Counted` into `slot`; + // on `Err(Error)` we never wrote, so `slot` is left uninitialized as required. + unsafe { + init_from_closure(move |slot: *mut Counted| { + if should_error { + Err(Error) + } else { + slot.write(Counted(count)); + Ok(()) + } + }) + } +} + +fn fallible_pin_init(count: &AtomicUsize, should_error: bool) -> impl PinInit, Error> { + // SAFETY: on `Ok(())` we have written a valid `Counted` into `slot`; + // on `Err(Error)` we never wrote, so `slot` is left uninitialized as required. + // + // `Counted: Unpin`, so pinning invariants are trivial. + unsafe { + pin_init_from_closure(move |slot: *mut Counted| { + if should_error { + Err(Error) + } else { + slot.write(Counted(count)); + 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 func = AssertUnwindSafe(|| { + let init = init_array_from_fn(|i| { + let should_panic = i == panic_at; + maybe_panicking_init(&drops, should_panic) + }); + let _array: Result, _> = Box::init(init); + }); + let result = catch_unwind(func); + 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 func = AssertUnwindSafe(|| { + let init = pin_init_array_from_fn(|i| { + let should_panic = i == panic_at; + maybe_panicking_pin_init(&drops, should_panic) + }); + let _array: Result>, _> = Box::pin_init(init); + }); + let result = catch_unwind(func); + 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 init = init_array_from_fn(|i| { + let should_error = i == error_at; + fallible_init(&drops, should_error) + }); + let result: Result, _> = Box::try_init(init); + 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); + let init = pin_init_array_from_fn(|i| { + let should_error = i == error_at; + fallible_pin_init(&drops, should_error) + }); + let result: Result>, _> = Box::try_pin_init(init); + 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 init = init_array_from_fn(|_| maybe_panicking_init(&drops, false)); + let result: Result, _> = Box::init(init); + assert!(result.is_ok()); + 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); + { + let pin_init = pin_init_array_from_fn(|_| maybe_panicking_pin_init(&drops, false)); + let result: Result>, _> = Box::pin_init(pin_init); + assert!(result.is_ok()); + assert_eq!(drops.load(Ordering::Relaxed), 0); + } + assert_eq!(drops.load(Ordering::Relaxed), N); +} + +#[test] +fn chain_init_drops_first_stage_on_panic() { + let drops = AtomicUsize::new(0); + let func = AssertUnwindSafe(|| { + let init = maybe_panicking_init(&drops, false).chain( + |_| -> Result<(), core::convert::Infallible> { + panic!(); + }, + ); + let _: Result, _> = Box::init(init); + }); + let result = catch_unwind(func); + assert!(result.is_err()); + assert_eq!(drops.load(Ordering::Relaxed), 1); +} + +#[test] +fn chain_pin_init_drops_first_stage_on_panic() { + let drops = AtomicUsize::new(0); + let func = AssertUnwindSafe(|| { + let init = maybe_panicking_pin_init(&drops, false).pin_chain( + |_| -> Result<(), core::convert::Infallible> { + panic!(); + }, + ); + let _: Result>, _> = Box::pin_init(init); + }); + let result = catch_unwind(func); + assert!(result.is_err()); + assert_eq!(drops.load(Ordering::Relaxed), 1); +} + +#[test] +fn chain_init_drops_first_stage_on_error() { + let drops = AtomicUsize::new(0); + let init = fallible_init(&drops, false).chain(|_| Err(Error)); + let result: Result, _> = Box::try_init(init); + assert!(result.is_err()); + assert_eq!(drops.load(Ordering::Relaxed), 1); +} + +#[test] +fn chain_pin_init_drops_first_stage_on_error() { + let drops = AtomicUsize::new(0); + let init = fallible_pin_init(&drops, false).pin_chain(|_| Err(Error)); + let result: Result>, _> = Box::try_pin_init(init); + 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 init = maybe_panicking_init(&drops, false).chain(|_| Ok(())); + let result: Result, _> = Box::init(init); + assert!(result.is_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); + { + let init = maybe_panicking_pin_init(&drops, false).pin_chain(|_| Ok(())); + let result: Result>, _> = Box::pin_init(init); + assert!(result.is_ok()); + assert_eq!(drops.load(Ordering::Relaxed), 0); + } + assert_eq!(drops.load(Ordering::Relaxed), 1); +}