Ensure macOS pointer addresses are aligned to eight bytes #313
Conversation
| @@ -500,34 +501,38 @@ impl OsIpcSender { | |||
| (*message).body.msgh_descriptor_count = | |||
| (ports.len() + shared_memory_regions.len()) as u32; | |||
|
|
|||
| let mut port_descriptor_dest = message.offset(1) as *mut mach_msg_port_descriptor_t; | |||
| let mut port_descriptor_dest = (message as *mut PaddedMessage).offset(1) as *mut PaddedPortDescriptor; | |||
There was a problem hiding this comment.
@jdm , I was able to fix the test failures with these changes.
I'm not 100% sure about this, but this is my theory of what is causing the tests to fail: Because padding is added to the wrapped mach_msg_port_descriptor_t & mach_ool_descriptor_t structs, the entries in the array of descriptors following Message struct will have the wrong layout from the perspective of kernel. Since the kernel needs to interpret the values in these fields for validating the transfer of ports, it could end up reading random invalid data as the port names.
I couldn't find any documentation explaining the alignment requirements for OOL descriptor entries so I assumed they are byte-aligned (so address is now [u8; 8]). Relaxing the alignment requirement allows the debug assertions inserted by rust on the reads/writes to succeed. The tests platform::test::shared_memory and platform::test:shared_memory_clone used to pass with Rust 1.67 and now continue to pass with my changes, so I think the Kernel doesn't have the 8-byte alignment requirement (for the descriptors) either.
I can raise a PR if the changes seem fine to you.
There was a problem hiding this comment.
Yikes! I'm going to need to sit and think about this solution.
There was a problem hiding this comment.
Based on this I think the kernel only expects 4-byte alignment. It also seems rust-bindgen didn't translate pragma pack directives until 2018 and the mach_sys.rs was added in 2015.
Should we just regenerate the mach_sys.rs using newer rust-bindgen? If so, I don't know which header file is the official one.
There was a problem hiding this comment.
Oh, that makes a lot of sense! I'd say give rust-bindgen a try on the header that you linked and see what the output looks like?
There was a problem hiding this comment.
Actually I guess https://github.com/apple-oss-distributions/xnu/blob/main/osfmk/mach/message.h is the official distribution now.
|
☔ The latest upstream changes (presumably 47c8263) made this pull request unmergeable. Please resolve the merge conflicts. |
|
Closed via #314 |
This is still a draft because it's not working yet.