Re: [PATCH 3/5] HID: wacom: check the input device in wacom_intuos_pad()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] HID: wacom: unintended fall-through for pad packets causes data corruption or crash
--

commit b724787c53f0eff408a5ebd5d5a4bed152ed7f82
Author: Jinmo Yang <jinmo44.yang@gmail.com>

    HID: wacom: check the input device in wacom_intuos_pad()
    
    wacom_intuos_pad() takes wacom->pad_input without checking it. An
    interface that declares no pad leaves the pointer NULL on a fully
    successful probe, and wacom_intuos_irq() still calls this function for
    pad report ids.

    [ ... ]
    
    Fixes: 10059cdc0ad0 ("Input: wacom - split out the pad device for Intuos/Cintiq")
    Cc: stable@vger.kernel.org
    Signed-off-by: Jinmo Yang <jinmo44.yang@gmail.com>

> diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
> index 794c2865064aa..48a58a6672c21 100644
> --- a/drivers/hid/wacom_wac.c
> +++ b/drivers/hid/wacom_wac.c
> @@ -553,6 +553,9 @@ static int wacom_intuos_pad(struct wacom_wac *wacom)
>  	bool wrench = false, keyboard = false, mute_touch = false, menu = false,
>  	     info = false;
>  
> +	if (!input)
> +		return 0;
> +

[Severity: High]
Does returning 0 here for a pad packet cause it to fall through and be misparsed
by other handlers?

In wacom_intuos_irq(), if wacom_intuos_pad() returns 0, the packet falls through
to wacom_intuos_inout():

drivers/hid/wacom_wac.c:wacom_intuos_irq() {
	...
	/* process pad events */
	result = wacom_intuos_pad(wacom);
	if (result)
		return result;

	/* process in/out prox events */
	result = wacom_intuos_inout(wacom);
	...
}

Because wacom_intuos_inout() only checks data[1] and doesn't verify the report
ID in data[0]:

drivers/hid/wacom_wac.c:wacom_intuos_inout() {
	...
	if (!(((data[1] & 0xfc) == 0xc0) ||  /* in prox */
	    ((data[1] & 0xfe) == 0x20) ||    /* in range */
	    ((data[1] & 0xfe) == 0x80)))     /* out prox */
		return 0;
	...
}

If data[1] of the unhandled pad packet happens to match one of these proximity
bitmasks (like 0x80 for out-prox), wacom_intuos_inout() will misparse the
packet. If it hits the out-prox case, it calls wacom_exit_report() which
dereferences wacom->pen_input. If pen_input is NULL (e.g. on a touch-only
interface), could this cause another NULL pointer dereference?

Should this !input check be placed after validating data[0], and return 1 to
correctly consume the report without falling through?

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

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