Re: [PATCH v2 2/5] HID: wacom: check the input devices in the legacy irq handlers

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] HID: wacom: requiring pad_input in wacom_dtus_irq() breaks DTUSX pen input
--

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

HID: wacom: check the input devices in the legacy irq handlers

This commit adds NULL checks for wacom->pen_input and wacom->pad_input at
the entry of six legacy Wacom interrupt handlers to prevent NULL pointer
dereferences when an interface only initializes a subset of the input
devices.

> diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
> index 567b01882eb1e..aa08afb3c5ef0 100644
> --- a/drivers/hid/wacom_wac.c
> +++ b/drivers/hid/wacom_wac.c

[ ... ]

> @@ -293,6 +317,13 @@ static int wacom_dtus_irq(struct wacom_wac *wacom)
>  	struct input_dev *input = wacom->pen_input;
>  	unsigned short prox, pressure = 0;
>  
> +	if (!input || !wacom->pad_input) {
> +		dev_warn_once(&container_of(wacom, struct wacom, wacom_wac)->hdev->dev,
> +			      "%s: expected pen and pad input devices\n",
> +			      __func__);
> +		return 0;
> +	}

[Severity: High]
Does requiring wacom->pad_input at the start of wacom_dtus_irq() break pen
input on Wacom DTU-1031X (DTUSX) tablets?

In drivers/hid/wacom_wac.c, wacom_wac_irq() dispatches both DTUS and DTUSX
devices to wacom_dtus_irq():

	case DTUS:
	case DTUSX:
		sync = wacom_dtus_irq(wacom_wac);
		break;

Unlike DTUS tablets, the Wacom DTU-1031X (wacom_features_0x32F, with type
DTUSX) is pen-only and has 0 numbered buttons:

static const struct wacom_features wacom_features_0x32F =
	{ "Wacom DTU1031X", 22672, 12928, 511, 0,
	  DTUSX, WACOM_INTUOS_RES, WACOM_INTUOS_RES, 0,
	  WACOM_DTU_OFFSET, WACOM_DTU_OFFSET,
	  WACOM_DTU_OFFSET, WACOM_DTU_OFFSET };

During probe, wacom_setup_device_quirks() only sets WACOM_DEVICETYPE_PAD
for DTUS, not DTUSX:

	if (features->type == GRAPHIRE_BT || features->type == WACOM_G4 ||
	    features->type == DTUS ||
	    (features->type >= INTUOS3S && features->type <= WACOM_MO)) {
		if (features->device_type & WACOM_DEVICETYPE_PEN)
			features->device_type |= WACOM_DEVICETYPE_PAD;
	}

And wacom_setup_pad_input_capabilities() returns -ENODEV for DTUSX, which
causes wacom_setup_inputs() to free pad_input and leave wacom->pad_input
as NULL.

When WACOM_REPORT_DTUS pen reports arrive from a DTUSX tablet, won't this
new check always trigger and drop all pen events? Should wacom->pad_input
only be required when features->type != DTUSX (with a check in the
WACOM_REPORT_DTUSPAD branch), or checked specifically when handling
WACOM_REPORT_DTUSPAD reports?

[ ... ]

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