Re: [PATCH] hwmon: (gigabyte_waterforce) reject short input reports

[Guenter Roeck]

On Thu, Sep 24, 2026 at 11:11:32PM +0800, Jiale Yao wrote:
> The HID driver raw_event callback runs before HID core validates the
> received report length.  waterforce_raw_event() reads the command bytes
> and then accesses fixed offsets for firmware or status data without any
> length checks.  A truncated report can therefore cause out-of-bounds reads
> and update hwmon state with data beyond the received report.
> 
> Require both command bytes before identifying a report, then check the
> minimum length needed by the firmware and status paths before accessing
> their payloads.  Commit 47669bec44fe ("HID: asus: refactor the two
> workqueues and init sequence") added equivalent raw-event length
> validation to hid-asus.
> 
> Fixes: 42ac68e3d4ba ("hwmon: Add driver for Gigabyte AORUS Waterforce AIO coolers")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Jiale Yao <yaojiale02@xxxxxxx>
> ---
>  drivers/hwmon/gigabyte_waterforce.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/hwmon/gigabyte_waterforce.c b/drivers/hwmon/gigabyte_waterforce.c
> index 4eea05f8b569..9499cb274cbd 100644
> --- a/drivers/hwmon/gigabyte_waterforce.c
> +++ b/drivers/hwmon/gigabyte_waterforce.c
> @@ -257,8 +257,14 @@ static int waterforce_raw_event(struct hid_device *hdev, struct hid_report *repo
>  {
>  	struct waterforce_data *priv = hid_get_drvdata(hdev);
>  
> +	if (size < sizeof(get_status_cmd))
> +		return 0;
> +

The return length must be > FIRMWARE_VER_START_OFFSET_2 in all situations,
so a single check against FIRMWARE_VER_START_OFFSET_2 would be sufficient here.

Guenter

>  	if (data[0] == get_firmware_ver_cmd[0] && data[1] == get_firmware_ver_cmd[1]) {
>  		/* Received a firmware version report */
> +		if (size <= FIRMWARE_VER_START_OFFSET_2)
> +			return 0;
> +
>  		priv->firmware_version =
>  		    data[FIRMWARE_VER_START_OFFSET_1] * 10 + data[FIRMWARE_VER_START_OFFSET_2];
>  
> @@ -269,6 +275,8 @@ static int waterforce_raw_event(struct hid_device *hdev, struct hid_report *repo
>  
>  	if (data[0] != get_status_cmd[0] || data[1] != get_status_cmd[1])
>  		return 0;
> +	if (size <= WATERFORCE_TEMP_SENSOR)
> +		return 0;
>  
>  	priv->temp_input[0] = data[WATERFORCE_TEMP_SENSOR] * 1000;
>  	priv->speed_input[0] = get_unaligned_le16(data + WATERFORCE_FAN_SPEED);
> -- 
> 2.34.1
>