Skip to content

Commit 523db78

Browse files
unamemgregkh
authored andcommitted
HID: letsketch: fix UAF on inrange_timer at driver unbind
commit 46c8bee upstream. letsketch_driver does not provide a .remove callback, but letsketch_probe() arms a per-device timer: timer_setup(&data->inrange_timer, letsketch_inrange_timeout, 0); The timer is re-armed from letsketch_raw_event() with a 100 ms timeout on every pen-in-range report, and its callback dereferences data->input_tablet to deliver a synthetic BTN_TOOL_PEN release. letsketch_data is allocated with devm_kzalloc(), and its input_dev fields are devm-allocated via letsketch_setup_input_tablet(). On device unbind (USB unplug or rmmod), the HID core runs its default teardown and devm cleanup frees both letsketch_data and the input devices. Because no .remove callback exists, nothing drains the timer first: if raw_event armed it within ~100 ms of the unbind, the pending timer fires on freed memory. This is a UAF read of data and of data->input_tablet, followed by input_report_key() / input_sync() into the freed input_dev. The same problem can occur on the probe error path: if hid_hw_start() enabled I/O on an always-poll-quirk device and then failed, raw_event may have armed the timer before devm releases data. Fix by adding a .remove callback that calls hid_hw_stop() first. hid_hw_stop() synchronously kills the URBs that deliver raw_event(), so once it returns no path can re-arm the timer. timer_shutdown_sync() then drains any in-flight callback and permanently disables further mod_timer() calls. Apply the same timer_shutdown_sync() in the probe error path so the timer is guaranteed not to outlive data. Fixes: 33a5c27 ("HID: Add new Letsketch tablet driver") Cc: stable@vger.kernel.org Signed-off-by: Manish Khadka <maskmemanish@gmail.com> Signed-off-by: Jiri Kosina <jkosina@suse.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent 1a1ebdc commit 523db78

1 file changed

Lines changed: 33 additions & 3 deletions

File tree

drivers/hid/hid-letsketch.c

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -295,13 +295,42 @@ static int letsketch_probe(struct hid_device *hdev, const struct hid_device_id *
295295

296296
ret = letsketch_setup_input_tablet(data);
297297
if (ret)
298-
return ret;
298+
goto err_shutdown_timer;
299299

300300
ret = letsketch_setup_input_tablet_pad(data);
301301
if (ret)
302-
return ret;
302+
goto err_shutdown_timer;
303+
304+
ret = hid_hw_start(hdev, HID_CONNECT_HIDRAW);
305+
if (ret)
306+
goto err_shutdown_timer;
303307

304-
return hid_hw_start(hdev, HID_CONNECT_HIDRAW);
308+
return 0;
309+
310+
err_shutdown_timer:
311+
/*
312+
* Drain any pending callback and permanently disable the timer
313+
* before devm releases data: if hid_hw_start() enabled I/O on an
314+
* always-poll-quirk device and then failed, raw_event may have
315+
* armed the timer already.
316+
*/
317+
timer_shutdown_sync(&data->inrange_timer);
318+
return ret;
319+
}
320+
321+
static void letsketch_remove(struct hid_device *hdev)
322+
{
323+
struct letsketch_data *data = hid_get_drvdata(hdev);
324+
325+
/*
326+
* hid_hw_stop() synchronously kills the URBs that deliver
327+
* raw_event(), so once it returns no path can re-arm
328+
* inrange_timer. timer_shutdown_sync() then drains any
329+
* in-flight callback and permanently disables further
330+
* mod_timer() calls before devm releases data.
331+
*/
332+
hid_hw_stop(hdev);
333+
timer_shutdown_sync(&data->inrange_timer);
305334
}
306335

307336
static const struct hid_device_id letsketch_devices[] = {
@@ -314,6 +343,7 @@ static struct hid_driver letsketch_driver = {
314343
.name = "letsketch",
315344
.id_table = letsketch_devices,
316345
.probe = letsketch_probe,
346+
.remove = letsketch_remove,
317347
.raw_event = letsketch_raw_event,
318348
};
319349
module_hid_driver(letsketch_driver);

0 commit comments

Comments
 (0)