Re: [PATCH RFC 4/4] HID: logitech-hidpp: support ratchet wheel

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] hid: UAF in hidpp20_ratchet_wheel_raw_event() on registration failure
--

commit ef099676a0f82cda930a8a2414084557ddb81f29
Author: Erik Håkansson <erikhakan@gmail.com>

HID: logitech-hidpp: support ratchet wheel

This commit adds support for the 0x2130 Ratchet wheel feature on Logitech
devices. It detects the feature, handles HID++ scroll events, and enables
HID++ wheel diversion by default for Bolt-connected devices.

> diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
> index 9217ad4a97f43..6195080505cbb 100644
> --- a/drivers/hid/hid-logitech-hidpp.c
> +++ b/drivers/hid/hid-logitech-hidpp.c

[ ... ]

> @@ -4213,6 +4267,44 @@ static int hidpp20_hires_wheel_raw_event(struct hidpp_device *hidpp,
>  	return 0;
>  }
>  
> +static int hidpp20_ratchet_wheel_raw_event(struct hidpp_device *hidpp,
> +					   u8 *data, int size)
> +{
> +	int delta_v, delta_h;
> +
> +	if (hidpp->ratchet_wheel_feature_index == 0xff)
> +		return 0;
> +
> +	if (size < 6)
> +		return 0;
> +
> +	if (data[0] != REPORT_ID_HIDPP_LONG ||
> +	    data[2] != hidpp->ratchet_wheel_feature_index)
> +		return 0;
> +
> +	if (data[3] != EVENT_RATCHET_WHEEL_MOVEMENT)
> +		return 0;
> +
> +	if (!hidpp->input)
> +		return 1;
> +
> +	delta_v = (s8)data[4];
> +	delta_h = (s8)data[5];
> +	if (delta_v) {
> +		input_report_rel(hidpp->input, REL_WHEEL_HI_RES, delta_v * 120);

[Severity: High]
Could this lead to a use-after-free if input device registration fails?

In the hidpp_connect_event() worker, the allocated input device is assigned to
hidpp->input before calling input_register_device(). If registration fails,
the worker handles the failure by setting hidpp->input = NULL and freeing the
device.

Because hidpp20_ratchet_wheel_raw_event() is a lockless interrupt handler,
is it possible for it to check that hidpp->input is valid, race with the
failure path freeing the device, and then dereference a freed pointer when
calling input_report_rel()?

> +		input_report_rel(hidpp->input, REL_WHEEL, delta_v);
> +	}
> +	if (delta_h) {
> +		input_report_rel(hidpp->input, REL_HWHEEL_HI_RES,
> +				 delta_h * 120);
> +		input_report_rel(hidpp->input, REL_HWHEEL, delta_h);
> +	}
> +	if (delta_v || delta_h)
> +		input_sync(hidpp->input);
> +
> +	return 1;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261002-bolt-input-rfc-v1-0-333e1f350586@gmail.com?part=4