drivers/hid/hid-playstation.c | 36 ++++++++++++++++++++++++++++------- 1 file changed, 29 insertions(+), 7 deletions(-)
dualshock4_remove() cancels ds4->dongle_hotplug_worker while input
reports can still arrive: hid_hw_close() and hid_hw_stop() only run
later in ps_remove(), so a dongle connect report in that window makes
dualshock4_dongle_parse_report() schedule the just-cancelled work
again. ds4 is devm-managed and freed once ps_remove() returns, so
dualshock4_dongle_calibration_work() then runs on freed memory.
The dualshock4_create() error path has the same exposure. Reports are
already flowing when it runs, because ps_probe() starts and opens the
device before creating it, so the connect branch of the dongle report
handler can also queue ds4->output_worker through its lightbar update.
Neither work is cancelled when creation fails, and ds4 is freed once
the failed probe unwinds its devm allocations.
Fix this by clearing a new dongle_hotplug_worker_initialized flag under
ps_dev->lock before cancelling the worker and checking it under the same
lock before scheduling, mirroring how this driver already guards
ds4->output_worker. Also drain both workers on every error path of
dualshock4_create(): cancel dongle_hotplug_worker on the early MAC-read
and device-list failures that currently return directly, and clear
output_worker_initialized and cancel ds4->output_worker as
dualshock4_remove() already does. The output-report buffer allocation
failure returns through the same cleanup.
The locked check closes the schedule-vs-clear race, and neither work
re-schedules itself, so the cancels leave nothing pending.
This issue was found by an in-house static analysis tool.
Fixes: c64ed0cd9324 ("HID: playstation: add DualShock4 dongle support.")
Cc: stable@vger.kernel.org
Co-developed-by: Song Li <songl@zju.edu.cn>
Signed-off-by: Song Li <songl@zju.edu.cn>
Signed-off-by: Fan Wu <fanwu01@zju.edu.cn>
---
drivers/hid/hid-playstation.c | 36 ++++++++++++++++++++++++++++-------
1 file changed, 29 insertions(+), 7 deletions(-)
diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
index f9dc937..1b7a18f 100644
--- a/drivers/hid/hid-playstation.c
+++ b/drivers/hid/hid-playstation.c
@@ -421,6 +421,7 @@ struct dualshock4 {
enum dualshock4_dongle_state dongle_state;
/* Used during calibration. */
struct work_struct dongle_hotplug_worker;
+ bool dongle_hotplug_worker_initialized;
/* Timestamp for sensor data */
bool sensor_timestamp_initialized;
@@ -2618,10 +2619,12 @@ static int dualshock4_dongle_parse_report(struct ps_device *ps_dev, struct hid_r
dualshock4_set_default_lightbar_colors(ds4);
- scoped_guard(spinlock_irqsave, &ps_dev->lock)
+ scoped_guard(spinlock_irqsave, &ps_dev->lock) {
ds4->dongle_state = DONGLE_CALIBRATING;
- schedule_work(&ds4->dongle_hotplug_worker);
+ if (ds4->dongle_hotplug_worker_initialized)
+ schedule_work(&ds4->dongle_hotplug_worker);
+ }
/* Don't process the report since we don't have
* calibration data, but let hidraw have it anyway.
@@ -2677,8 +2680,12 @@ static void dualshock4_remove(struct ps_device *ps_dev)
cancel_work_sync(&ds4->output_worker);
- if (ps_dev->hdev->product == USB_DEVICE_ID_SONY_PS4_CONTROLLER_DONGLE)
+ if (ps_dev->hdev->product == USB_DEVICE_ID_SONY_PS4_CONTROLLER_DONGLE) {
+ scoped_guard(spinlock_irqsave, &ds4->base.lock)
+ ds4->dongle_hotplug_worker_initialized = false;
+
cancel_work_sync(&ds4->dongle_hotplug_worker);
+ }
}
static inline void dualshock4_schedule_work(struct dualshock4 *ds4)
@@ -2770,12 +2777,15 @@ static struct ps_device *dualshock4_create(struct hid_device *hdev)
max_output_report_size = sizeof(struct dualshock4_output_report_bt);
ds4->output_report_dmabuf = devm_kzalloc(&hdev->dev, max_output_report_size, GFP_KERNEL);
- if (!ds4->output_report_dmabuf)
- return ERR_PTR(-ENOMEM);
+ if (!ds4->output_report_dmabuf) {
+ ret = -ENOMEM;
+ goto err_cancel;
+ }
if (hdev->product == USB_DEVICE_ID_SONY_PS4_CONTROLLER_DONGLE) {
ds4->dongle_state = DONGLE_DISCONNECTED;
INIT_WORK(&ds4->dongle_hotplug_worker, dualshock4_dongle_calibration_work);
+ ds4->dongle_hotplug_worker_initialized = true;
/* Override parse report for dongle specific hotplug handling. */
ps_dev->parse_report = dualshock4_dongle_parse_report;
@@ -2784,7 +2794,7 @@ static struct ps_device *dualshock4_create(struct hid_device *hdev)
ret = dualshock4_get_mac_address(ds4);
if (ret) {
hid_err(hdev, "Failed to get MAC address from DualShock4\n");
- return ERR_PTR(ret);
+ goto err_cancel;
}
snprintf(hdev->uniq, sizeof(hdev->uniq), "%pMR", ds4->base.mac_address);
@@ -2796,7 +2806,7 @@ static struct ps_device *dualshock4_create(struct hid_device *hdev)
ret = ps_devices_list_add(ps_dev);
if (ret)
- return ERR_PTR(ret);
+ goto err_cancel;
ret = dualshock4_get_calibration_data(ds4);
if (ret) {
@@ -2858,6 +2868,18 @@ static struct ps_device *dualshock4_create(struct hid_device *hdev)
err:
ps_devices_list_remove(ps_dev);
+err_cancel:
+ scoped_guard(spinlock_irqsave, &ps_dev->lock)
+ ds4->output_worker_initialized = false;
+
+ cancel_work_sync(&ds4->output_worker);
+
+ if (ds4->dongle_hotplug_worker_initialized) {
+ scoped_guard(spinlock_irqsave, &ps_dev->lock)
+ ds4->dongle_hotplug_worker_initialized = false;
+
+ cancel_work_sync(&ds4->dongle_hotplug_worker);
+ }
return ERR_PTR(ret);
}
Please drop this patch. __hid_input_report() returns -EBUSY when it cannot take hdev->driver_input_lock, and hid_device_probe() and hid_device_remove() hold that lock across the driver's ->probe() and ->remove() callbacks. hid-playstation never calls hid_device_io_start(), so input reports cannot reach dualshock4_dongle_parse_report() while dualshock4_create() or dualshock4_remove() run. The window this patch tried to guard does not exist, and the cancel_work_sync() calls that were already there are sufficient. The issue came from our static analysis tooling, which missed that the HID core serializes the report path against probe and remove. Sorry for the noise. Fan Wu
© 2016 - 2026 Red Hat, Inc.