diff for duplicates of <cover.1790795726.git.f3nr1l@me.com> diff --git a/a/1.txt b/N1/1.txt index ed7fae7..9a4f501 100644 --- a/a/1.txt +++ b/N1/1.txt @@ -1,58 +1,67 @@ -Two teardown bugs in hid-winwing, both present in mainline. The series is -based on hid.git for-next, which already carries the related fix -a1a5ad37e50c ("HID: winwing: fix use-after-free in force feedback -teardown"); it does not depend on it. - -Patch 1 fixes a use-after-free of the rumble work item. winwing_remove() -cancels the work before it stops the device, but stopping the device flushes -the force feedback effects, which calls the driver's play_effect handler one -last time and queues the work again: - - hid_hw_stop() -> input_unregister_device() -> evdev_cleanup() - -> input_flush_device() -> input_ff_flush() -> erase_effect() - -> ml_ff_playback(dev, id, 0) -> ml_play_effects() - -> winwing_play_effect() -> schedule_work(&data->rumble_work) - -The data the work runs on is devm-allocated, and hid_device_remove() releases -the driver's devres group as soon as .remove returns, so the requeued work -runs on freed memory. Cancelling after hid_hw_stop() closes the window: once -the input device is gone nothing can queue the work again, and the driver -data is still valid until .remove returns. - -Patch 2 initialises data->lights_lock, which winwing_led_write() has been -taking since the driver was merged. The mutex only ever gets the zeroing from -devm_kzalloc(); CONFIG_DEBUG_MUTEXES and lockdep both flag it on the first -brightness write. - -Patch 1 needs a device with a rumble motor to trigger; patch 2 affects every -supported device. Both were pointed out by the automated Sashiko review of -the earlier force feedback fix on linux-input, and confirmed by reading the -teardown path rather than by a crash. I have since exercised patch 1 on URSA MINOR sticks, with the URSA -MINOR series (posted separately, on top of this one) applied: unloading the -module while a 5 s rumble effect is playing. ftrace shows the chain above -taking place inside hid_hw_stop(), and the requeued work running before -winwing_remove() returns; no warning or oops (on a kernel without KASAN). - -A third, related issue is deliberately left out of this series. The LED class -devices are registered with devm_led_classdev_register(), so they outlive -hid_hw_stop() by the length of the devres pass that hid_device_remove() runs -after .remove returns. A sysfs brightness write in that window reaches -hid_hw_output_report() on a stopped device; usbhid returns an error once its -output URB pointer has been cleared, but that check is not serialised against -usbhid_stop(). Fixing it inside the driver means dropping devm for the LEDs -and unwinding them by hand in the probe error paths - a fair amount of churn -for a narrow race, and the same shape exists in other HID drivers that -register LEDs with devm, so it may belong in the HID core instead. I have a -driver-side patch ready and will post it separately if you prefer that. - -René Onier (2): - HID: winwing: fix use-after-free of the rumble work - HID: winwing: initialize the lights_lock mutex - - drivers/hid/hid-winwing.c | 12 +++++++++--- - 1 file changed, 9 insertions(+), 3 deletions(-) +This series adds support for the WinWing URSA MINOR joysticks (a pair of +single-hand sticks) to hid-winwing, on top of the existing Orion 2 throttle +support. + +The URSA MINOR sticks differ from the Orion 2 in three ways the series +addresses: + + - they drive a single backlight LED through the stick base (a second HID + controller on the same endpoint, addressed by a fixed device id), not + through the Orion 2 lighting controller; + - they carry a single rumble motor in the grip, rather than the two + motors of the Orion 2 grips; + - they enumerate under their own product ids. + +Patches 1 and 2 are preparatory refactors with no functional change (verified +byte-for-byte): patch 1 factors the vendor report builder out of the LED and +rumble paths and names the report fields, patch 2 makes the LED set and the +lighting controller model-dependent. Patch 3 adds the device ids and the +backlight. Patch 4 drives the single grip motor while leaving the Orion 2 +two-motor path untouched. + +This supersedes my earlier "HID: winwing: add support for URSA MINOR combat +joysticks", sent from my gmail address, which Jiri asked me to resend with +full paths: +https://lore.kernel.org/linux-input/20260404172641.195619-1-rene.onier@gmail.com/ +The device ids and the extended button mapping it enabled are in patch 3, +now together with the backlight and the rumble motor. + +The series is based on hid.git for-next and applies on top of the two-patch +series "HID: winwing: two teardown fixes": +https://lore.kernel.org/linux-input/cover.1790795726.git.f3nr1l@me.com/ +The only interaction is one line of context in winwing_probe(). for-next +already carries a1a5ad37e50c ("HID: winwing: fix use-after-free in force +feedback teardown"), which the new ids need since they take the +force-feedback path. + +The Fighter and Space URSA MINOR variants are electrically identical and share +these product ids; only a stick-tilt accessory differs. The Civil variant, with +fewer buttons, is likely compatible but its ids have not been verified on +hardware, so it is left out. + +Tested on URSA MINOR hardware (a Space left and a Fighter right): backlight, +rumble and buttons, and unloading the module while a rumble effect plays. +I could not test the Orion 2 path myself; the two-motor rumble is unchanged +and its reports are byte-identical to before (verified), but a Tested-by on +Orion 2 would be welcome. + +The vendor command names used here (SET_LEDX and the report layout) were +recovered from the vendor software's own debug logs and the command table in +its WWTHID.dll, so the report fields can be named rather than left as magic +numbers. + +René Onier (4): + HID: winwing: factor out vendor SET_LEDX report builder + HID: winwing: make the LED set and lighting controller model-dependent + HID: winwing: add URSA MINOR sticks + HID: winwing: drive the URSA MINOR rumble motor + + drivers/hid/hid-winwing.c | 267 ++++++++++++++++++++++++++------------ + 1 file changed, 185 insertions(+), 82 deletions(-) base-commit: 145c2b2e9a5c0f794fb4009bcb072ab19f8ccfcd +prerequisite-patch-id: d1a20c8f9775ea37cf424f0c7026816fbfecd127 +prerequisite-patch-id: e6427bc64a0650061572e3923ea44827dbd63809 -- 2.55.0 diff --git a/a/content_digest b/N1/content_digest index 9e9d58e..75cf7d7 100644 --- a/a/content_digest +++ b/N1/content_digest @@ -1,72 +1,80 @@ "From\0Ren\303\251 Onier <f3nr1l@me.com>\0" - "Subject\0[PATCH 0/2] HID: winwing: two teardown fixes\0" - "Date\0Wed, 30 Sep 2026 15:37:49 -0400\0" + "Subject\0[PATCH 0/4] HID: winwing: add WinWing URSA MINOR sticks\0" + "Date\0Wed, 30 Sep 2026 15:55:56 -0400\0" "To\0Benjamin Tissoires <bentiss@kernel.org>" " Jiri Kosina <jikos@kernel.org>\0" "Cc\0Ren\303\251 Onier <f3nr1l@me.com>" Ivan Gorinov <linux-kernel@altimeter.info> linux-input@vger.kernel.org - linux-kernel@vger.kernel.org - " Dmitry Torokhov <dmitry.torokhov@gmail.com>\0" + " linux-kernel@vger.kernel.org\0" "\00:1\0" "b\0" - "Two teardown bugs in hid-winwing, both present in mainline. The series is\n" - "based on hid.git for-next, which already carries the related fix\n" - "a1a5ad37e50c (\"HID: winwing: fix use-after-free in force feedback\n" - "teardown\"); it does not depend on it.\n" + "This series adds support for the WinWing URSA MINOR joysticks (a pair of\n" + "single-hand sticks) to hid-winwing, on top of the existing Orion 2 throttle\n" + "support.\n" "\n" - "Patch 1 fixes a use-after-free of the rumble work item. winwing_remove()\n" - "cancels the work before it stops the device, but stopping the device flushes\n" - "the force feedback effects, which calls the driver's play_effect handler one\n" - "last time and queues the work again:\n" + "The URSA MINOR sticks differ from the Orion 2 in three ways the series\n" + "addresses:\n" "\n" - " hid_hw_stop() -> input_unregister_device() -> evdev_cleanup()\n" - " -> input_flush_device() -> input_ff_flush() -> erase_effect()\n" - " -> ml_ff_playback(dev, id, 0) -> ml_play_effects()\n" - " -> winwing_play_effect() -> schedule_work(&data->rumble_work)\n" + " - they drive a single backlight LED through the stick base (a second HID\n" + " controller on the same endpoint, addressed by a fixed device id), not\n" + " through the Orion 2 lighting controller;\n" + " - they carry a single rumble motor in the grip, rather than the two\n" + " motors of the Orion 2 grips;\n" + " - they enumerate under their own product ids.\n" "\n" - "The data the work runs on is devm-allocated, and hid_device_remove() releases\n" - "the driver's devres group as soon as .remove returns, so the requeued work\n" - "runs on freed memory. Cancelling after hid_hw_stop() closes the window: once\n" - "the input device is gone nothing can queue the work again, and the driver\n" - "data is still valid until .remove returns.\n" + "Patches 1 and 2 are preparatory refactors with no functional change (verified\n" + "byte-for-byte): patch 1 factors the vendor report builder out of the LED and\n" + "rumble paths and names the report fields, patch 2 makes the LED set and the\n" + "lighting controller model-dependent. Patch 3 adds the device ids and the\n" + "backlight. Patch 4 drives the single grip motor while leaving the Orion 2\n" + "two-motor path untouched.\n" "\n" - "Patch 2 initialises data->lights_lock, which winwing_led_write() has been\n" - "taking since the driver was merged. The mutex only ever gets the zeroing from\n" - "devm_kzalloc(); CONFIG_DEBUG_MUTEXES and lockdep both flag it on the first\n" - "brightness write.\n" + "This supersedes my earlier \"HID: winwing: add support for URSA MINOR combat\n" + "joysticks\", sent from my gmail address, which Jiri asked me to resend with\n" + "full paths:\n" + "https://lore.kernel.org/linux-input/20260404172641.195619-1-rene.onier@gmail.com/\n" + "The device ids and the extended button mapping it enabled are in patch 3,\n" + "now together with the backlight and the rumble motor.\n" "\n" - "Patch 1 needs a device with a rumble motor to trigger; patch 2 affects every\n" - "supported device. Both were pointed out by the automated Sashiko review of\n" - "the earlier force feedback fix on linux-input, and confirmed by reading the\n" - "teardown path rather than by a crash. I have since exercised patch 1 on URSA MINOR sticks, with the URSA\n" - "MINOR series (posted separately, on top of this one) applied: unloading the\n" - "module while a 5 s rumble effect is playing. ftrace shows the chain above\n" - "taking place inside hid_hw_stop(), and the requeued work running before\n" - "winwing_remove() returns; no warning or oops (on a kernel without KASAN).\n" + "The series is based on hid.git for-next and applies on top of the two-patch\n" + "series \"HID: winwing: two teardown fixes\":\n" + "https://lore.kernel.org/linux-input/cover.1790795726.git.f3nr1l@me.com/\n" + "The only interaction is one line of context in winwing_probe(). for-next\n" + "already carries a1a5ad37e50c (\"HID: winwing: fix use-after-free in force\n" + "feedback teardown\"), which the new ids need since they take the\n" + "force-feedback path.\n" "\n" - "A third, related issue is deliberately left out of this series. The LED class\n" - "devices are registered with devm_led_classdev_register(), so they outlive\n" - "hid_hw_stop() by the length of the devres pass that hid_device_remove() runs\n" - "after .remove returns. A sysfs brightness write in that window reaches\n" - "hid_hw_output_report() on a stopped device; usbhid returns an error once its\n" - "output URB pointer has been cleared, but that check is not serialised against\n" - "usbhid_stop(). Fixing it inside the driver means dropping devm for the LEDs\n" - "and unwinding them by hand in the probe error paths - a fair amount of churn\n" - "for a narrow race, and the same shape exists in other HID drivers that\n" - "register LEDs with devm, so it may belong in the HID core instead. I have a\n" - "driver-side patch ready and will post it separately if you prefer that.\n" + "The Fighter and Space URSA MINOR variants are electrically identical and share\n" + "these product ids; only a stick-tilt accessory differs. The Civil variant, with\n" + "fewer buttons, is likely compatible but its ids have not been verified on\n" + "hardware, so it is left out.\n" "\n" - "Ren\303\251 Onier (2):\n" - " HID: winwing: fix use-after-free of the rumble work\n" - " HID: winwing: initialize the lights_lock mutex\n" + "Tested on URSA MINOR hardware (a Space left and a Fighter right): backlight,\n" + "rumble and buttons, and unloading the module while a rumble effect plays.\n" + "I could not test the Orion 2 path myself; the two-motor rumble is unchanged\n" + "and its reports are byte-identical to before (verified), but a Tested-by on\n" + "Orion 2 would be welcome.\n" "\n" - " drivers/hid/hid-winwing.c | 12 +++++++++---\n" - " 1 file changed, 9 insertions(+), 3 deletions(-)\n" + "The vendor command names used here (SET_LEDX and the report layout) were\n" + "recovered from the vendor software's own debug logs and the command table in\n" + "its WWTHID.dll, so the report fields can be named rather than left as magic\n" + "numbers.\n" + "\n" + "Ren\303\251 Onier (4):\n" + " HID: winwing: factor out vendor SET_LEDX report builder\n" + " HID: winwing: make the LED set and lighting controller model-dependent\n" + " HID: winwing: add URSA MINOR sticks\n" + " HID: winwing: drive the URSA MINOR rumble motor\n" + "\n" + " drivers/hid/hid-winwing.c | 267 ++++++++++++++++++++++++++------------\n" + " 1 file changed, 185 insertions(+), 82 deletions(-)\n" "\n" "\n" "base-commit: 145c2b2e9a5c0f794fb4009bcb072ab19f8ccfcd\n" + "prerequisite-patch-id: d1a20c8f9775ea37cf424f0c7026816fbfecd127\n" + "prerequisite-patch-id: e6427bc64a0650061572e3923ea44827dbd63809\n" "-- \n" 2.55.0 -d586f028a932a12f87766c13b3edbf90c91f64cd3d2fa6e51c3fc18538fca1f3 +5443298b7fee0ca720575bbccb5ffc9c62bba254fc6a46ac772fae5c5b833bf8
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.