Skip to content

Panics in MutableBuffer::into_buffer() break invariants in arrow-buffer, leading to double free #10378

Description

@Evian-Zhang

Describe the bug

Split from #10281

Sources are located here:

#[inline]
pub(super) fn into_buffer(self) -> Buffer {
let bytes = unsafe { Bytes::new(self.data, self.len, Deallocation::Standard(self.layout)) };
#[cfg(feature = "pool")]
{
let reservation = self.reservation.lock().unwrap().take();
*bytes.reservation.lock().unwrap() = reservation;
}
std::mem::forget(self);
Buffer::from(bytes)
}

If the mutex self.reservation was poisoned before, then the unwrap will leading to panic. However, The mem::forget has not yet been invoked, so both bytes and self's drop implementation will be invoked when unwinding, and these two variable owns the same memory region, leading to a double free (both Bytes and MutableBuffer's drop implementation involves std::alloc::dealloc)

To fix it, I think we can just use if let Some instead of unwrap to the mutex.

To Reproduce

use arrow_buffer::buffer::MutableBuffer;
use arrow_buffer::{MemoryPool, MemoryReservation};
use std::panic::{AssertUnwindSafe, catch_unwind};

#[derive(Debug)]
struct PanicReservation;

impl MemoryReservation for PanicReservation {
    fn size(&self) -> usize {
        0
    }

    fn resize(&mut self, _new_size: usize) {
        panic!("Issue 1: panic in resize to poison the mutex");
    }
}

#[derive(Debug)]
struct PanicPool;

impl MemoryPool for PanicPool {
    fn reserve(&self, _size: usize) -> Box<dyn MemoryReservation> {
        Box::new(PanicReservation)
    }

    fn available(&self) -> isize {
        isize::MAX
    }

    fn used(&self) -> usize {
        0
    }

    fn capacity(&self) -> usize {
        usize::MAX
    }
}

fn main() {
    let pool = PanicPool;
    let mut buf = MutableBuffer::with_capacity(128);
    buf.resize(64, 0);
    buf.claim(&pool);

    let result = catch_unwind(AssertUnwindSafe(|| {
        buf.resize(100, 0);
    }));

    let _ = catch_unwind(AssertUnwindSafe(|| {
        let _b: arrow_buffer::Buffer = buf.into();
    }));
}

Expected behavior

No soundness issue happens

Additional context

No response

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions