From cd3f5474bfe9b5c3ea5b6dd4ccfbaf111b7ef93d Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Tue, 11 Aug 2026 13:57:34 +0200 Subject: [PATCH] fix(pf-driver-proto): a layout test read an align-8 struct out of an align-1 buffer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `control_structs_roundtrip_through_bytes` built the legacy-size wire form in a stack `let mut legacy = [0u8; 40]` (align 1) and then called `bytemuck::from_bytes::`. `AddRequest` opens with `session_id: u64`, so it is align 8, and `from_bytes` hands back a REFERENCE into the buffer — it panics unless the buffer happens to be 8-aligned. A stack `[u8; 40]` usually is, which is why this passed on every machine and every CI leg since it was written. Under Miri it fails outright: Miri does not let an accidentally-favourable stack slot stand in for a guarantee. Switched to `pod_read_unaligned`, which reads by value and has no alignment precondition. That is not a new idea here — `ChannelProof::parse` at lib.rs:1013 already carries a comment saying "`pod_read_unaligned`, NOT `from_bytes`" for exactly this reason. This site is the only other one in the crate that reads a POD out of a stack byte array; every other `from_bytes` call in the tests reads from `bytes_of(&x)`, which is aligned by construction. Test-only, so no shipped defect — but the crate is `#![forbid(unsafe_code)]` and is path-dep'd by BOTH the main workspace and the driver workspace, so it is the layout oracle for every frame and IOCTL that crosses that boundary. A test that cannot be trusted to fail is worth fixing there more than anywhere else. Found by the first Miri run ever performed against this repo. Verified on 192.168.1.25 (Ubuntu, cargo 1.96.0): cargo +nightly miri test -p pf-driver-proto 21/21 cargo +nightly miri test -p pf-driver-proto --target x86_64-pc-windows-msvc 21/21 cargo test -p pf-driver-proto --locked ok cargo clippy -p pf-driver-proto --all-targets --locked -- -D warnings clean The cross-target run is the interesting one: it interprets the crate at MSVC layout on a Linux box with no Windows anywhere. Nothing else in CI does that. --- crates/pf-driver-proto/src/lib.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/crates/pf-driver-proto/src/lib.rs b/crates/pf-driver-proto/src/lib.rs index 48d3813b..809bb2f8 100644 --- a/crates/pf-driver-proto/src/lib.rs +++ b/crates/pf-driver-proto/src/lib.rs @@ -1712,7 +1712,13 @@ mod tests { let mut legacy = [0u8; 40]; legacy[..control::ADD_REQUEST_LEGACY_SIZE] .copy_from_slice(&bytes[..control::ADD_REQUEST_LEGACY_SIZE]); - let old = *bytemuck::from_bytes::(&legacy); + // `pod_read_unaligned`, NOT `from_bytes` — same rule as `ChannelProof::parse` above, and + // for the same reason. `legacy` is a `[u8; 40]` (align 1) but `AddRequest` opens with a + // `u64`, so it is align 8; `from_bytes` takes a REFERENCE into the buffer and panics + // unless the buffer happens to be 8-aligned. A stack `[u8; 40]` usually is, which is why + // this passed everywhere for so long — Miri caught it because Miri does not let an + // accidentally-favourable stack slot stand in for a guarantee. + let old = bytemuck::pod_read_unaligned::(&legacy); assert_eq!(old.preferred_monitor_id, 7); assert_eq!( (