Skip to content

Commit e0f6974

Browse files
Roderick ColenbranderJiri Kosina
authored andcommitted
HID: sony: Fix race condition between rumble and device remove.
Valve reported a kernel crash on Ubuntu 18.04 when disconnecting a DS4 gamepad while rumble is enabled. This issue is reproducible with a frequency of 1 in 3 times in the game Borderlands 2 when using an automatic weapon, which triggers many rumble operations. We found the issue to be a race condition between sony_remove and the final device destruction by the HID / input system. The problem was that sony_remove didn't clean some of its work_item state in "struct sony_sc". After sony_remove work, the corresponding evdev node was around for sufficient time for applications to still queue rumble work after "sony_remove". On pre-4.19 kernels the race condition caused a kernel crash due to a NULL-pointer dereference as "sc->output_report_dmabuf" got freed during sony_remove. On newer kernels this crash doesn't happen due the buffer now being allocated using devm_kzalloc. However we can still queue work, while the driver is an undefined state. This patch fixes the described problem, by guarding the work_item "state_worker" with an initialized variable, which we are setting back to 0 on cleanup. Signed-off-by: Roderick Colenbrander <[email protected]> CC: [email protected] Signed-off-by: Jiri Kosina <[email protected]>
1 parent 6d4472d commit e0f6974

File tree

1 file changed

+12
-3
lines changed

1 file changed

+12
-3
lines changed

drivers/hid/hid-sony.c

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -585,10 +585,14 @@ static void sony_set_leds(struct sony_sc *sc);
585585
static inline void sony_schedule_work(struct sony_sc *sc,
586586
enum sony_worker which)
587587
{
588+
unsigned long flags;
589+
588590
switch (which) {
589591
case SONY_WORKER_STATE:
590-
if (!sc->defer_initialization)
592+
spin_lock_irqsave(&sc->lock, flags);
593+
if (!sc->defer_initialization && sc->state_worker_initialized)
591594
schedule_work(&sc->state_worker);
595+
spin_unlock_irqrestore(&sc->lock, flags);
592596
break;
593597
case SONY_WORKER_HOTPLUG:
594598
if (sc->hotplug_worker_initialized)
@@ -2558,13 +2562,18 @@ static inline void sony_init_output_report(struct sony_sc *sc,
25582562

25592563
static inline void sony_cancel_work_sync(struct sony_sc *sc)
25602564
{
2565+
unsigned long flags;
2566+
25612567
if (sc->hotplug_worker_initialized)
25622568
cancel_work_sync(&sc->hotplug_worker);
2563-
if (sc->state_worker_initialized)
2569+
if (sc->state_worker_initialized) {
2570+
spin_lock_irqsave(&sc->lock, flags);
2571+
sc->state_worker_initialized = 0;
2572+
spin_unlock_irqrestore(&sc->lock, flags);
25642573
cancel_work_sync(&sc->state_worker);
2574+
}
25652575
}
25662576

2567-
25682577
static int sony_input_configured(struct hid_device *hdev,
25692578
struct hid_input *hidinput)
25702579
{

0 commit comments

Comments
 (0)