jonas-k/coremidi-sys

MIDIPacketNext Potential UB: pointer.offset() crossing object boundaries

Closed this issue ยท 2 comments

Context

The MIDIPacket struct from CoreMidi has a flexible array member (FAM) at the end. The field for this FAM is currently defined in Rust as pub data: [u8; 256] by bindgen.

The Problem

(Link to source)

This one is only potentially Undefined Behavior (UB) - if it is or not depends on the specific way Rust is implemented, so I think we'll likely have to get some expert guidance in here to determine that. From the pointer.offset() docs:

If any of the following conditions are violated, the result is Undefined Behavior:
...

  • Both the starting and resulting pointer must be either in bounds or one byte past the end of the same allocated object.

Since MIDIPacket.data is a FAM, my understanding is that there could potentially be a variable amount of additional space after the end of the 256-byte data array in MIDIPacket.data that the pointer will have to be offset by. This means that if MIDIPacket.length is > 256, the pointer that is calculated via pointer.offset() would be outside the size of the #[repr(C)] MIDIPacket that Rust knows about.

The real question here is if Rust/LLVM would consider that "past the end of the same allocated object" or not. If it is, then it is UB to even construct that pointer. If not, then we're all good! So it would be great to get a more precise understanding of this notion of an "allocated object".

Potential Fixes

If it turns out that this is considered "crossing the object boundary", it seems like we still have an option for calculating the pointer offset - as the pointer.wrapping_offset() docs mention:

If you need to cross object boundaries, cast the pointer to an integer and do the arithmetic there.

I would need to do additional research to be sure that this won't cause UB some other way.

Just posted a question about this ticket to Reddit and the Rust Users Forum. ๐Ÿ‘

There was a very helpful answer on the Rust Users Forum. The most relevant quote is:

For pointers created in C, consult the C specification (i.e. if it isn't UB to access that address in C with the pointer, it's also OK in Rust) .

So since Apple's implementation in MIDIServices.h offsets the pointer to point to the next MIDIPacket, it seems like we're in the clear here too (seeing as Rust doesn't impose additional requirements for pointers created in C). ๐ŸŽ‰