Re: [PATCH v2] HID: hid-oxp: fix UAF on pending work in remove()
"Shengzhuo Wei" <[email protected]> Wed, 5 Aug 2026 16:01:12 +0800
| Newsgroups | org.kernel.vger.linux-input,org.kernel.vger.linux-kernel,org.kernel.vger.stable |
|---|---|
| Message-ID | <anLtyBB-Vq2D6REw@pve> |
Hi Derek, Dmitry, Derek wrote: > Out of curiosity, do you have a device available to test as well? If > so, which model do you have? I don't have any OneXPlayer hardware. I found this bug by creating a gen2 OXP device through /dev/uhid and tearing it down under KASAN in QEMU, not on a real device, and I'm not deeply familiar with the HID subsystem overall. On disable_delayed_work_sync() I seem to be caught between your two positions. Derek: > I don't think the bug fix switching to disable_delayed_work_sync is > worth the side effects without first addressing the global drvdata > issue. That would introduce a true regression to fix a theoretical > logic bug. I would rather you either switch to the devm_alloc drvdata > first or hold that change until I'm able to do it later myself. Dmitry: > If there is possibility to have multiple instances then this driver is > FUBAR in the current shape: one device instance fires up work items > for another device. So holding the patch makes no sense IMO. I'm not well placed to own the per-device drvdata refactor: I have no hardware to validate the multi-interface behavior, and I don't have the HID-subsystem familiarity to do that rework confidently. So I'd rather not take that piece on. Given that, what would you like me to do? The realistic contribution I can stand behind is a minimal v3 that only fixes the use-after-free I actually found and verified -- cancel_delayed_work() -> cancel_delayed_work_sync() in oxp_hid_remove() (the v1 approach), reverting v2's probe change and dropping disable_delayed_work_sync(). It's KASAN-verified, introduces no regression, and needs no hardware. The re-arm hardening and the drvdata/attr-ordering rework would then be Derek's, on real hardware with the hid-msi pattern. If you'd rather I drop the patch entirely and leave the whole thing (including the cancel_delayed_work_sync part) to Derek, that's fine too. I'd like to land this fix if there's a version of it you'd accept from me, but I defer to your judgment on scope -- I just didn't want to silently walk away from a bug I reported. Thanks, Shengzhuo