Re: [PATCH v3 3/3] hwmon: (asus_rog_ryujin) Add ROG Ryujin III White Edition

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

Pre-existing issues:
- [High] Missing bounds check on incoming HID report data in raw_event callback, leading to out-of-bounds read.
--

--- Patch [3]: [PATCH v3 3/3] hwmon: (asus_rog_ryujin) Add ROG Ryujin III White Edition ---
commit c24d3645c70fad1763a76ddee1293e1040bce990
Author: Will Smith <[email protected]>
Subject: hwmon: (asus_rog_ryujin) Add ROG Ryujin III White Edition

This commit adds support for the ROG Ryujin III White Edition cooler by
adding its USB product ID to the asus_rog_ryujin driver. It utilizes the
same report layout as the other Ryujin III variants.

Link: https://github.com/aleksamagicka/asus_rog_ryujin-hwmon/pull/10

> diff --git a/drivers/hwmon/asus_rog_ryujin.c b/drivers/hwmon/asus_rog_ryujin.c
> index 79e5b6d6dae0a..702edb8313945 100644
> --- a/drivers/hwmon/asus_rog_ryujin.c
> +++ b/drivers/hwmon/asus_rog_ryujin.c
[ ... ]
> @@ -590,6 +591,8 @@ static const struct hid_device_id rog_ryujin_table[] = {
>  	  .driver_data = (kernel_ulong_t)&rog_ryujin_iii_info },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_ASUS_ROG, USB_PRODUCT_ID_RYUJIN_III_EVA),
>  	  .driver_data = (kernel_ulong_t)&rog_ryujin_iii_info },
> +	{ HID_USB_DEVICE(USB_VENDOR_ID_ASUS_ROG, USB_PRODUCT_ID_RYUJIN_III_WHITE),
> +	  .driver_data = (kernel_ulong_t)&rog_ryujin_iii_info },

[Severity: High]
This is a pre-existing issue, but does adding this new device expose it to an
out-of-bounds read in rog_ryujin_raw_event()?

If the new device sends a short report or has a short report descriptor,
rog_ryujin_raw_event() accesses data buffers without checking the size first:

drivers/hwmon/asus_rog_ryujin.c:rog_ryujin_raw_event() {
	struct rog_ryujin_data *priv = hid_get_drvdata(hdev);

	if (data[0] != RYUJIN_CMD_PREFIX)
		return 0;
...
}

Would a malicious USB device sending a short report trigger a slab
out-of-bounds read here, or when reading larger offsets like
priv->info->fan_speed_offset? Should there be a size validation check
added to the raw event handler?

>  	{ }
>  };

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3