From 1af11cc64da76c414c2a28da93486dd0b4adba98 Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Mon, 13 Jul 2026 22:05:13 +0200 Subject: [PATCH] fix(inject/host/windows): order the pad change-detect fields with Release/Acquire (G21) The XUSB `packet` publish and the XUSB `rumble_seq` / DualSense `out_seq` reads used plain unaligned accesses with no fence, so a driver could observe a bumped change-detect field over a torn body on a weakly-ordered core (ARM64). Publish `packet` via a Release AtomicU32 store behind a Release fence, and Acquire-load the seq fields, mirroring the gamepad_raii PadChannel seq-fence precedent. The DualSense input report embeds its seq mid-report with no driver-gated change-detect field, so it gets a Release fence after the copy and a documented residual (a per-frame input generation is deferred). No-op on x86-TSO. Verified: Windows .173 `cargo clippy -p punktfunk-host --all-targets -- -D warnings` (green). Co-Authored-By: Claude Opus 4.8 --- .../src/inject/windows/dualsense_windows.rs | 25 ++++++++++++++++--- .../src/inject/windows/gamepad_windows.rs | 21 +++++++++++++--- 2 files changed, 38 insertions(+), 8 deletions(-) diff --git a/crates/punktfunk-host/src/inject/windows/dualsense_windows.rs b/crates/punktfunk-host/src/inject/windows/dualsense_windows.rs index 47e356ed..29873c30 100644 --- a/crates/punktfunk-host/src/inject/windows/dualsense_windows.rs +++ b/crates/punktfunk-host/src/inject/windows/dualsense_windows.rs @@ -27,6 +27,7 @@ use crate::inject::pad_gate::PadGate; use anyhow::{anyhow, Result}; use punktfunk_core::quic::{HidOutput, RichInput}; use std::ffi::c_void; +use std::sync::atomic::{fence, AtomicU32, Ordering}; use std::time::{Duration, Instant}; use windows::core::{w, GUID, PCWSTR}; use windows::Win32::Devices::Enumeration::Pnp::{ @@ -291,13 +292,24 @@ impl DsWinPad { self.ts = self.ts.wrapping_add(1); let mut r = [0u8; DS_INPUT_REPORT_LEN]; serialize_state(&mut r, st, self.seq, self.ts); - // SAFETY: base points at SHM_SIZE bytes; input slot is OFF_INPUT..OFF_INPUT+64. + // SAFETY: base points at SHM_SIZE bytes; input slot is OFF_INPUT..OFF_INPUT+64. Unlike the + // XUSB `packet` / DualSense `out_seq` fields, the input path has NO driver-polled change-detect + // field to publish last: the `pf_dualsense` driver streams the whole `input` region to game + // READ_REPORTs on its ~125 Hz timer, and the report's own sequence counter (r[7], mid-report) + // is consumed by the game's HID stack, not the driver — so it cannot serve as a separable + // publish flag without a seqlock generation the driver `Acquire`-reads (a `PadShm` layout + + // driver change, deferred). The `Release` fence after the copy orders the report-body stores + // ahead of this pad's next `Release` publish (the bootstrap/seq stores in `channel.pump()`), + // giving the copy Release visibility on a weakly-ordered core (ARM64); on x86-TSO it is a + // no-op. Residual: absent a driver-side `Acquire` on a per-frame input generation, a torn + // single frame is still theoretically possible but self-heals on the next ~250 Hz write. unsafe { std::ptr::copy_nonoverlapping( r.as_ptr(), self.channel.data_base().add(OFF_INPUT), r.len(), - ) + ); + fence(Ordering::Release); }; } @@ -313,9 +325,14 @@ impl DsWinPad { std::ptr::read_unaligned(self.channel.data_base().add(OFF_DRIVER_PROTO) as *const u32) }; self.attach.observe(proto); - // SAFETY: base points at SHM_SIZE bytes. + // SAFETY: base points at SHM_SIZE bytes; `OFF_OUT_SEQ` (== 72) is 4-aligned off the + // page-aligned base, so the `AtomicU32` view is valid. The driver bumps `out_seq` AFTER + // writing the `output` report, so an `Acquire` load here orders the `output` copy below after + // it — a fresh seq guarantees a coherent snapshot of the output bytes on a weakly-ordered core + // (ARM64). On x86-TSO it is a plain load. let seq = unsafe { - std::ptr::read_unaligned(self.channel.data_base().add(OFF_OUT_SEQ) as *const u32) + (*(self.channel.data_base().add(OFF_OUT_SEQ) as *const AtomicU32)) + .load(Ordering::Acquire) }; if seq != self.last_out_seq { self.last_out_seq = seq; diff --git a/crates/punktfunk-host/src/inject/windows/gamepad_windows.rs b/crates/punktfunk-host/src/inject/windows/gamepad_windows.rs index 5537676a..68e929b8 100644 --- a/crates/punktfunk-host/src/inject/windows/gamepad_windows.rs +++ b/crates/punktfunk-host/src/inject/windows/gamepad_windows.rs @@ -17,6 +17,7 @@ use crate::gamestream::gamepad::{GamepadEvent, MAX_PADS}; use crate::inject::pad_gate::PadGate; use anyhow::{anyhow, Result}; use std::ffi::c_void; +use std::sync::atomic::{fence, AtomicU32, Ordering}; use std::time::{Duration, Instant}; use windows::core::{w, GUID, PCWSTR}; use windows::Win32::Devices::Enumeration::Pnp::{ @@ -193,7 +194,13 @@ impl XusbWinPad { let base = self.channel.data_base(); // SAFETY: `base` is the start of the mapped section (`SHM_SIZE` bytes, owned by `Shm`); every // `OFF_*` is a fixed in-range offset into it and `write_unaligned` handles the unaligned field - // writes. Single owner (`&mut self`), so no concurrent writer races these stores. + // writes. Single owner (`&mut self`), so no concurrent writer races these stores. `packet` (the + // field XInput reads to detect a new state) is published LAST: the `Release` fence orders the + // state-body stores above before the `Release` `AtomicU32` store of `packet`, so the driver — + // which `Acquire`-loads `packet` — never observes a bumped packet over a torn body on a + // weakly-ordered core (ARM64). On x86-TSO both are plain stores. `OFF_PACKET` (== 4) is + // 4-aligned off the page-aligned section base, so the `AtomicU32` view is valid (mirrors the + // seq-fenced publish in `gamepad_raii::PadChannel::create`). unsafe { std::ptr::write_unaligned(base.add(OFF_BUTTONS) as *mut u16, buttons); *base.add(OFF_LT) = lt; @@ -202,7 +209,8 @@ impl XusbWinPad { std::ptr::write_unaligned(base.add(OFF_LY) as *mut i16, ly); std::ptr::write_unaligned(base.add(OFF_RX) as *mut i16, rx); std::ptr::write_unaligned(base.add(OFF_RY) as *mut i16, ry); - std::ptr::write_unaligned(base.add(OFF_PACKET) as *mut u32, self.packet); + fence(Ordering::Release); + (*(base.add(OFF_PACKET) as *const AtomicU32)).store(self.packet, Ordering::Release); } } @@ -216,8 +224,13 @@ impl XusbWinPad { // SAFETY: base points at SHM_SIZE bytes. let proto = unsafe { std::ptr::read_unaligned(base.add(OFF_DRIVER_PROTO) as *const u32) }; self.attach.observe(proto); - // SAFETY: base points at SHM_SIZE bytes. - let seq = unsafe { std::ptr::read_unaligned(base.add(OFF_RUMBLE_SEQ) as *const u32) }; + // SAFETY: base points at SHM_SIZE bytes; `OFF_RUMBLE_SEQ` (== 24) is 4-aligned off the + // page-aligned base, so the `AtomicU32` view is valid. The driver bumps `rumble_seq` AFTER + // writing the rumble bytes, so an `Acquire` load here orders the `rumble_large`/`rumble_small` + // reads below after it — a fresh seq guarantees a coherent snapshot of the rumble bytes on a + // weakly-ordered core (ARM64). On x86-TSO it is a plain load. + let seq = + unsafe { (*(base.add(OFF_RUMBLE_SEQ) as *const AtomicU32)).load(Ordering::Acquire) }; if seq == self.last_rumble_seq { return None; }