Re: [PATCH 1/2] HID: logitech-hidpp: fix hi-res scroll for Bolt-connected MX Master

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] hid: logitech: OOB read via invalid parent cast in hidpp_is_bolt_child()
--

commit 4a9ffd2a32ed4f113b3f3e2880f7d7d6145199e3
Author: Rafael Passos <rafael@rcpassos.me>

HID: logitech-hidpp: fix hi-res scroll for Bolt-connected MX Master

This patch routes the mouse wheel using HID++ for Bolt receivers so ticks
are scaled by the multiplier and correctly reported as a hi-res event. It
also reorders the hidpp_is_bolt_child function to be used in
hidpp_connect_event to check the gate for hidpp_initialize_hires_scroll.

> diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
> index 4e00ac91493f7..166980be57b7c 100644
> --- a/drivers/hid/hid-logitech-hidpp.c
> +++ b/drivers/hid/hid-logitech-hidpp.c

[ ... ]

> @@ -3891,6 +3891,19 @@ static int hidpp10_consumer_keys_raw_event(struct hidpp_device *hidpp,
>  	return 1;
>  }
>  
> +static bool hidpp_is_bolt_child(struct hid_device *hdev)
> +{
> +	struct device *parent = hdev->dev.parent;
> +	struct hid_device *receiver_hdev;
> +
> +	if (!parent)
> +		return false;
> +
> +	receiver_hdev = to_hid_device(parent);
> +	return receiver_hdev->vendor == USB_VENDOR_ID_LOGITECH &&

[Severity: High]
Does this code perform an invalid cast for non-Bolt devices that leads to an
out-of-bounds read?

When standard USB devices or Bluetooth devices connect, their parent is not a
struct hid_device (it's typically a struct usb_interface or an HCI connection).
Using to_hid_device() on these parents results in an invalid container_of cast,
and accessing receiver_hdev->vendor reads from unrelated slab memory, which
could lead to a KASAN slab-out-of-bounds splat or kernel panic.

> +	       receiver_hdev->product == USB_DEVICE_ID_LOGITECH_BOLT_RECEIVER;
> +}
> +
>  /* -------------------------------------------------------------------------- */
>  /* High-resolution scroll wheels                                              */
>  /* -------------------------------------------------------------------------- */
> @@ -3901,7 +3914,9 @@ static int hi_res_scroll_enable(struct hidpp_device *hidpp)
>  	u8 multiplier = 1;
>  
>  	if (hidpp->capabilities & HIDPP_CAPABILITY_HIDPP20_HI_RES_WHEEL) {
> -		ret = hidpp_hrw_set_wheel_mode(hidpp, false, true, false);
> +		bool use_hidpp = hidpp_is_bolt_child(hidpp->hid_dev);

[Severity: High]
For Bluetooth devices with hi-res scroll capabilities, doesn't this
unconditionally call hidpp_is_bolt_child(), exposing them to the invalid
cast mentioned above?

> +
> +		ret = hidpp_hrw_set_wheel_mode(hidpp, false, true, use_hidpp);
>  		if (ret == 0)
>  			ret = hidpp_hrw_get_wheel_capability(hidpp, &multiplier);
>  	} else if (hidpp->capabilities & HIDPP_CAPABILITY_HIDPP20_HI_RES_SCROLL) {

[ ... ]

> @@ -4553,7 +4568,9 @@ static void hidpp_connect_event(struct work_struct *work)
>  	}
>  
>  	hidpp_initialize_battery(hidpp);
> -	if (!hid_is_usb(hidpp->hid_dev))
> +
> +	if (!hid_is_usb(hidpp->hid_dev) ||
> +	    hidpp_is_bolt_child(hidpp->hid_dev))

[Severity: High]
For standard USB devices, !hid_is_usb(hidpp->hid_dev) is false, which forces
the evaluation of hidpp_is_bolt_child(). Doesn't this expose all standard
native USB Logitech devices to the invalid parent cast and out-of-bounds read?

>  		hidpp_initialize_hires_scroll(hidpp);
>  
>  	/* forward current battery state */

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260922-feature-bolt-fix-v1-0-63b0fa8da0d3@gmail.com?part=1