Re: [PATCH] hwmon: valid the data size before reading the sensor data

Subject is supposed to be "hwmon: (driver) Description".

On 8/21/26 22:34, Edward Adam Davis wrote:
> The user-forged sensor data is only 65 bytes long; however, aqc_raw_event()
> fails to handle cases where the sensor data length is smaller than the buffer
> size when reading the data, resulting in [1] during the read process.
> 

The device simulated by syzbot is D5 next, and its control buffer size is
0x329 or 809. I _asked_ earlier if that is the value to check against,
but did not claim that this is actually the case.

We know that the report must be much longer than 65 bytes. D5NEXT_PUMP_OFFSET
is 0x6c = 108, and the field is two bytes long, meaning the report size
must be at least 110 bytes long. What we do not know is its actual length.

> Add a check for the data size, if it less than the buffer size, the sensor
> data read is aborted.
>

Apparently Sashiko is aware that this is wrong - not only is the report size
smaller than 809 bytes, but apparently buffer_size is not even set for all
supported devices.

Please do not submit a patch to fix this problem if you can not test if
the code actually works.

Thanks,
Guenter

> [1]
> BUG: KASAN: slab-out-of-bounds in aqc_raw_event+0x213e/0x25d0 drivers/hwmon/aquacomputer_d5next.c:1327
> Read of size 2 at addr ffff888108aba257 by task swapper/1/0
> Call Trace:
>   get_unaligned_be16 include/linux/unaligned.h:48 [inline]
>   aqc_raw_event drivers/hwmon/aquacomputer_d5next.c:1345 [inline]
>   aqc_raw_event+0x213e/0x25d0 drivers/hwmon/aquacomputer_d5next.c:1327
>   __hid_input_report.constprop.0+0x319/0x470 drivers/hid/hid-core.c:2168
>   hid_irq_in+0x55d/0x710 drivers/hid/usbhid/hid-core.c:287
>   __usb_hcd_giveback_urb+0x38d/0x610 drivers/usb/core/hcd.c:1657
>   usb_hcd_giveback_urb+0x3ca/0x4a0 drivers/usb/core/hcd.c:1741
> 
> Fixes: 0e35f63f7f4e ("hwmon: add driver for Aquacomputer D5 Next")
> Reported-by: [email protected]
> Closes: https://syzkaller.appspot.com/bug?extid=9ee5f5dc18673d6b2f37
> Tested-by: [email protected]
> Signed-off-by: Edward Adam Davis <[email protected]>
> ---
>   drivers/hwmon/aquacomputer_d5next.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/hwmon/aquacomputer_d5next.c b/drivers/hwmon/aquacomputer_d5next.c
> index 1ca70e726298..1cc6c220ffe9 100644
> --- a/drivers/hwmon/aquacomputer_d5next.c
> +++ b/drivers/hwmon/aquacomputer_d5next.c
> @@ -1334,6 +1334,9 @@ static int aqc_raw_event(struct hid_device *hdev, struct hid_report *report, u8
>   
>   	priv = hid_get_drvdata(hdev);
>   
> +	if (size < priv->buffer_size)
> +		return 0;
> +
>   	/* Info provided with every report */
>   	priv->serial_number[0] = get_unaligned_be16(data + priv->serial_number_start_offset);
>   	priv->serial_number[1] = get_unaligned_be16(data + priv->serial_number_start_offset +