Use after free in `bumpalo::Vec::into_iter`

[uselessgoddess]

Miri humiliate the following code:

use bumpalo::{vec, Bump};

fn foo() -> impl Iterator<Item = usize> {
    let arena = Bump::new();
    vec![in &arena; 1, 2, 3].into_iter()
}

fn main() {
    for x in foo() {
        println!("{x}");
    }
}

error: Undefined Behavior: pointer to alloc2451 was dereferenced after this allocation got freed
2207 |                 self.ptr = self.ptr.offset(1);
     |                            ^^^^^^^^^^^^^^^^^^ pointer to alloc2451 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: backtrace:
note: inside `main` at src\main.rs:16:14
    --> src\main.rs:16:14
     |
16   |     for x in foo() {
     |              ^^^^^

Why is that? Burrow into bumpalo into_iter implementations.

// Hm... unsafe. Interesting
unsafe {
    let begin = self.as_mut_ptr();
    // assume(!begin.is_null());
    let end = if mem::size_of::<T>() == 0 {
        arith_offset(begin as *const i8, self.len() as isize) as *const T
    } else {
        // oh no, we forgot the borrowing checker
        begin.add(self.len()) as *const T
    };
    mem::forget(self);
    IntoIter {
        phantom: PhantomData,
        ptr: begin,
        end,
    }
}

fn foo() -> impl Iterator<Item = usize> {
    let arena = Bump::new();
    vec![in &arena; 1, 2, 3].into_iter()
    // <-- bump drop here and free our memory 
    // but ptrs in `IntoIter` still point to this memory
}

[konsumlamm]

See also #164 and #168.

[konsumlamm]

The problem is that this code shouldn't compile in the first place. I think the solution is to add a lifetime parameter to IntoIter (and all the other consuming iterators).

[uselessgoddess]

I think it would be interesting if it were possible to return an IntoIter that would extend the lifetime of the Bump

[fitzgen]

Fixed in #181

[shinmao]

@uselessgoddess hi I felt quite confused about this issue. It seem that in the implementation of into_iter, it already prevented bump from being dropped by mem::forget(); however, why is it still dropped in foo()?

[uselessgoddess]

@shinmao In fact, it is not dropped because of the .into_iter() call. At the end of any scope, local objects are always being dropped. The foo function actually illustrated the undefined behavior resulting from an incorrect implementation of IntoIterators lifetimes. Now this function should not compile, the issue has been closed Fix missing lifetimes PR: https://github.com/fitzgen/bumpalo/pull/181

[shinmao]

fn foo() -> impl Iterator<Item = usize> {
    let arena = Bump::new();
    let v = vec![in &arena; 1, 2, 3];
    v.into_iter()
    // <-- bump drop here and free our memory 
    // but ptrs in `IntoIter` still point to this memory
}

@uselessgoddess thanks for the response. Does it mean v is forgot by into_iter() here, but bump is still dropped? If we insert a line of mem::forget(arena) after v, could it also solve the problem?

[uselessgoddess]

I don't know how to study it properly, I recommend studying moving and Drop trait. Also try running this code with the new version of bumpalo, I hope the compilation error will let you figure it out.