Re: [PATCH v2] HID: wacom: fix NULL pointer dereference in wacom_intuos_pad()

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

New issues:
- [High] The patch's early return in `wacom_intuos_pad()` causes valid pad reports to fall through to pen handlers when `pad_input` is NULL.

Pre-existing issues:
- [High] The patch addresses a NULL pointer dereference for `pad_input` but completely misses identical vulnerabilities for `pen_input` and `touch_input`.
- [High] Concurrent execution of `wacom_mode_change_work()` and `wacom_raw_event()` can lead to a Use-After-Free of input devices.
- [High] A dangling pointer in `wacom->shared->touch_input` leads to Use-After-Free when the Touch interface unbinds while the Pad interface remains active.
--

commit e74a772fd06994c14eb8d49710acd3ad8740df1f
Author: Jinmo Yang [off-list ref]

HID: wacom: fix NULL pointer dereference in wacom_intuos_pad()

This commit adds a NULL check for pad_input at the top of
wacom_intuos_pad() to bail out early when the pad input device was not set
up, preventing a general protection fault.

[Severity: High]
Does returning 0 here allow the event to fall through to other handlers?

wacom_intuos_irq() relies on a non-zero return to halt processing. If this
returns 0, a crafted pad packet might fall through to wacom_intuos_inout().

Since wacom_intuos_inout() only checks data[1] and not the report ID, it
could misinterpret a pad report as a pen proximity event. For example, if
data[1] == 0x80, could it trigger wacom_exit_report() and either inject a
spurious pen exit event or cause a NULL pointer dereference if pen_input
is also NULL?

Would it be better to place this check after verifying the report ID, and
return 1 to securely drop the packet?

[Severity: High]
This isn't a bug introduced by this patch, but does the same NULL pointer
vulnerability exist for pen_input and touch_input?

A crafted UHID descriptor can omit pen and touch capabilities, leaving
wacom->pen_input and wacom->touch_input as NULL.

If valid pen or touch packets are subsequently received, handlers like
wacom_exit_report(), wacom_intuos_general(), and wacom_bpt3_touch_msg()
appear to blindly dereference these NULL pointers.

[Severity: High]
This is a pre-existing issue, but can concurrent execution of
wacom_mode_change_work() and wacom_raw_event() lead to a use-after-free
of the input devices?

wacom_mode_change_work() frees input devices asynchronously via
wacom_release_resources() and devres_release_group(), without holding the
lock that serializes input report processing.

Concurrently, a raw input report could enter wacom_intuos_pad(), read the
wacom->pad_input pointer, successfully pass this new if (!input) check,
and then access the input device right after it gets freed.

 
 	/* pad packets. Works as a second tool and is always in prox */
 	if (!(data[0] == WACOM_REPORT_INTUOSPAD || data[0] == WACOM_REPORT_INTUOS5PAD ||

[Severity: High]
This is a pre-existing issue, but can wacom->shared->touch_input become a
dangling pointer if the touch interface unbinds while the pad interface
remains active?

When a touch interface is unbound, its input devices are freed via
devres_release_group(), but wacom_remove_shared_data() does not appear to
set wacom->shared->touch_input to NULL.

If a pad event is received later, the pad event handler invokes
input_report_switch() and input_sync() on the dangling pointer:

    if (wacom->shared && wacom->shared->touch_input) {
        input_report_switch(wacom->shared->touch_input,
                            SW_MUTE_DEVICE,
                            !wacom->shared->is_touch_on);
        input_sync(wacom->shared->touch_input);
    }

Could this result in a use-after-free?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260523150619.615565-1-jinmo44.yang@gmail.com?part=1