Pointer dereference after free with allocator containing shared state

[jhorstmann]

I'm trying out the allocator api with the goal of being able to track memory usage for parts of a program:

#![feature(allocator_api)]
#![feature(core_intrinsics)]

use std::alloc::{AllocError, Allocator, Layout, System};
use std::intrinsics::black_box;
use std::ptr::NonNull;
use std::sync::atomic::{AtomicUsize, Ordering};
use std::sync::Arc;

#[derive(Default, Clone)]
struct TrackingAllocator {
    memory_usage: Arc<AtomicUsize>,
}

unsafe impl Allocator for TrackingAllocator {
    fn allocate(&self, layout: Layout) -> Result<NonNull<[u8]>, AllocError> {
        self.memory_usage
            .fetch_add(layout.size(), Ordering::Relaxed);
        System.allocate(layout)
    }

    fn allocate_zeroed(&self, layout: Layout) -> Result<NonNull<[u8]>, AllocError> {
        self.memory_usage
            .fetch_add(layout.size(), Ordering::Relaxed);
        System.allocate_zeroed(layout)
    }

    unsafe fn deallocate(&self, ptr: NonNull<u8>, layout: Layout) {
        self.memory_usage
            .fetch_sub(layout.size(), Ordering::Relaxed);
        System.deallocate(ptr, layout)
    }
}

fn bool_to_int(input: Vec<bool, TrackingAllocator>) -> Vec<i32, TrackingAllocator> {
    let allocator = input.allocator().clone();
    let mut output = Vec::with_capacity_in(input.len(), allocator);
    for b in input {
        output.push(b as i32);
    }
    output
}

fn main() {
    let tracking_alloc = TrackingAllocator::default();
    {
        let mut input = Vec::new_in(tracking_alloc.clone());
        input.push(true);
        input.push(false);
        let output = bool_to_int(input);
        black_box(output);
    }
    {
        let mut input = Vec::new_in(tracking_alloc.clone());
        input.push(true);
        input.push(true);
        let output = bool_to_int(input);
        black_box(output);
    }
    eprintln!("{}", tracking_alloc.memory_usage.load(Ordering::Relaxed));
}

I expected this to work (and print 0 at the end), but instead it fails by freeing an invalid pointer:

$ ./target/debug/memory_tracking
free(): invalid pointer
Aborted

Running with miri gives a bit more context:

error: Undefined Behavior: pointer to alloc984 was dereferenced after this allocation got freed
   --> /home/user/.rustup/toolchains/nightly-2022-03-23-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ptr/non_null.rs:328:18
    |
328 |         unsafe { &*self.as_ptr() }
    |                  ^^^^^^^^^^^^^^^ pointer to alloc984 was dereferenced after this allocation got freed
    |
    = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
    = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
            
    = note: inside `std::ptr::NonNull::<alloc::sync::ArcInner<std::sync::atomic::AtomicUsize>>::as_ref` at /home/user/.rustup/toolchains/nightly-2022-03-23-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ptr/non_null.rs:328:18
    = note: inside `std::sync::Arc::<std::sync::atomic::AtomicUsize>::inner` at /home/user/.rustup/toolchains/nightly-2022-03-23-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/sync.rs:1084:18
    = note: inside `<std::sync::Arc<std::sync::atomic::AtomicUsize> as std::clone::Clone>::clone` at /home/user/.rustup/toolchains/nightly-2022-03-23-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/sync.rs:1342:24
note: inside `<TrackingAllocator as std::clone::Clone>::clone` at src/main.rs:12:5
   --> src/main.rs:12:5
    |
10  | #[derive(Default, Clone)]
    |                   ----- in this derive macro expansion
11  | struct TrackingAllocator {
12  |     memory_usage: Arc<AtomicUsize>,
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
note: inside `main` at src/main.rs:54:37
   --> src/main.rs:54:37
    |
54  |         let mut input = Vec::new_in(tracking_alloc.clone());
    |                                     ^^^^^^^^^^^^^^^^^^^^^^
    = note: this error originates in the derive macro `Clone` (in Nightly builds, run with -Z macro-backtrace for more info)

note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace

Meta

$ rustc --version --verbose
rustc 1.61.0-nightly (5f3700105 2022-03-22)
binary: rustc
commit-hash: 5f37001055c29982f4c27ee9edd90449c8e07774
commit-date: 2022-03-22
host: x86_64-unknown-linux-gnu
release: 1.61.0-nightly
LLVM version: 14.0.0

[Urgau]

This seems to be a problem with the into_iter() implementation of Vec because if I do:

-    for b in input {
-        output.push(b as i32);
+    for b in &input {
+        output.push(*b as i32);
    }

There is no crash and no Miri error, this to me indicate that there is a problem in the implementation of IntoIter.

Another interesting things is the message I have locally malloc(): unaligned tcache chunk detected, I don't know what it means but it's certainly interesting.

@rustbot label +T-libs +I-unsound

[tmiasko]

The drop guard used in the implementation of IntoIter makes a raw copy of an allocator when reconstructing RawVec. The allocator is then dropped twice, once by RawVec drop and second time by IntoIter drop.

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

Lines 315 to 317 in d2df372

	let alloc = ptr::read(&self.0.alloc);
	// RawVec handles deallocation
	let _ = RawVec::from_raw_parts_in(self.0.buf.as_ptr(), self.0.cap, alloc);

cc @TimDiekmann #78461.

[orlp]

Minimal reproduction of problem:

#![feature(allocator_api)]

use core::ptr::NonNull;
use core::alloc::{AllocError, Allocator, Layout};

struct PrintOnDrop;
impl Drop for PrintOnDrop {
    fn drop(&mut self) {
        println!("dropped")
    }
}

unsafe impl Allocator for PrintOnDrop {
    fn allocate(&self, _layout: Layout) -> Result<NonNull<[u8]>, AllocError> {
        Err(AllocError)
    }

    unsafe fn deallocate(&self, _ptr: NonNull<u8>, _layout: Layout) { }
}

fn main() {
    Vec::<u32, _>::new_in(PrintOnDrop { }).into_iter();
}

Prints:

dropped
dropped