Re: [PATCH] HID: corsair: cancel worker after unregistering LED, not before

[Danish Khateeb]

> - [High] Reordering `led_classdev_unregister()` before
> `cancel_work_sync()` reintroduces a Time-of-Check to Time-of-Use
> (TOCTOU) Use-After-Free race condition on `led->cdev.dev` inside the
> worker threads.

You are right. Please drop v1.

There are two races here and each ordering fixes only one. Cancelling
first leaves led_classdev_unregister() free to re-arm the worker via
led_set_brightness(LED_OFF), so the structure is freed while still
linked into the worklist. Cancelling last leaves a worker that has
already tested led->removed free to dereference led->cdev.dev after
device_unregister() has freed it -- the race eb51c9f8cb4f fixed. v1 was
in effect a revert of it, which I should have caught: that race is
spelled out in the changelog of the commit in my own Fixes: tag.

v2 uses disable_work_sync(), which waits for a running worker and makes
the later schedule_work() fail rather than queue, closing both. It also
converts the fail_sysfs error path, which still had the second race.

My reproducer only covered the re-arm race; the other is a few
instructions wide and I could not trigger it. v2 says so.