Re: [PATCH v2] HID: hid-oxp: fix UAF on pending work in remove()

["Shengzhuo Wei" <me@cherr.cc>]

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