You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@arrow.apache.org by je...@apache.org on 2026/10/22 04:28:02 UTC
(arrow-rs) branch 59_maintenance updated: [59_maintenance] fix(arrow-buffer): prevent use-after-free in Buffer::shrink_to_fit when MemoryReservation::resize panics (#11787)
This is an automated email from the ASF dual-hosted git repository.
Jefffrey pushed a commit to branch 59_maintenance
in repository https://gitbox.apache.org/repos/asf/arrow-rs.git
The following commit(s) were added to refs/heads/59_maintenance by this push:
new a33a8d9de1 [59_maintenance] fix(arrow-buffer): prevent use-after-free in Buffer::shrink_to_fit when MemoryReservation::resize panics (#11787)
a33a8d9de1 is described below
commit a33a8d9de17a5998e7bcc48a0a4142868e12740e
Author: Andrew Lamb <an...@nerdnetworks.org>
AuthorDate: Thu Oct 22 00:27:56 2026 -0400
[59_maintenance] fix(arrow-buffer): prevent use-after-free in Buffer::shrink_to_fit when MemoryReservation::resize panics (#11787)
- Backport https://github.com/apache/arrow-rs/pull/10932
- For inclusion in https://github.com/apache/arrow-rs/issues/11664
Cherry-pick of
https://github.com/apache/arrow-rs/commit/9a8400ea86447d2b83a348285b5554170c7a830c
Conflict resolved by hand in `arrow-buffer/src/buffer/immutable.rs`:
kept the nested `if let` form instead of let-chains (59's MSRV is 1.85).
`bytes.rs` applied cleanly.
Co-authored-by: RIchard Baah <13...@users.noreply.github.com>
Co-authored-by: Jeffrey Vo <je...@gmail.com>
---
arrow-buffer/src/buffer/immutable.rs | 75 ++++++++++++++++++++++++++++++------
arrow-buffer/src/bytes.rs | 11 +++++-
2 files changed, 74 insertions(+), 12 deletions(-)
diff --git a/arrow-buffer/src/buffer/immutable.rs b/arrow-buffer/src/buffer/immutable.rs
index 9c7fc1a901..6c09e9df61 100644
--- a/arrow-buffer/src/buffer/immutable.rs
+++ b/arrow-buffer/src/buffer/immutable.rs
@@ -211,17 +211,16 @@ impl Buffer {
};
if desired_capacity < self.capacity() {
if let Some(bytes) = Arc::get_mut(&mut self.data) {
- if bytes.try_realloc(desired_capacity).is_ok() {
- // Realloc complete - update our pointer into `bytes`:
- self.ptr = if is_empty {
- bytes.as_ptr()
- } else {
- // SAFETY: we kept all elements leading up to the offset
- unsafe { bytes.as_ptr().add(offset) }
- }
- } else {
- // Failure to reallocate is fine; we just failed to free up memory.
- }
+ bytes
+ .try_realloc(desired_capacity, |base| {
+ self.ptr = if is_empty {
+ base.as_ptr()
+ } else {
+ // SAFETY: we kept all elements leading up to the offset
+ unsafe { base.as_ptr().add(offset) }
+ };
+ })
+ .ok(); // Failure to reallocate is fine; we just failed to free up memory.
}
}
}
@@ -1135,4 +1134,58 @@ mod tests {
assert_eq!(buffer_back.as_slice(), expected.as_slice());
}
}
+
+ #[test]
+ #[cfg(feature = "pool")]
+ fn test_shrink_to_fit_panicking_reservation() {
+ use std::panic::{AssertUnwindSafe, catch_unwind};
+
+ use crate::pool::{MemoryPool, MemoryReservation};
+
+ #[derive(Debug)]
+ struct PanicPool;
+
+ #[derive(Debug)]
+ struct PanicReservation {
+ panicked: bool,
+ }
+
+ impl MemoryReservation for PanicReservation {
+ fn size(&self) -> usize {
+ 0
+ }
+ fn resize(&mut self, _: usize) {
+ if !self.panicked {
+ self.panicked = true;
+ panic!("intentional panic in resize");
+ }
+ }
+ }
+
+ impl MemoryPool for PanicPool {
+ fn reserve(&self, _: 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
+ }
+ }
+
+ let pool = PanicPool;
+ let data: Vec<u8> = (0..8).collect();
+ let mut buf = Buffer::from_slice_ref(data.as_slice());
+ buf.claim(&pool);
+
+ // shrink_to_fit panics because PanicReservation::resize panics, but
+ // Buffer::ptr must stay consistent with Bytes::ptr (no use-after-free).
+ let _ = catch_unwind(AssertUnwindSafe(|| buf.shrink_to_fit()));
+
+ assert_eq!(buf.as_slice(), data.as_slice());
+ }
}
diff --git a/arrow-buffer/src/bytes.rs b/arrow-buffer/src/bytes.rs
index a80a347fa1..dd7331b0e3 100644
--- a/arrow-buffer/src/bytes.rs
+++ b/arrow-buffer/src/bytes.rs
@@ -134,7 +134,15 @@ impl Bytes {
/// Returns `Err` if the memory was allocated with a custom allocator,
/// or the call to `realloc` failed, for whatever reason.
/// In case of `Err`, the [`Bytes`] will remain as it was (i.e. have the old size).
- pub(crate) fn try_realloc(&mut self, new_len: usize) -> Result<(), ()> {
+ ///
+ /// `on_reallocated` is called after [`Bytes`] has updated its internal
+ /// pointer, but before resizing the memory reservation, which may call user
+ /// code.
+ pub(crate) fn try_realloc(
+ &mut self,
+ new_len: usize,
+ on_reallocated: impl FnOnce(NonNull<u8>),
+ ) -> Result<(), ()> {
if let Deallocation::Standard(old_layout) = self.deallocation {
if old_layout.size() == new_len {
return Ok(()); // Nothing to do
@@ -162,6 +170,7 @@ impl Bytes {
self.ptr = ptr;
self.len = new_len;
self.deallocation = Deallocation::Standard(new_layout);
+ on_reallocated(ptr);
#[cfg(feature = "pool")]
{