Evian-Zhang opened a new issue, #10379:
URL: https://github.com/apache/arrow-rs/issues/10379

   ### Describe the bug
   
   Split from #10281 
   
   Sources are located here:
   
   
https://github.com/apache/arrow-rs/blob/d9919194eef5ecca7f3b42726359a820825b30e6/arrow-buffer/src/buffer/immutable.rs#L194-L227
   
   
https://github.com/apache/arrow-rs/blob/d9919194eef5ecca7f3b42726359a820825b30e6/arrow-buffer/src/bytes.rs#L131-L178
   
   In `Buffer::shrink_to_fit`, it will invokes `Bytes::try_realloc` to 
reallocate memories, and update the `ptr` if the pointer is updated. In 
`Bytes::try_realloc`, **after** the memory hold by `Bytes` is reallocated, 
`self.resize_reservation()` will be invoked. `Bytes::resize_reservation` will 
further invokes user-defined custom reservation function. If such reservation 
function panics, in view of buffer, the memory is in fact reallocated, but 
`self.ptr` has not updated yet. If user uses `catch_unwind` to recover from 
unwinding, methods such as `Buffer::as_slice` will let user access 
freed-memories, leading to use-after-free.
   
   I can't come up with a graceful simple solution to fix it. For me, there are 
three approaches:
   
   * Declare `shrink_to_fit` as unsafe, and requires the memory pool should not 
panic in the `SAFETY` requirement
   * Modify `Bytes::try_realloc` to accept a callback function, which will be 
invoked after the reallocating and before calling `self.resize_reservation()`
   * Before calling `Bytes::try_realloc`, let `self.ptr` points to a static 
valid memory buffer. If `try_realloc` succeeds, `self.ptr` will still be 
updated correctly; If it panics, at least there will be no memory-related 
issues.
   
   
   ### To Reproduce
   
   ```rust
   use arrow_buffer::Buffer;
   use arrow_buffer::{MemoryPool, MemoryReservation};
   use std::panic::{AssertUnwindSafe, catch_unwind};
   
   #[derive(Debug)]
   struct PanicReservation {
       panicked: bool,
   }
   
   impl MemoryReservation for PanicReservation {
       fn size(&self) -> usize {
           0
       }
   
       fn resize(&mut self, _new_size: usize) {
           if !self.panicked {
               self.panicked = true;
               panic!("Issue 2: panic in resize during try_realloc → Bytes.ptr 
updated but Buffer.ptr not");
           }
       }
   }
   
   #[derive(Debug)]
   struct PanicPool;
   
   impl MemoryPool for PanicPool {
       fn reserve(&self, _size: usize) -> Box<dyn MemoryReservation> {
           Box::new(PanicReservation { panicked: false })
       }
   
       fn available(&self) -> isize {
           isize::MAX
       }
   
       fn used(&self) -> usize {
           0
       }
   
       fn capacity(&self) -> usize {
           usize::MAX
       }
   }
   
   fn main() {
       let pool = PanicPool;
   
       // Use from_slice_ref: capacity is rounded up to 64 (via 
MutableBuffer::with_capacity)
       // while length is the actual slice length. This gives us excess 
capacity to shrink.
       let data: &[u8] = &[0xAA, 0xBB, 0xCC, 0xDD, 0xEE, 0xFF, 0x11, 0x22];
       let mut buf = Buffer::from_slice_ref(data);
       buf.claim(&pool);
   
       let result = catch_unwind(AssertUnwindSafe(|| {
           buf.shrink_to_fit();
       }));
   
       let slice = buf.as_slice();
       println!("as_slice() returned {} bytes", slice.len());
       if !slice.is_empty() {
           println!(
               "first bytes: {:02x?}",
               &slice[..slice.len().min(8)]
           );
       }
   }
   ```
   
   ### Expected behavior
   
   No soundness issue happens
   
   ### Additional context
   
   _No response_


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to