fix(arrow-buffer): prevent use-after-free in Buffer::shrink_to_fit when MemoryReservation::resize panics

[Rich-T-kid]

Which issue does this PR close?

• Closes Panics in Buffer::shrink_to_fit() break invariants in arrow-buffer, leading to use of uninitialized memory #10379.

Rationale for this change

Buffer maintains two pointers that must always agree: Buffer::ptr (the raw *const u8 into the allocation) and Bytes::ptr (the NonNull that owns the allocation). Before this fix, shrink_to_fit only updated Buffer::ptr if try_realloc returned Ok. But try_realloc can panic; specifically inside MemoryReservation::resize, a user-supplied callback after the realloc syscall already succeeded and Bytes::ptr was updated. That left the two pointers out of sync: Bytes::ptr pointed to the new (valid) allocation, Buffer::ptr still pointed to the old (freed) allocation. Any caller that used catch_unwind to recover from the panic could then call buf.as_slice() and read freed memory.

What changes are included in this PR?

replaced the conditional if try_realloc(...).is_ok() { update ptr } with a PtrSync drop guard that unconditionally syncs Buffer::ptr to Bytes::ptr + offset when it drops. Since Rust runs destructors during panic unwinding, the guard fires before the panic propagates.

Are these changes tested?

yes. a similar test to what was used in #10379

Are there any user-facing changes?

no

[Rich-T-kid]

@Evian-Zhang

[Evian-Zhang]

Looks good to me! Thank you!

[Rich-T-kid]

Looks good to me! Thank you!

@Evian-Zhang, updated it a bit could you review the PR

[Jefffrey]

seems like theres some real miri failures?

    running 1 test
    test buffer::immutable::tests::test_shrink_to_fit ... 
  stderr ───
    error: Undefined Behavior: trying to retag from <216778> for SharedReadOnly permission at alloc67519[0x10], but that tag does not exist in the borrow stack for this location
       --> arrow-buffer/src/buffer/immutable.rs:238:41
        |
    238 |                     let base = unsafe { (*self.bytes_ptr).as_ptr() };
        |                                         ^^^^^^^^^^^^^^^^^ this error occurs as part of retag at alloc67519[0x10..0x38]
        |
        = help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental
        = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information
    help: <216778> was created by a SharedReadOnly retag at offsets [0x10..0x38]
       --> arrow-buffer/src/buffer/immutable.rs:249:28
        |
    249 |                 bytes_ptr: std::ptr::from_ref::<Bytes>(bytes),
        |                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    help: <216778> was later invalidated at offsets [0x10..0x38] by a Unique function-entry retag inside this call
       --> arrow-buffer/src/buffer/immutable.rs:253:13
        |
    253 |             bytes.try_realloc(desired_capacity).ok();
        |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
        = note: this is on thread `buffer::immutab`
        = note: stack backtrace:
                0: <buffer::immutable::Buffer::shrink_to_fit::PtrSync<'_> as std::ops::Drop>::drop
                    at arrow-buffer/src/buffer/immutable.rs:238:41: 238:58
                1: std::ptr::drop_glue::<buffer::immutable::Buffer::shrink_to_fit::PtrSync<'_>> - shim(Some(buffer::immutable::Buffer::shrink_to_fit::PtrSync<'_>))
                    at /home/runner/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ptr/mod.rs:848:1: 850:25
                2: buffer::immutable::Buffer::shrink_to_fit
                    at arrow-buffer/src/buffer/immutable.rs:254:9: 254:10
                3: buffer::immutable::tests::test_shrink_to_fit
                    at arrow-buffer/src/buffer/immutable.rs:761:9: 761:31
                4: buffer::immutable::tests::test_shrink_to_fit::{closure#0}
                    at arrow-buffer/src/buffer/immutable.rs:750:28: 750:28

    running 1 test
    test test_shrink_to_fit_after_concat ... 
  stderr ───
    error: Undefined Behavior: trying to retag from <2246064> for SharedReadOnly permission at alloc720449[0x10], but that tag does not exist in the borrow stack for this location
     --> arrow-buffer/src/buffer/immutable.rs:238:40
      |
      = note: this error occurs as part of retag at alloc720449[0x10..0x38]
      |
      = help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental
      = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information
    help: <2246064> was created by a SharedReadOnly retag at offsets [0x10..0x38]
     --> arrow-buffer/src/buffer/immutable.rs:249:27
    help: <2246064> was later invalidated at offsets [0x10..0x38] by a Unique function-entry retag inside this call
     --> arrow-buffer/src/buffer/immutable.rs:253:12
      = note: this is on thread `test_shrink_to_`
      = note: stack backtrace:
              0: <arrow::buffer::Buffer::shrink_to_fit::PtrSync<'_> as std::ops::Drop>::drop
                  at arrow-buffer/src/buffer/immutable.rs:238:41: 238:58
              1: std::ptr::drop_glue::<arrow::buffer::Buffer::shrink_to_fit::PtrSync<'_>> - shim(Some(arrow::buffer::Buffer::shrink_to_fit::PtrSync<'_>))
                  at /home/runner/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ptr/mod.rs:848:1: 850:25
              2: arrow::buffer::Buffer::shrink_to_fit
                  at arrow-buffer/src/buffer/immutable.rs:254:9: 254:10
              3: arrow::buffer::ScalarBuffer::<i32>::shrink_to_fit
                  at arrow-buffer/src/buffer/scalar.rs:111:9: 111:36
              4: arrow::buffer::OffsetBuffer::<i32>::shrink_to_fit
                  at arrow-buffer/src/buffer/offset.rs:250:9: 250:31
              5: <arrow::array::GenericListArray<i32> as arrow::array::Array>::shrink_to_fit
                  at arrow-array/src/array/list_array.rs:626:9: 626:43
              6: <std::sync::Arc<(dyn arrow::array::Array + 'static)> as arrow::array::Array>::shrink_to_fit
                  at arrow-array/src/array/mod.rs:481:13: 481:32
              7: test_shrink_to_fit_after_concat::{closure#1}
                  at arrow/tests/shrink_to_fit.rs:47:13: 47:41
              8: memory_use::<std::sync::Arc<dyn arrow::array::Array>, {closure@arrow/tests/shrink_to_fit.rs:45:20: 45:22}>
                  at arrow/tests/shrink_to_fit.rs:155:15: 155:20
              9: test_shrink_to_fit_after_concat
                  at arrow/tests/shrink_to_fit.rs:45:9: 49:11
              10: test_shrink_to_fit_after_concat::{closure#0}
                  at arrow/tests/shrink_to_fit.rs:26:37: 26:37

[Rich-T-kid]

this isn't public + the only call site of this is in buffer/immutable.rs

[Rich-T-kid]

@Jefffrey miri issues are now resolved 👍

[Jefffrey]

makes sense to me

[Jefffrey]

would be nice to keep this comment about failure