[PATCH] HID: wacom: Cancel init_work when probe fails after scheduling it

[PATCH] HID: wacom: Cancel init_work when probe fails after scheduling it

[Aamir Ahmed]

wacom_parse_and_register() schedules wacom->init_work with a one second
delay from wacom_query_tablet_data(), and only then opens the hardware
for devices with WACOM_DEVICETYPE_WL_MONITOR, that is the wireless
receiver's monitor interface and the ExpressKey Remote. If that
hid_hw_open() fails, the error path stops the hardware and releases the
driver's resources but leaves init_work pending.

wacom_probe() then returns the error and the HID core releases the
devres group it opened around probe(), which frees the wacom structure
allocated with devm_kzalloc(). When init_work runs a second later,
wacom_init_work() derives the wacom pointer from the work item, so
_wacom_query_tablet_data() and wacom_led_control() read and write freed
memory.

This is the only work item that can be pending at that point. The
others are scheduled from incoming reports, and the HID core drops
reports for the whole of probe() by holding driver_input_lock, which
this driver never releases early. The neighbouring BAMBOO_TOUCH error
path already cancels init_work before failing for the same reason.

Cancel init_work on the shared error path, after hid_hw_stop() as
wacom_remove() does. The cancel is harmless where the work was never
scheduled or was already cancelled.

Fixes: a544c619a54b ("HID: wacom: do not attempt to switch mode while in probe")
Reported-by: Sashiko AI <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/linux-input/20260905102908.ADE1F1F00A3D@smtp.kernel.org/
Assisted-by: LLM
Signed-off-by: Aamir Ahmed <elb12345@hotmail.co.uk>
---
The Sashiko reviewer raised this in reply to "[PATCH] HID: wacom:
validate GRAPHIRE_BT report length" today, and had raised the same class
in May on "HID: wacom: stop hardware after post-start probe failures",
which reworked this error path without addressing it. I verified it
against drivers/base/dd.c, drivers/hid/hid-core.c (__hid_device_probe
releases the devres group on failure; __hid_input_report drops reports
while probe holds driver_input_lock) and drivers/hid/usbhid/hid-core.c
before writing this.

The reviewer also named wireless_work, battery_work, remote_work and the
timers. Those are only scheduled from reports, which cannot reach the
driver during probe, so I have not claimed them. Cancelling everything
wacom_remove() cancels would be harmless if you prefer a belt-and-braces
version; say so and I will send a v2.

The reviewer's remaining points, that the GRAPHIRE_BT length check could
be extended to the other device types handled by wacom_graphire_irq()
and to wacom_intuos_irq(), are hardening rather than a fix. Happy to
send that separately if wanted.

Compile-tested only, on x86_64 with GCC 13.3. I do not have a wireless
receiver or ExpressKey Remote, so the failure path was not exercised.
The fix and this changelog were drafted with an LLM assistant and
reviewed by hand.

 drivers/hid/wacom_sys.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 0eafa483b7f..ba8adefd932 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -2501,6 +2501,7 @@ static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
 
 fail_hw_stop:
 	hid_hw_stop(hdev);
+	cancel_delayed_work_sync(&wacom->init_work);
 fail:
 	wacom_release_resources(wacom);
 	return error;

base-commit: 654ae5d73c05bd2943d65636ce6cd0aa46e62f18
-- 
2.53.0.windows.1

Re: [PATCH] HID: wacom: Cancel init_work when probe fails after scheduling it

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Uncancelled background workers and timers in the wacom_parse_and_register() error path lead to UAF when called outside of probe().
--

commit 611f293eb48407c71bbdfd98b12f13ec53358ab0
Author: Aamir Ahmed <elb12345@hotmail.co.uk>

HID: wacom: Cancel init_work when probe fails after scheduling it

This commit cancels init_work on the shared error path in
wacom_parse_and_register() to prevent a use-after-free when probe fails
and devres cleans up driver resources.

Closes: https://lore.kernel.org/linux-input/20260905102908.ADE1F1F00A3D@smtp.kernel.org/

> diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
> index 0eafa483b7f75..ba8adefd93266 100644
> --- a/drivers/hid/wacom_sys.c
> +++ b/drivers/hid/wacom_sys.c
[ ... ]
> @@ -2501,6 +2501,7 @@ static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
>  
>  fail_hw_stop:
>  	hid_hw_stop(hdev);
> +	cancel_delayed_work_sync(&wacom->init_work);
>  fail:
>  	wacom_release_resources(wacom);
>  	return error;

[Severity: High]
This is a pre-existing issue, but can this error path still result in a
use-after-free if it is reached outside of probe()?

The commit message notes that reports cannot reach the driver during probe
because driver_input_lock is held, preventing other work items from being
scheduled.

However, looking at wacom_wireless_work() and wacom_mode_change_work(),
they also call wacom_parse_and_register(). In these workqueue contexts,
driver_input_lock is not held.

Once hid_hw_start() is called earlier in this function, could incoming
reports immediately schedule asynchronous tasks like remote_work,
battery_work, and aes_battery_work?

If a subsequent step like hid_hw_open() fails, execution jumps to
fail_hw_stop. While init_work is cancelled here, the other background
tasks are not.

Will wacom_release_resources() then free the devres group containing
wacom->remote and other structures, leaving those uncancelled tasks to
execute and access freed memory?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/PAXP251MB001050EFD88752318EBA75F5C8B42@PAXP251MB0010.EURP251.PROD.OUTLOOK.COM?part=1