Commit e0f6974a authored by Roderick Colenbrander's avatar Roderick Colenbrander Committed by Jiri Kosina

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: default avatarRoderick Colenbrander <roderick.colenbrander@sony.com>
CC: stable@vger.kernel.org
Signed-off-by: default avatarJiri Kosina <jkosina@suse.cz>
parent 6d4472d7
...@@ -585,10 +585,14 @@ static void sony_set_leds(struct sony_sc *sc); ...@@ -585,10 +585,14 @@ static void sony_set_leds(struct sony_sc *sc);
static inline void sony_schedule_work(struct sony_sc *sc, static inline void sony_schedule_work(struct sony_sc *sc,
enum sony_worker which) enum sony_worker which)
{ {
unsigned long flags;
switch (which) { switch (which) {
case SONY_WORKER_STATE: case SONY_WORKER_STATE:
if (!sc->defer_initialization) spin_lock_irqsave(&sc->lock, flags);
if (!sc->defer_initialization && sc->state_worker_initialized)
schedule_work(&sc->state_worker); schedule_work(&sc->state_worker);
spin_unlock_irqrestore(&sc->lock, flags);
break; break;
case SONY_WORKER_HOTPLUG: case SONY_WORKER_HOTPLUG:
if (sc->hotplug_worker_initialized) if (sc->hotplug_worker_initialized)
...@@ -2558,13 +2562,18 @@ static inline void sony_init_output_report(struct sony_sc *sc, ...@@ -2558,13 +2562,18 @@ static inline void sony_init_output_report(struct sony_sc *sc,
static inline void sony_cancel_work_sync(struct sony_sc *sc) static inline void sony_cancel_work_sync(struct sony_sc *sc)
{ {
unsigned long flags;
if (sc->hotplug_worker_initialized) if (sc->hotplug_worker_initialized)
cancel_work_sync(&sc->hotplug_worker); cancel_work_sync(&sc->hotplug_worker);
if (sc->state_worker_initialized) if (sc->state_worker_initialized) {
spin_lock_irqsave(&sc->lock, flags);
sc->state_worker_initialized = 0;
spin_unlock_irqrestore(&sc->lock, flags);
cancel_work_sync(&sc->state_worker); cancel_work_sync(&sc->state_worker);
}
} }
static int sony_input_configured(struct hid_device *hdev, static int sony_input_configured(struct hid_device *hdev,
struct hid_input *hidinput) struct hid_input *hidinput)
{ {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment