Skip to content

Fix double free when an element's Drop or a predicate panics - #2

Open
tooson9010-spec wants to merge 1 commit into
Mysvac:mainfrom
tooson9010-spec:fix-panic-safety-double-free
Open

Fix double free when an element's Drop or a predicate panics#2
tooson9010-spec wants to merge 1 commit into
Mysvac:mainfrom
tooson9010-spec:fix-panic-safety-double-free

Conversation

@tooson9010-spec

@tooson9010-spec tooson9010-spec commented Aug 27, 2026

Copy link
Copy Markdown

clear and truncate destroy the elements before committing the new length. If
T::drop panics the commit is skipped, so the container keeps a stale length
covering slots that were already destroyed, and its own Drop destroys them
again.

ArrayVec::retain_mut has a second path. It compacts with ptr::copy, which
leaves the source slot as a duplicate, and commits self.len only after the loop
— without the self.len = 0; // Ensure safety if panicked guard that
FastVecData::retain_mut already uses. A panicking predicate is enough here; no
element Drop has to panic.

Reported in #1. Affected: clear and truncate on all three types, plus
ArrayVec::retain_mut (and retain, which delegates to it).

The added tests count destructor calls on a 4-element vec with one armed Drop.
On the current code every element is destroyed twice:

clear: 8 drops for 4 elements

The fix commits the length before the destroying loop, and gives
ArrayVec::retain_mut the same guard FastVecData::retain_mut uses. SmallVec
keeps its MARKER bit.

One regression test per module, gated on feature = "std". cargo test --features std catches it, no sanitizer needed; the rest of the suite passes on
default, --features std and --all-features.

clear and truncate destroy elements before committing the new length, and
ArrayVec::retain_mut compacts with ptr::copy without the pre-loop guard the
other retain_mut implementations already use. A panic in either place leaves
a stale length covering destroyed or duplicated slots, and the container's
own Drop revisits them.

Commit the length first, and give ArrayVec::retain_mut the same guard as
FastVecData::retain_mut.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant