fix(inject): heap the SwDeviceCreate callback context; stop latching a pad slot on a failed create
Two medium findings from the round-1 sweep, each applied to both siblings. - create_swdevice stack-allocated the SwCreateCtx that the async PnP completion callback writes through (result + up to 127 u16 of instance id) and then SetEvents. The wait is bounded at 10s, so on a wedged-PnP timeout the callback can still be PENDING: the frame is popped, the input thread reuses that stack, and a late callback corrupts it and SetEvents an already-closed (possibly recycled) handle. The context is now heap-allocated and reclaimed only where the callback provably ran; on the timeout path the box is deliberately leaked and the event left open, so a late write always targets live memory. Costs a one-off ~264 B + one HANDLE on that rare path. Applied to the DualSense path and its XUSB sibling in gamepad_windows.rs. - Ds4WinPad::open swallowed a create_swdevice failure into a WARN and returned Ok with no devnode. PadSlots::ensure then stored Some(pad) AND called gate.on_success(), so the slot short-circuited on is_some() forever and the capped-backoff retry that exists precisely to self-heal a transient PnP failure never ran — the game saw no controller for the rest of the session unless the client unplugged the pad. Now propagates, matching the XUSB sibling. Same fix applied to steam_deck_windows.rs. Windows .173: pf-inject 53/0. Linux .21: pf-inject 74/0 (8 ignored). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -299,13 +299,20 @@ pub(super) fn create_swdevice(p: &SwDeviceProfile) -> Result<(HSWDEVICE, Option<
|
|||||||
let event = unsafe { CreateEventW(None, true, false, PCWSTR::null())? };
|
let event = unsafe { CreateEventW(None, true, false, PCWSTR::null())? };
|
||||||
// `result` starts as E_FAIL, NOT S_OK: if the wait below times out, a zero-initialised HRESULT
|
// `result` starts as E_FAIL, NOT S_OK: if the wait below times out, a zero-initialised HRESULT
|
||||||
// would read as success and mask the failure (found by the 2026-07 driver-health audit).
|
// would read as success and mask the failure (found by the 2026-07 driver-health audit).
|
||||||
let mut ctx = SwCreateCtx {
|
// HEAP-allocated, deliberately: `sw_create_cb` writes `result` + up to 127 u16 of instance id
|
||||||
|
// through this pointer and then `SetEvent`s. The wait below is bounded (10 s), so on a wedged-PnP
|
||||||
|
// timeout the callback may still be PENDING — a stack context would be popped and a late callback
|
||||||
|
// would corrupt whatever the input thread put there next, and SetEvent a closed/recycled handle.
|
||||||
|
// On the timeout path we therefore LEAK the box and leave the event open (a one-off ~264 B + one
|
||||||
|
// HANDLE, only on that rare path) so a late callback always writes to live memory.
|
||||||
|
let ctx = Box::into_raw(Box::new(SwCreateCtx {
|
||||||
event,
|
event,
|
||||||
result: E_FAIL,
|
result: E_FAIL,
|
||||||
instance_id: [0; 128],
|
instance_id: [0; 128],
|
||||||
};
|
}));
|
||||||
// SAFETY: info + the buffers + ctx outlive the call (we wait on the event before returning);
|
// SAFETY: info + the buffers outlive the call; `ctx` is a live heap allocation that outlives every
|
||||||
// windows-rs returns the HSWDEVICE (the C out-param) as the Result value.
|
// path below (reclaimed only where the callback provably ran). windows-rs returns the HSWDEVICE
|
||||||
|
// (the C out-param) as the Result value.
|
||||||
let hsw = match unsafe {
|
let hsw = match unsafe {
|
||||||
SwDeviceCreate(
|
SwDeviceCreate(
|
||||||
w!("punktfunk"),
|
w!("punktfunk"),
|
||||||
@@ -313,13 +320,15 @@ pub(super) fn create_swdevice(p: &SwDeviceProfile) -> Result<(HSWDEVICE, Option<
|
|||||||
&info,
|
&info,
|
||||||
None,
|
None,
|
||||||
Some(sw_create_cb),
|
Some(sw_create_cb),
|
||||||
Some(&mut ctx as *mut SwCreateCtx as *const c_void),
|
Some(ctx as *const c_void),
|
||||||
)
|
)
|
||||||
} {
|
} {
|
||||||
Ok(h) => h,
|
Ok(h) => h,
|
||||||
Err(e) => {
|
Err(e) => {
|
||||||
// SAFETY: event is valid.
|
// SAFETY: the call failed, so no callback was registered and `ctx` is ours to reclaim;
|
||||||
|
// `event` is valid and unreferenced.
|
||||||
unsafe {
|
unsafe {
|
||||||
|
drop(Box::from_raw(ctx));
|
||||||
let _ = CloseHandle(event);
|
let _ = CloseHandle(event);
|
||||||
}
|
}
|
||||||
return Err(anyhow!("SwDeviceCreate failed: {e}"));
|
return Err(anyhow!("SwDeviceCreate failed: {e}"));
|
||||||
@@ -328,17 +337,22 @@ pub(super) fn create_swdevice(p: &SwDeviceProfile) -> Result<(HSWDEVICE, Option<
|
|||||||
// Block until PnP finishes enumerating (the callback signals), then check its result.
|
// Block until PnP finishes enumerating (the callback signals), then check its result.
|
||||||
// SAFETY: event is valid.
|
// SAFETY: event is valid.
|
||||||
let wait = unsafe { WaitForSingleObject(event, 10_000) };
|
let wait = unsafe { WaitForSingleObject(event, 10_000) };
|
||||||
// SAFETY: event is valid.
|
|
||||||
unsafe {
|
|
||||||
let _ = CloseHandle(event);
|
|
||||||
}
|
|
||||||
if wait != WAIT_OBJECT_0 {
|
if wait != WAIT_OBJECT_0 {
|
||||||
|
// Timed out: the callback may still fire. Intentionally leak `ctx` AND leave `event` open so
|
||||||
|
// its eventual write + SetEvent target live memory/handle rather than freed ones.
|
||||||
// SAFETY: hsw is the handle SwDeviceCreate returned.
|
// SAFETY: hsw is the handle SwDeviceCreate returned.
|
||||||
unsafe { SwDeviceClose(hsw) };
|
unsafe { SwDeviceClose(hsw) };
|
||||||
return Err(anyhow!(
|
return Err(anyhow!(
|
||||||
"SwDeviceCreate enumeration callback never fired (10s) — PnP may be wedged"
|
"SwDeviceCreate enumeration callback never fired (10s) — PnP may be wedged"
|
||||||
));
|
));
|
||||||
}
|
}
|
||||||
|
// The callback ran (it is what signalled the event), so nothing else will touch `ctx`/`event`.
|
||||||
|
// SAFETY: `ctx` came from `Box::into_raw` above and is reclaimed exactly once here; `event` is
|
||||||
|
// valid and no longer referenced by a pending callback.
|
||||||
|
let ctx = unsafe {
|
||||||
|
let _ = CloseHandle(event);
|
||||||
|
Box::from_raw(ctx)
|
||||||
|
};
|
||||||
if ctx.result.is_err() {
|
if ctx.result.is_err() {
|
||||||
// SAFETY: hsw is the handle SwDeviceCreate returned.
|
// SAFETY: hsw is the handle SwDeviceCreate returned.
|
||||||
unsafe { SwDeviceClose(hsw) };
|
unsafe { SwDeviceClose(hsw) };
|
||||||
|
|||||||
@@ -62,7 +62,7 @@ impl Ds4WinPad {
|
|||||||
std::ptr::write_unaligned(base as *mut u32, SHM_MAGIC);
|
std::ptr::write_unaligned(base as *mut u32, SHM_MAGIC);
|
||||||
}
|
}
|
||||||
let inst = format!("pf_ds4_{index}");
|
let inst = format!("pf_ds4_{index}");
|
||||||
let (hsw, instance_id) = match create_swdevice(&SwDeviceProfile {
|
let (hsw, instance_id) = create_swdevice(&SwDeviceProfile {
|
||||||
instance: &inst,
|
instance: &inst,
|
||||||
container_tag: 0x5046_4453, // "PFDS"
|
container_tag: 0x5046_4453, // "PFDS"
|
||||||
container_index: index,
|
container_index: index,
|
||||||
@@ -70,13 +70,13 @@ impl Ds4WinPad {
|
|||||||
usb_vid_pid: "VID_054C&PID_09CC",
|
usb_vid_pid: "VID_054C&PID_09CC",
|
||||||
usb_mi: None,
|
usb_mi: None,
|
||||||
description: "punktfunk Virtual DualShock 4",
|
description: "punktfunk Virtual DualShock 4",
|
||||||
}) {
|
})?; // Propagate, do NOT swallow — see below.
|
||||||
Ok((h, id)) => (Some(h), id),
|
let (hsw, instance_id) = (Some(hsw), instance_id);
|
||||||
Err(e) => {
|
// Swallowing a create failure here (the previous behaviour) latched the pad slot to
|
||||||
tracing::warn!(error = %format!("{e:#}"), "SwDeviceCreate failed; DualShock 4 devnode unavailable");
|
// `Some(pad)` with no live devnode: `PadSlots::ensure` short-circuits on `is_some()` and
|
||||||
(None, None)
|
// `gate.on_success()` cleared the backoff, so the create-gate that exists precisely to
|
||||||
}
|
// self-heal a transient PnP failure never retried. The game saw no controller for the whole
|
||||||
};
|
// session unless the client unplugged the pad. Matches the XUSB sibling, which propagates.
|
||||||
let _sw = hsw.map(super::gamepad_raii::SwDevice::new);
|
let _sw = hsw.map(super::gamepad_raii::SwDevice::new);
|
||||||
// Bounded eager delivery — for the DS4 this is what closes the identity race: the driver
|
// Bounded eager delivery — for the DS4 this is what closes the identity race: the driver
|
||||||
// must read `device_type = 1` from the delivered DATA section before hidclass asks it for
|
// must read `device_type = 1` from the delivered DATA section before hidclass asks it for
|
||||||
|
|||||||
@@ -82,12 +82,16 @@ fn create_swdevice(index: u8) -> Result<(HSWDEVICE, Option<String>)> {
|
|||||||
let event = unsafe { CreateEventW(None, true, false, PCWSTR::null())? };
|
let event = unsafe { CreateEventW(None, true, false, PCWSTR::null())? };
|
||||||
// `result` starts as E_FAIL, NOT S_OK: if the wait below times out, a zero-initialised HRESULT
|
// `result` starts as E_FAIL, NOT S_OK: if the wait below times out, a zero-initialised HRESULT
|
||||||
// would read as success and mask the failure (found by the 2026-07 driver-health audit).
|
// would read as success and mask the failure (found by the 2026-07 driver-health audit).
|
||||||
let mut ctx = SwCreateCtx {
|
// HEAP-allocated for the same reason as the DualSense sibling: the callback writes through this
|
||||||
|
// pointer and SetEvents, and the wait below is bounded — a stack context would be popped while a
|
||||||
|
// late callback still holds it. On the timeout path the box is deliberately leaked and the event
|
||||||
|
// left open so a late write/SetEvent always targets live memory/handle.
|
||||||
|
let ctx = Box::into_raw(Box::new(SwCreateCtx {
|
||||||
event,
|
event,
|
||||||
result: E_FAIL,
|
result: E_FAIL,
|
||||||
instance_id: [0; 128],
|
instance_id: [0; 128],
|
||||||
};
|
}));
|
||||||
// SAFETY: info + buffers + ctx outlive the call (we wait on the event before returning).
|
// SAFETY: info + buffers outlive the call; `ctx` is a live heap allocation outliving every path.
|
||||||
let hsw = match unsafe {
|
let hsw = match unsafe {
|
||||||
SwDeviceCreate(
|
SwDeviceCreate(
|
||||||
w!("punktfunk"),
|
w!("punktfunk"),
|
||||||
@@ -95,13 +99,14 @@ fn create_swdevice(index: u8) -> Result<(HSWDEVICE, Option<String>)> {
|
|||||||
&info,
|
&info,
|
||||||
None,
|
None,
|
||||||
Some(sw_create_cb),
|
Some(sw_create_cb),
|
||||||
Some(&mut ctx as *mut SwCreateCtx as *const c_void),
|
Some(ctx as *const c_void),
|
||||||
)
|
)
|
||||||
} {
|
} {
|
||||||
Ok(h) => h,
|
Ok(h) => h,
|
||||||
Err(e) => {
|
Err(e) => {
|
||||||
// SAFETY: event is valid.
|
// SAFETY: the call failed, so no callback is pending and `ctx` is ours to reclaim.
|
||||||
unsafe {
|
unsafe {
|
||||||
|
drop(Box::from_raw(ctx));
|
||||||
let _ = CloseHandle(event);
|
let _ = CloseHandle(event);
|
||||||
}
|
}
|
||||||
return Err(anyhow!("SwDeviceCreate(pf_xusb) failed: {e}"));
|
return Err(anyhow!("SwDeviceCreate(pf_xusb) failed: {e}"));
|
||||||
@@ -109,17 +114,20 @@ fn create_swdevice(index: u8) -> Result<(HSWDEVICE, Option<String>)> {
|
|||||||
};
|
};
|
||||||
// SAFETY: event valid; block until PnP finishes enumerating, then check the callback result.
|
// SAFETY: event valid; block until PnP finishes enumerating, then check the callback result.
|
||||||
let wait = unsafe { WaitForSingleObject(event, 10_000) };
|
let wait = unsafe { WaitForSingleObject(event, 10_000) };
|
||||||
// SAFETY: event is valid.
|
|
||||||
unsafe {
|
|
||||||
let _ = CloseHandle(event);
|
|
||||||
}
|
|
||||||
if wait != WAIT_OBJECT_0 {
|
if wait != WAIT_OBJECT_0 {
|
||||||
|
// Timed out — intentionally leak `ctx` and leave `event` open (see above).
|
||||||
// SAFETY: hsw is the handle SwDeviceCreate returned.
|
// SAFETY: hsw is the handle SwDeviceCreate returned.
|
||||||
unsafe { SwDeviceClose(hsw) };
|
unsafe { SwDeviceClose(hsw) };
|
||||||
return Err(anyhow!(
|
return Err(anyhow!(
|
||||||
"SwDeviceCreate(pf_xusb) enumeration callback never fired (10s) — PnP may be wedged"
|
"SwDeviceCreate(pf_xusb) enumeration callback never fired (10s) — PnP may be wedged"
|
||||||
));
|
));
|
||||||
}
|
}
|
||||||
|
// The callback ran (it signalled the event), so nothing else will touch `ctx`/`event`.
|
||||||
|
// SAFETY: `ctx` came from `Box::into_raw` and is reclaimed exactly once here.
|
||||||
|
let ctx = unsafe {
|
||||||
|
let _ = CloseHandle(event);
|
||||||
|
Box::from_raw(ctx)
|
||||||
|
};
|
||||||
if ctx.result.is_err() {
|
if ctx.result.is_err() {
|
||||||
// SAFETY: hsw is the handle SwDeviceCreate returned.
|
// SAFETY: hsw is the handle SwDeviceCreate returned.
|
||||||
unsafe { SwDeviceClose(hsw) };
|
unsafe { SwDeviceClose(hsw) };
|
||||||
|
|||||||
@@ -66,7 +66,7 @@ impl DeckWinPad {
|
|||||||
std::ptr::write_unaligned(base as *mut u32, SHM_MAGIC);
|
std::ptr::write_unaligned(base as *mut u32, SHM_MAGIC);
|
||||||
}
|
}
|
||||||
let inst = format!("pf_deck_{index}");
|
let inst = format!("pf_deck_{index}");
|
||||||
let (hsw, instance_id) = match create_swdevice(&SwDeviceProfile {
|
let (hsw, instance_id) = create_swdevice(&SwDeviceProfile {
|
||||||
instance: &inst,
|
instance: &inst,
|
||||||
container_tag: 0x5046_4453, // "PFDS"
|
container_tag: 0x5046_4453, // "PFDS"
|
||||||
container_index: index,
|
container_index: index,
|
||||||
@@ -77,13 +77,8 @@ impl DeckWinPad {
|
|||||||
// spike's run-1 failure).
|
// spike's run-1 failure).
|
||||||
usb_mi: Some(2),
|
usb_mi: Some(2),
|
||||||
description: "punktfunk Virtual Steam Deck",
|
description: "punktfunk Virtual Steam Deck",
|
||||||
}) {
|
})?; // Propagate — swallowing latched the slot to a pad with no devnode (see the DS4 twin).
|
||||||
Ok((h, i)) => (Some(h), i),
|
let (hsw, instance_id) = (Some(hsw), instance_id);
|
||||||
Err(e) => {
|
|
||||||
tracing::warn!(error = %format!("{e:#}"), "SwDeviceCreate failed; Steam Deck devnode unavailable");
|
|
||||||
(None, None)
|
|
||||||
}
|
|
||||||
};
|
|
||||||
let _sw = hsw.map(super::gamepad_raii::SwDevice::new);
|
let _sw = hsw.map(super::gamepad_raii::SwDevice::new);
|
||||||
// Bounded eager delivery — the driver must read `device_type = 3` before hidclass asks
|
// Bounded eager delivery — the driver must read `device_type = 3` before hidclass asks
|
||||||
// it for descriptors, or the pad would enumerate with the default DualSense identity.
|
// it for descriptors, or the pad would enumerate with the default DualSense identity.
|
||||||
|
|||||||
Reference in New Issue
Block a user