fix(ffi): read byte array offsets unaligned in buffer_len by AndreaBozzo · Pull Request #12010 · apache/arrow-rs

AndreaBozzo · 2026-10-31T11:21:33Z

Which issue does this PR close?

Part of Array data imported through the FFI might contain unaligned buffers #7136.

Rationale for this change

Since #10030, from_ffi realigns under-aligned buffers, but two reads happen before that and assume alignment: buffer_len dereferences the last offset of a Utf8, Binary, LargeUtf8 or LargeBinary array, and buffers views the variadic buffer lengths of a Utf8View or BinaryView array as an &[i64]. The C data interface allows either to be under-aligned, and when they are, a debug build aborts and a release build has undefined behaviour. Found with arrow-abi-doctor, whose alignment cells place utf8 offsets one byte off.

What changes are included in this PR?

Both reads now go through read_unaligned: the last offset in ImportedArrowArray::buffer_len, and the variadic buffer lengths in ImportedArrowArray::buffers, which are still borrowed in place when aligned and copied only when not.

Are these changes tested?

Yes. test_bytes_with_unaligned_offsets imports each of the four byte array types with its offsets one byte off alignment, built as ArrayData and exported through to_ffi, so it runs without force_validate. test_views_with_unaligned_variadic_buffer_lengths hand-builds a C array with the variadic buffer lengths of both view types one byte off, since FFI_ArrowArray::new always exports them aligned. Reverting any of the three reads makes them abort.

Are there any significant user-facing changes?

No.

Closes apache#7136.

AndreaBozzo · 2026-10-31T11:25:10Z

Left one out on purpose to keep this small: the variadic buffer lengths of the view types (from_raw_parts(lengths.cast::<i64>(), ...) in buffers) assume alignment the same way. Happy to add it here or in a follow-up, whichever you prefer.

AndreaBozzo · 2026-10-31T11:41:19Z

            let data = array.to_data();
            // The same offsets, one byte past an aligned address.
-            let mut offsets = vec![0_u8];
+            let mut offsets = arrow_buffer::MutableBuffer::from_len_zeroed(1);


changed this since on MIRI the one byte shift can land aligned and fail

Jefffrey

Left one out on purpose to keep this small: the variadic buffer lengths of the view types (from_raw_parts(lengths.cast::<i64>(), ...) in buffers) assume alignment the same way. Happy to add it here or in a follow-up, whichever you prefer.

I'm fine with either, but if we leave it to a followup we should avoid closing the issue with this PR

Jefffrey · 2026-11-04T06:12:07Z

+    fn test_bytes_with_unaligned_offsets() -> Result<()> {
+        // The C layout of `FFI_ArrowArray`, filled in by hand as a C producer would.
+        #[repr(C)]
+        struct RawArrowArray {


are we handrolling this struct to better mimic an end to end representation of the issue? or is it because creating the requisite misaligned data via FFI_ArrowArray (via ArrayData) is not possible?

Mostly the second: ArrayDataBuilder::build rejects the misaligned offsets, and build_unchecked still validates under force_validate. The #10034 test gets around it by building FixedSizeBinary and importing as Decimal128, but there's no such pun for byte array offsets, and FFI_ArrowArray's fields are private, so the test fills in the C struct itself, which is also what a C producer hands over.

we can just gate the test under not(force_validate) since CI should still run with & without force validate for arrow-array; it should make the test setup simpler that way, using arraydata instead of redefining the C data interface as a struct there

done for the bytes test in b4600c5. i couldnt manage to get rid of the hand built array for the views one, no proper way to get the lengths unaligned from ArrayData since FFI_ArrowArray::new always writes them into a fresh aligned buffer

AndreaBozzo · 2026-11-04T09:00:38Z

I'll add it here then, so the PR keeps closing the issue.

felipecrv · 2026-11-04T15:26:58Z

This PR is good, but it's not a fix for the issue I reported.

...and a release build has undefined behaviour.

IIRC, the behavior is defined: it's a panic. It can't be UB because that would be a security hole. We either fix by realigning the buffers or panic. Letting unaligned buffers enter a system will cause problems since Rust code assumes alignment.

AndreaBozzo · 2026-11-04T15:37:45Z

This PR is good, but it's not a fix for the issue I reported.

...and a release build has undefined behaviour.

IIRC, the behavior is defined: it's a panic. It can't be UB because that would be a security hole. We either fix by realigning the buffers or panic. Letting unaligned buffers enter a system will cause problems since Rust code assumes alignment.

Hi @felipecrv , i'm fine with not closing your ticket so i'll modify the body to only refer it

on the panic point, the alignment check is debug only, release does misaligned load

felipecrv · 2026-11-04T16:24:21Z

Hi @felipecrv , i'm fine with not closing your ticket so i'll modify the body to only refer it

Thanks!

on the panic point, the alignment check is debug only, release does misaligned load

Scary stuff 👻

alamb · 2026-11-04T21:17:47Z

Part of #7136.

What is the implication of a unaligned load? On x86 I think the hardware makes it work efficiently in hardware. On Arm or other architectures, would it cause some sort of SEGFAULT/SIGBUS (aka process crash) or is there some other failure mode ?

AndreaBozzo · 2026-11-04T21:46:46Z

What is the implication of a unaligned load?

Good question. I ran it on GitHub runners (Xeon, Neoverse N2, Apple M1) and with Miri. In release builds the old code reads the right values everywhere, no crash. In debug builds Rust's alignment check aborts the process. Miri flags both reads on main as UB and passes with this PR.

Arm does fault on some misaligned accesses. On both Arm machines an 8 byte atomic load that crosses a 16 byte boundary got SIGBUS, but plain loads like these two work on these CPUs. So it works today, but it's still UB and the compiler is free to assume alignment, which read_unaligned takes away.

Probe and results are in this run https://github.com/AndreaBozzo/arrow-rs/actions/runs/41917860455

Jefffrey · 2026-11-05T06:08:55Z

This PR is good, but it's not a fix for the issue I reported.

...and a release build has undefined behaviour.

IIRC, the behavior is defined: it's a panic. It can't be UB because that would be a security hole. We either fix by realigning the buffers or panic. Letting unaligned buffers enter a system will cause problems since Rust code assumes alignment.

@felipecrv could you clarify what is needed to close the issue? per #10030 (also linked in PR body), unaligned buffers are automatically aligned, so this PR is just a bug fix for cases where we can still panic on unaligned inputs. once we shore up this hole (and assuming no other bugs), the ffi functions should be able to take unaligned input safely without error/panic

Jefffrey · 2026-11-05T06:36:34Z

+                .cast::<i64>();
+            // SAFETY: if lengths is non-null, then it must be valid for up to num_variadic_buffers.
+            (0..num_variadic_buffers)
+                .map(|i| unsafe { lengths.add(i).read_unaligned() })


it might be better to use a Cow here, to avoid allocation in the aligned case; then we only copy to aligned buffer for unaligned case

done in 392cddf

Jefffrey · 2026-11-05T06:37:43Z

+    fn test_bytes_with_unaligned_offsets() -> Result<()> {
+        // The C layout of `FFI_ArrowArray`, filled in by hand as a C producer would.
+        #[repr(C)]
+        struct RawArrowArray {


we can just gate the test under not(force_validate) since CI should still run with & without force validate for arrow-array; it should make the test setup simpler that way, using arraydata instead of redefining the C data interface as a struct there

…igned ones

…nder force_validate

Rich-T-kid

makes sense to me

Jefffrey · 2026-11-08T02:36:49Z

thanks all

AndreaBozzo · 2026-11-08T08:41:51Z

Thanks everyone, have a nice end of the week!

fix(ffi): read byte array offsets unaligned in buffer_len

8444961

Closes apache#7136.

github-actions Bot added arrow-core Core crates related to representing arrow data enhancement New feature/addition labels Oct 31, 2026

test(ffi): back the unaligned offsets with an aligned MutableBuffer

c3015c0

AndreaBozzo marked this pull request as ready for review October 31, 2026 11:41

AndreaBozzo commented Oct 31, 2026

View reviewed changes

Rich-T-kid self-requested a review November 1, 2026 22:53

Jefffrey reviewed Nov 4, 2026

View reviewed changes

fix(ffi): read variadic buffer lengths of view arrays unaligned

36b4fa7

Jefffrey reviewed Nov 5, 2026

View reviewed changes

AndreaBozzo added 2 commits November 5, 2026 15:52

refactor(ffi): borrow aligned variadic buffer lengths, copy only unal…

392cddf

…igned ones

test(ffi): build the unaligned offsets test from ArrayData, skipped u…

b4600c5

…nder force_validate

Jefffrey added bug and removed enhancement New feature/addition labels Nov 5, 2026

Jefffrey approved these changes Nov 5, 2026

View reviewed changes

Jefffrey mentioned this pull request Nov 5, 2026

Array data imported through the FFI might contain unaligned buffers #7136

Closed

Rich-T-kid approved these changes Nov 7, 2026

View reviewed changes

Jefffrey added this pull request to the merge queue Nov 8, 2026

Merged via the queue into apache:main with commit 3a2c356 Nov 8, 2026
24 of 26 checks passed

AndreaBozzo deleted the fix/ffi-unaligned-offsets branch November 8, 2026 08:41

Conversation

AndreaBozzo commented Oct 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any significant user-facing changes?

Uh oh!

AndreaBozzo commented Oct 31, 2026

Uh oh!

AndreaBozzo Oct 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Jefffrey left a comment

Choose a reason for hiding this comment

Uh oh!

Jefffrey Nov 4, 2026

Choose a reason for hiding this comment

Uh oh!

AndreaBozzo Nov 4, 2026

Choose a reason for hiding this comment

Uh oh!

Jefffrey Nov 5, 2026

Choose a reason for hiding this comment

Uh oh!

AndreaBozzo Nov 5, 2026

Choose a reason for hiding this comment

Uh oh!

AndreaBozzo commented Nov 4, 2026

Uh oh!

felipecrv commented Nov 4, 2026

Uh oh!

AndreaBozzo commented Nov 4, 2026

Uh oh!

felipecrv commented Nov 4, 2026

Uh oh!

alamb commented Nov 4, 2026

Uh oh!

AndreaBozzo commented Nov 4, 2026

Uh oh!

Jefffrey commented Nov 5, 2026

Uh oh!

Jefffrey Nov 5, 2026

Choose a reason for hiding this comment

Uh oh!

AndreaBozzo Nov 5, 2026

Choose a reason for hiding this comment

Uh oh!

Jefffrey Nov 5, 2026

Choose a reason for hiding this comment

Uh oh!

Rich-T-kid left a comment

Choose a reason for hiding this comment

Uh oh!

Jefffrey commented Nov 8, 2026

Uh oh!

Uh oh!

AndreaBozzo commented Nov 8, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

AndreaBozzo commented Oct 31, 2026 •

edited

Loading

AndreaBozzo Oct 31, 2026 •

edited

Loading