Double free in Vec::from_iter specialization when drop panics

[Qwaz]

rust/library/alloc/src/vec/source_iter_marker.rs

Lines 71 to 72 in 4a20eb6

	// drop any remaining values at the tail of the source
	src.drop_remaining();

rust/library/alloc/src/vec/into_iter.rs

Lines 88 to 93 in 4a20eb6

	pub(super) fn drop_remaining(&mut self) {
	unsafe {
	ptr::drop_in_place(self.as_mut_slice());
	}
	self.ptr = self.end;
	}

SpecFromIter<T, I> for Vec<T> calls Vec::IntoIter::drop_remaining(). drop_remaining() calls drop_in_place() before overwriting the pointer. As a result, dropped elements are not invalidated and dropped again under panic.

PoC:

#![forbid(unsafe_code)]

use std::iter::FromIterator;

#[derive(Debug)]
enum MyEnum {
    DroppedTwice(Box<i32>),
    PanicOnDrop,
}

impl Drop for MyEnum {
    fn drop(&mut self) {
        match self {
            MyEnum::DroppedTwice(_) => println!("Dropping!"),
            MyEnum::PanicOnDrop => {
                if !std::thread::panicking() {
                    panic!();
                }
            }
        }
    }
}

fn main() {
    let v = vec![MyEnum::DroppedTwice(Box::new(123)), MyEnum::PanicOnDrop];
    Vec::from_iter(v.into_iter().take(0));
}

Output:

Dropping!
thread 'main' panicked at 'explicit panic', src/main.rs:17:21
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
Dropping!
free(): double free detected in tcache 2

Tested with rustc 1.51.0. Here is a playground link to the code snippet.

[the8472]

Thanks, I totally didn't consider that case!
According to #14875 panic during drop should result in the storage being leaked. That shouldn't be too difficult to achieve by forgetting the original allocation before the drop.

The question is whether it is necessary to attempt to drop the stuff moved to output vec or whether we can leak those too.

@rustbot claim

[JohnTitor]

Assigning P-critical as discussed as part of the Prioritization Working Group procedure and removing I-prioritize.