Re: [PATCH AUTOSEL 6.18] HID: wacom: avoid copying Bluetooth input reports

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] HID: wacom: unaligned 16-bit reads in wacom_intuos_general() on BT reports
--

commit c4e8a30b627ffd270cacd2aea99e17a24a724562
Author: Ruoyu Wang <ruoyuw560@gmail.com>

HID: wacom: avoid copying Bluetooth input reports

This commit avoids duplicating Bluetooth reports with kmemdup() in
wacom_intuos_bt_irq() by temporarily updating wacom->data to point at each
10-byte payload slice in wacom_intuos_bt_process_data() before calling
wacom_intuos_irq().

> diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
> index af76e49fd0f25..8feb8027be95e 100644
> --- a/drivers/hid/wacom_wac.c
> +++ b/drivers/hid/wacom_wac.c
> @@ -1192,8 +1192,11 @@ static int int_dist(int x1, int y1, int x2, int y2)
>  static void wacom_intuos_bt_process_data(struct wacom_wac *wacom,
>  		unsigned char *data)
>  {
> -	memcpy(wacom->data, data, 10);
> +	u8 *saved_data = wacom->data;
> +
> +	wacom->data = data;
>  	wacom_intuos_irq(wacom);
> +	wacom->data = saved_data;

[Severity: Medium]
Could pointing wacom->data directly at the payload slice introduce unaligned
16-bit memory accesses in wacom_intuos_general()?

When an Intuos4 Wireless report 0x03 or 0x04 arrives in wacom_intuos_bt_irq(),
it calls wacom_intuos_bt_process_data(wacom, data + i) with odd byte offsets
(i = 1, 11, and 21):

drivers/hid/wacom_wac.c:wacom_intuos_bt_irq() {
	...
	switch (data[0]) {
	case 0x04:
		...
		wacom_intuos_bt_process_data(wacom, data + i);
		i += 10;
		fallthrough;
	case 0x03:
		...
		wacom_intuos_bt_process_data(wacom, data + i);
		i += 10;
		wacom_intuos_bt_process_data(wacom, data + i);
		i += 10;
	...
}

Previously, memcpy(wacom->data, data, 10) copied each 10-byte payload to the
start of the aligned wacom->data buffer. By setting wacom->data to data + 1,
data + 11, or data + 21 directly, the call chain
wacom_intuos_bt_process_data() -> wacom_intuos_irq() ->
wacom_intuos_general() will dereference &data[2] and &data[4] at odd byte
offsets (3, 5, 13, 15, 23, and 25) via be16_to_cpup():

drivers/hid/wacom_wac.c:wacom_intuos_general() {
	...
	x = (be16_to_cpup((__be16 *)&data[2]) << 1) | ((data[9] >> 1) & 1);
	y = (be16_to_cpup((__be16 *)&data[4]) << 1) | (data[9] & 1);
	...
}

Since be16_to_cpup() expects natural 16-bit alignment, won't this trigger
UBSAN alignment splats or alignment faults on strict-alignment architectures
unless get_unaligned_be16() is used?

>  
>  	input_sync(wacom->pen_input);
>  	if (wacom->pad_input)

[ ... ]

>
>
>

[]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261019135441.1420968-386-sashal@kernel.org?part=1