From bd19e40f70fa161b6d1ee7a7cfbff8ee0490a4db Mon Sep 17 00:00:00 2001 From: potpiemuncher Date: Fri, 31 Jul 2026 23:45:23 -0500 Subject: [PATCH] Never ingest a pad attached through usbip-win2 as input Input discovery's protection against re-ingesting our own virtual output was a memory: an in-process path registry and an active-port check, both records of what this session created. A session that dies hard takes the records with it, so its leftover pad was a perfectly ordinary DualSense to the next start -- ingested, mapped to an output, and recursed on. The old startup port sweep prevented that by detaching the leftover, and stopped doing so when it turned out to be detaching other applications' live pads; this closes the residual that removal left, at the layer it always belonged. The rule is positional: any pad whose devnode ancestry reaches the usbip-win2 UDE controller is a virtual output being served by something -- this session's own, a dead session's leftover, or another program's live controller -- and none of those is input. Ancestry does not depend on who remembers creating the device. The walk already existed (Global.CheckIfUsbIpWin2Device, built for the active-port check); the change is consulting it for every candidate rather than only for ports this session registered. Our own live outputs are rejected quietly, as before. An unmanaged usbip pad is rejected with one line per path, because nothing else will ever explain to the user why a pad they can see is being ignored: the line names both readings and points at the backend-process card that can clear a leftover with consent. Deliberately given up: a real controller forwarded from another machine over usbip-win2 can no longer be input. On these machines usbip-win2 exists solely as VIIPER's transport, remote forwarding has VirtualHere as the supported route, and the refusal is named in the log rather than silent. Five policy tests, and the live coexistence scenario as the proof: with the native-mode DS4Windows serving its virtual DualSense and DualSense input enabled, one discovery pass refused the usbip-attached pad and accepted the physical Bluetooth DualSense -- same VID and PID -- in the same breath. Position in the device tree is the discriminator, not controller identity. Co-Authored-By: Claude Fable 5 --- DS4Windows/DS4Control/ControlService.cs | 16 ++- DS4Windows/DS4Library/DS4Devices.cs | 27 +++- .../DS4Library/UsbipAttachedInputPolicy.cs | 125 ++++++++++++++++++ .../UsbipAttachedInputPolicyTests.cs | 92 +++++++++++++ docs/dev/PLAN-PROGRESS.md | 51 +++++++ 5 files changed, 302 insertions(+), 9 deletions(-) create mode 100644 DS4Windows/DS4Library/UsbipAttachedInputPolicy.cs create mode 100644 DS4WindowsTests/UsbipAttachedInputPolicyTests.cs diff --git a/DS4Windows/DS4Control/ControlService.cs b/DS4Windows/DS4Control/ControlService.cs index 5281e74..e89ee94 100644 --- a/DS4Windows/DS4Control/ControlService.cs +++ b/DS4Windows/DS4Control/ControlService.cs @@ -1747,13 +1747,15 @@ public bool Start(bool showlog = true) // VID/PID and a localhost URL, another application's live // virtual pad is indistinguishable from a leftover, and on // 2026-07-31 the sweep disconnected one mid-game. The pass - // still runs for its log line — any unmanaged local import - // is named, with a pointer to the Settings backend-process - // card, which can attribute leftovers and clear them with - // consent. The self-ingestion case that motivated the - // detach is accepted as a residual risk until inputs can - // recognise this app's own virtual pads; the startup - // warning above fires for exactly that state. + // runs purely for its log line — any unmanaged local + // import is named, with a pointer to the Settings + // backend-process card, which can attribute leftovers and + // clear them with consent. The self-ingestion case that + // motivated the old detach is closed at the correct layer + // instead: input discovery refuses any pad attached + // through the usbip-win2 controller + // (UsbipAttachedInputPolicy), which needs no memory of who + // created it. ViiperUsbipPortManager.ObserveLocalImports(); StartupDiag("DS4Devices.findControllers dispatch begin"); diff --git a/DS4Windows/DS4Library/DS4Devices.cs b/DS4Windows/DS4Library/DS4Devices.cs index 6545565..d368fa5 100644 --- a/DS4Windows/DS4Library/DS4Devices.cs +++ b/DS4Windows/DS4Library/DS4Devices.cs @@ -399,9 +399,32 @@ private static bool IsRealDS4(HidDevice hDevice) { // Our own virtual output controllers are never valid input, regardless of // mode. This is what breaks the Moonlight + DS4-output feedback loop. - if (IsOwnVirtualDevice(hDevice.DevicePath)) + // + // The second question widens that to position: ANY pad attached + // through usbip-win2's controller is a virtual output being served + // by something — a leftover of a session that died hard, or + // another application's live pad. The in-memory registry cannot + // recognise either (it died with its session, or never knew them), + // which is how a crashed session's own output used to come back as + // an input. Ancestry does not depend on who remembers creating the + // device. + string devicePath = hDevice.DevicePath; + bool ownLiveOutput = IsOwnVirtualDevice(devicePath); + bool usbipAttached = ownLiveOutput || + Global.CheckIfUsbIpWin2Device(devicePath); + switch (UsbipAttachedInputPolicy.Decide(ownLiveOutput, usbipAttached)) { - return false; + case UsbipInputVerdict.RejectOwnLiveOutput: + return false; + case UsbipInputVerdict.RejectUnmanagedImport: + if (UsbipAttachedInputPolicy.ShouldWarnOnce(devicePath)) + { + AppLogger.LogToGui( + UsbipAttachedInputPolicy.DescribeRejectedImport( + devicePath), true); + } + + return false; } bool isVirtualDevice = Global.CheckIfVirtualDevice(hDevice.DevicePath); diff --git a/DS4Windows/DS4Library/UsbipAttachedInputPolicy.cs b/DS4Windows/DS4Library/UsbipAttachedInputPolicy.cs new file mode 100644 index 0000000..c214c8b --- /dev/null +++ b/DS4Windows/DS4Library/UsbipAttachedInputPolicy.cs @@ -0,0 +1,125 @@ +/* +DS4Windows +Copyright (C) 2026 DS4Windows contributors + +This program is free software: you can redistribute it and/or modify +it under the terms of the GNU General Public License as published by +the Free Software Foundation, either version 3 of the License, or +(at your option) any later version. +*/ + +using System; +using System.Collections.Generic; + +namespace DS4Windows +{ + /// What input discovery should do with a candidate pad. + internal enum UsbipInputVerdict + { + /// Not usbip-attached; the normal admission rules apply. + Accept, + + /// + /// One of this session's own live VIIPER outputs. Rejected quietly — + /// its creation already logged, and it leaves with its lifetime. + /// + RejectOwnLiveOutput, + + /// + /// Attached through usbip-win2 but not this session's. Rejected with + /// one log line, because unlike our own output nothing else will ever + /// explain to the user why this pad is being ignored. + /// + RejectUnmanagedImport, + } + + /// + /// Admission policy for pads attached through usbip-win2's emulated host + /// controller: they are never input. + /// + /// Why position decides, not ownership. A pad under that + /// controller is a virtual device being served to something — this + /// session's own output, a leftover of a session that died hard, or + /// another application's live controller. Ingesting the first recurses + /// (our output becomes our input, which maps to another output); the + /// in-memory path registry has always rejected it. The other two are the + /// gap this policy closes: the registry dies with its session, so a + /// leftover was re-ingested on the next start — the recursion the old + /// startup port sweep existed to prevent, and the reason its removal + /// (2026-07-31, after it disconnected another application's live pad) + /// left this as the accepted residual. Recognising the pads intrinsically + /// retires that residual without touching anything: a devnode's ancestry + /// does not depend on who remembers creating it. + /// + /// What this deliberately gives up: a real controller + /// forwarded from another machine over usbip-win2 can no longer be used + /// as input. On this project's machines usbip-win2 exists solely as + /// VIIPER's transport, remote forwarding has VirtualHere as the supported + /// route (see CheckIfVirtualDevice's exclusion list), and the log + /// line names the refusal — if the combination ever matters, it arrives + /// as a feature request with its evidence attached, not as silence. + /// + /// Same shape as : the + /// verdict is a pure function of the two probes' answers, so the rule is + /// testable without a device tree. + /// + internal static class UsbipAttachedInputPolicy + { + private static readonly object warnedLock = new object(); + private static readonly HashSet warnedPaths = + new HashSet(StringComparer.OrdinalIgnoreCase); + + internal static UsbipInputVerdict Decide(bool isOwnLiveOutput, + bool isUsbipAttached) + { + if (isOwnLiveOutput) + { + return UsbipInputVerdict.RejectOwnLiveOutput; + } + + return isUsbipAttached + ? UsbipInputVerdict.RejectUnmanagedImport + : UsbipInputVerdict.Accept; + } + + /// + /// Whether this rejection is the first for the path. Discovery re-runs + /// on every hotplug, and a pad that is being deliberately ignored + /// would otherwise be re-announced each time anything else arrives. + /// + internal static bool ShouldWarnOnce(string devicePath) + { + if (string.IsNullOrEmpty(devicePath)) + { + return false; + } + + lock (warnedLock) + { + return warnedPaths.Add(devicePath); + } + } + + /// + /// The one line the user gets for a pad that exists but is ignored. + /// It has to say what was seen, why it is not input, both readings of + /// what it might be, and where to act if it is a leftover. + /// + internal static string DescribeRejectedImport(string devicePath) => + "Ignoring a controller attached through usbip-win2 (" + + devicePath + "): pads on that controller are virtual outputs " + + "being served by some application - possibly " + + ProductInfo.ProductName + "'s own from a session that ended " + + "abruptly, possibly another program's live controller - and are " + + "never used as input. If it is a leftover, Settings > VIIPER " + + "Virtual Controller Support > Backend process can clear it."; + + internal static void ResetForTests() + { + lock (warnedLock) + { + warnedPaths.Clear(); + } + } + } +} diff --git a/DS4WindowsTests/UsbipAttachedInputPolicyTests.cs b/DS4WindowsTests/UsbipAttachedInputPolicyTests.cs new file mode 100644 index 0000000..bfae132 --- /dev/null +++ b/DS4WindowsTests/UsbipAttachedInputPolicyTests.cs @@ -0,0 +1,92 @@ +using DS4Windows; + +namespace DS4WindowsTests; + +/// +/// Pads attached through usbip-win2's controller are never input. This closes +/// the residual PR #30 left open: the in-memory own-output registry dies with +/// its session, so a hard-killed session's leftover pad used to come back as +/// an input on the next start — the recursion the old startup port sweep +/// existed to prevent, and could no longer prevent once it stopped detaching +/// what it could not attribute. Ancestry does not depend on who remembers +/// creating the device, so the rule covers our leftovers and other +/// applications' virtual pads alike. +/// +[TestClass] +public class UsbipAttachedInputPolicyTests +{ + [TestInitialize] + public void ResetWarnState() + { + UsbipAttachedInputPolicy.ResetForTests(); + } + + [TestMethod] + public void APhysicalPadIsAccepted() + { + Assert.AreEqual(UsbipInputVerdict.Accept, + UsbipAttachedInputPolicy.Decide(isOwnLiveOutput: false, + isUsbipAttached: false)); + } + + [TestMethod] + public void OurOwnLiveOutputIsRejectedQuietly() + { + Assert.AreEqual(UsbipInputVerdict.RejectOwnLiveOutput, + UsbipAttachedInputPolicy.Decide(isOwnLiveOutput: true, + isUsbipAttached: true)); + Assert.AreEqual(UsbipInputVerdict.RejectOwnLiveOutput, + UsbipAttachedInputPolicy.Decide(isOwnLiveOutput: true, + isUsbipAttached: false), + "Ownership wins over whatever the ancestry probe said; a device " + + "the registry claims is ours is ours to reject as ours."); + } + + /// + /// The case that used to recurse: usbip-attached, but nothing in this + /// session remembers creating it — a dead session's leftover or another + /// application's live pad, indistinguishable and equally not input. + /// + [TestMethod] + public void AnUnmanagedUsbipPadIsRejected() + { + Assert.AreEqual(UsbipInputVerdict.RejectUnmanagedImport, + UsbipAttachedInputPolicy.Decide(isOwnLiveOutput: false, + isUsbipAttached: true)); + } + + [TestMethod] + public void EachIgnoredPadIsAnnouncedExactlyOnce() + { + const string pad = @"\\?\hid#vid_054c&pid_0ce6#7&2ab44e7&0&0000#{guid}"; + const string otherPad = @"\\?\hid#vid_054c&pid_0df2#8&11112222&0&0000#{guid}"; + + Assert.IsTrue(UsbipAttachedInputPolicy.ShouldWarnOnce(pad)); + Assert.IsFalse(UsbipAttachedInputPolicy.ShouldWarnOnce(pad), + "Discovery re-runs on every hotplug; a deliberately ignored pad " + + "must not be re-announced each time anything else arrives."); + Assert.IsTrue(UsbipAttachedInputPolicy.ShouldWarnOnce(otherPad)); + Assert.IsFalse(UsbipAttachedInputPolicy.ShouldWarnOnce(null)); + Assert.IsFalse(UsbipAttachedInputPolicy.ShouldWarnOnce(string.Empty)); + } + + /// + /// The line is all the user gets for a pad that visibly exists but is + /// ignored: it must say what was seen, admit both readings of what it + /// might be, and point at the affordance that can clear a leftover. + /// + [TestMethod] + public void TheAnnouncementCarriesTheEvidenceAndTheRemedy() + { + const string pad = @"\\?\hid#vid_054c&pid_0ce6#7&2ab44e7&0&0000#{guid}"; + string line = UsbipAttachedInputPolicy.DescribeRejectedImport(pad); + + StringAssert.Contains(line, pad); + StringAssert.Contains(line, "never used as input"); + StringAssert.Contains(line, "session that ended abruptly"); + StringAssert.Contains(line, "another program's live controller", + "Naming only the leftover reading would invite the user to " + + "clear a pad that another application is actively serving."); + StringAssert.Contains(line, "Backend process"); + } +} diff --git a/docs/dev/PLAN-PROGRESS.md b/docs/dev/PLAN-PROGRESS.md index b134b1d..0c6701d 100644 --- a/docs/dev/PLAN-PROGRESS.md +++ b/docs/dev/PLAN-PROGRESS.md @@ -3731,3 +3731,54 @@ the controller survived both Thrum start and shutdown. The log carried the report the detach became. Suite: **862 passed / 0 failed** (CI filter), from 853. + +## 2026-07-31 — Pads attached through usbip-win2 are never input + +**Session scope:** the residual the port-sweep fix (PR #30) deliberately left open, closed at +the correct layer. Branch `fix/usbip-attached-pads-are-not-input`. + +### The gap + +Input discovery's protection against ingesting our own virtual output was an in-memory +before/after path registry (`ownVirtualSonyPaths`) plus an active-port check — both records of +*this session's* creations. A session that dies hard takes the records with it, so its leftover +pad was a perfectly ordinary DualSense to the next start: ingested, mapped, and recursed on. +That recursion is what the old startup port sweep prevented by detaching — and PR #30 +established that detach-on-heuristic disconnects other applications' live pads, so the sweep +now observes only, and the ingestion path had nothing. + +The same gap covered another case nobody had named: another application's live virtual pad +(the maintainer's native-mode DS4Windows serving a DualSense) was equally ingestible the moment +its device type was enabled — reading input from a pad that a different program is serving to +games. + +### The fix + +`UsbipAttachedInputPolicy`, consulted from `DS4Devices.IsRealDS4`: any pad whose devnode +ancestry reaches the usbip-win2 UDE controller (`Global.CheckIfUsbIpWin2Device`, the ancestry +walk that already existed for the active-port check) is a virtual output being served by +something, and is never input. Ancestry does not depend on who remembers creating the device, +which is exactly what the in-memory registry could not offer. Our own live outputs are rejected +quietly as before; an unmanaged usbip pad is rejected with one log line per path — naming both +readings (leftover of a dead session / another program's live controller) and pointing at the +backend-process card that can clear a leftover with consent. + +Deliberately given up: a *real* controller forwarded from another machine over usbip-win2 can +no longer be input. usbip-win2 exists on these machines solely as VIIPER's transport, remote +forwarding has VirtualHere as the supported route (`CheckIfVirtualDevice`'s exclusion list), +and the refusal is named in the log — if the combination ever matters it arrives as a feature +request with evidence, not as silence. + +### Verification + +Five policy tests (verdict table, warn-once per path across hotplug re-runs, announcement +content: evidence, both readings, remedy). Suite: **867 passed / 0 failed** (CI filter), +from 862. + +**Live, against the real coexistence scenario.** With the native-mode build serving its virtual +DualSense on usbip port 1 and DualSense input enabled in an isolated run, one discovery pass +produced both halves of the proof: the usbip-attached virtual pad was refused with the new log +line, and the user's *physical* DualSense on Bluetooth — same VID/PID — was accepted as a +controller in the same breath. Selectivity, not a VID/PID blanket: position in the device tree +is the discriminator. The native application, its virtual pad, and its usbip import were +untouched throughout.