Re: [PATCH 14/15] HID: hid-oxp: add Gen3 joystick ring RGB support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Invalid cast to usb_interface when bound to non-USB parent.
--

commit 6855a3cf4aaf330705ad5f598a86f86db5c13fc3
Author: Andrei Aldea <andrei1998@gmail.com>

    HID: hid-oxp: add Gen3 joystick ring RGB support
    
    Select the X2/Gen3 RGB protocol early for the ONEXPLAYER 3 and X2 Mini Pro.
    These controllers share the Gen2 HID usage page but require a 59-byte
    color payload and writes to ring zones 1, 2 and 7. Keep the Gen1 55-byte
    and legacy Gen2 57-byte color builders separate and unchanged.

> diff --git a/drivers/hid/hid-oxp.c b/drivers/hid/hid-oxp.c
> index 55f8b4773e334..9a46602238aa3 100644
> --- a/drivers/hid/hid-oxp.c
> +++ b/drivers/hid/hid-oxp.c

[ ... ]

> @@ -1868,6 +2199,7 @@ static int oxp_cfg_probe(struct hid_device *hdev, u16 up,
>  		return -ENOMEM;
>  
>  	cfg->hdev = hdev;
> +	cfg->x2_rgb = quirks && quirks->x2_rgb;

[Severity: High]
This is a pre-existing issue, but does oxp_is_cfg_interface() perform an
unsafe cast of the parent device?

During probe, before oxp_cfg_probe() is called, the driver checks:

	if (!oxp_is_cfg_interface(hdev, quirks))
		return 0;

Looking at oxp_is_cfg_interface(), it uses:

	if (hdev->bus != BUS_USB)
		return false;

	intf = to_usb_interface(hdev->dev.parent);
	return intf->cur_altsetting->desc.bInterfaceNumber ==
	       quirks->cfg_interface_num;

Could a userspace program create a UHID device spoofing the BUS_USB bus type
and target device IDs? If so, the hdev->bus != BUS_USB check would pass, but
the parent device would be a UHID device rather than a USB interface. 

Does unconditionally casting a non-USB parent device to struct
usb_interface lead to an out-of-bounds memory read or kernel panic when
accessing intf->cur_altsetting->desc.bInterfaceNumber? Should this code use
hid_is_usb(hdev) to safely guard the cast?

>  	mutex_init(&cfg->cfg_mutex);
>  	mutex_init(&cfg->rgb_mutex);
>  	spin_lock_init(&cfg->rgb_reply_lock);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260910032115.28669-1-andrei1998@gmail.com?part=14