Re: [PATCH v10 1/5] HID: wacom: Use hdev->product in wacom_setup_touch_input_capabilities

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Critical] Missing cancellation of asynchronous works and timers in the probe error path leads to use-after-free.
--

commit ca8b53c76679ad840d06b68ac883d22ba58fe317
Author: Lee Jones <lee@kernel.org>
Subject: HID: wacom: Use hdev->product in wacom_setup_touch_input_capabilities

This commit replaces lookup-dependent touch product references with
hdev->product inside wacom_setup_touch_input_capabilities(). This
simplifies the code since hdev is already available via container_of
and represents the touch device itself.

> diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
> index 8feb8027be95e..7cf2b4de52be7 100644
> --- a/drivers/hid/wacom_wac.c
> +++ b/drivers/hid/wacom_wac.c
[ ... ]

While reviewing the wacom driver lifecycle, I noticed a potential issue in
a related file during the probe error path:

drivers/hid/wacom_sys.c:wacom_parse_and_register() {
        ...
        if (features->device_type & WACOM_DEVICETYPE_WL_MONITOR) {
                error = hid_hw_open(hdev);
                if (error) {
                        hid_err(hdev, "hw open failed\n");
                        goto fail_hw_stop;
                }
        }
        ...
fail_hw_stop:
        hid_hw_stop(hdev);
fail:
        wacom_release_resources(wacom);
        return error;
}

[Severity: Critical]
This is a pre-existing issue, but does this error path leave asynchronous
works and timers running after freeing the wacom structure?

The driver allocates the wacom structure using devm_kzalloc(). If an
initialization step fails after hid_hw_start() is called, such as
hid_hw_open() failing, the code jumps to the fail_hw_stop label.

While hid_hw_stop() is called, it appears to omit cancelling other
asynchronous resources like wireless_work, battery_work, remote_work,
mode_change_work, aes_battery_work, and idleprox_timer.

Since hid_hw_start() enables hardware interrupts, an incoming interrupt can
trigger wacom_raw_event() -> wacom_wac_irq() -> wacom_schedule_work(),
which queues these works.

If the probe function returns an error, devres will automatically free the
wacom structure. Could an untrusted device triggering rapid reports cause
these pending works to execute later and dereference the freed memory?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/fa3cb42f7b58c1b3022d8208969a81882783a5ec.1788868602.git.lee@kernel.org?part=1