[PATCH v4] hwmon: (aquacomputer_d5next) valid the data size before reading the sensor data

[PATCH v4] hwmon: (aquacomputer_d5next) valid the data size before reading the sensor data

[Edward Adam Davis]

The user-forged sensor data is only 65 bytes long; however, aqc_raw_event()
fails to handle cases where the sensor data length is too small when reading
the data, resulting in [1] during the read process.

Add a data size check, if the size is less than that required for the
specific data item to be read, abort the sensor data read operation.

[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: syzbot+9ee5f5dc18673d6b2f37@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=9ee5f5dc18673d6b2f37
Tested-by: syzbot+9ee5f5dc18673d6b2f37@syzkaller.appspotmail.com
Signed-off-by: Edward Adam Davis <eadavis@qq.com>
---
v1 -> v2: change to check the data item and update comments
v2 -> v3: check all sub items and update subject
v3 -> v4: add speed and flow check

 drivers/hwmon/aquacomputer_d5next.c | 104 ++++++++++++++++++++++++++++
 1 file changed, 104 insertions(+)

diff --git a/drivers/hwmon/aquacomputer_d5next.c b/drivers/hwmon/aquacomputer_d5next.c
index 1ca70e726298..89c9fc0c77e5 100644
--- a/drivers/hwmon/aquacomputer_d5next.c
+++ b/drivers/hwmon/aquacomputer_d5next.c
@@ -1324,6 +1324,107 @@ static const struct hwmon_chip_info aqc_chip_info = {
 	.info = aqc_info,
 };
 
+/* aqc_raw_data_valid()
+ * Does not support special-case sensor readings data size check
+ */
+static bool aqc_raw_data_valid(struct aqc_data *priv, int size)
+{
+	int off, fan_off, i;
+	char *msg;
+
+	if (!priv)
+		return false;
+
+	/* +1 for get_unaligned_be16(), it reads 2 bytes */
+	off = priv->serial_number_start_offset + SERIAL_PART_OFFSET + 1;
+	if (off >= size) {
+		msg = "serial number start offset";
+		goto invalid;
+	}
+
+	off = priv->firmware_version_offset + 1;
+	if (off >= size) {
+		msg = "firmware version offset";
+		goto invalid;
+	}
+
+	/* Physical temperature sensor readings data size check*/
+	for (i = 0; i < priv->num_temp_sensors; i++) {
+		off = priv->temp_sensor_start_offset + i * AQC_SENSOR_SIZE + 1;
+
+		if (off >= size) {
+			msg = "temp sensor start offset";
+			goto invalid;
+		}
+	}
+
+	/* Virtual temperature sensor readings data size check*/
+	for (i = 0; i < priv->num_virtual_temp_sensors; i++) {
+		off = priv->virtual_temp_sensor_start_offset +
+		      i * AQC_SENSOR_SIZE + 1;
+
+		if (off >= size) {
+			msg = "virtual temp sensor start offset";
+			goto invalid;
+		}
+	}
+
+	/* Fan speed and related readings data size check */
+	if (!priv->fan_structure)
+		goto flow;
+
+	for (i = 0; i < priv->num_fans; i++) {
+		fan_off = priv->fan_sensor_offsets[i] + 1;
+		off = fan_off + priv->fan_structure->speed;
+		if (off >= size) {
+			msg = "fan speed offset";
+			goto invalid;
+		}
+
+		off = fan_off + priv->fan_structure->power;
+		if (off >= size) {
+			msg = "fan power offset";
+			goto invalid;
+		}
+
+		off = fan_off + priv->fan_structure->voltage;
+		if (off >= size) {
+			msg = "fan voltage offset";
+			goto invalid;
+		}
+
+		off = fan_off + priv->fan_structure->curr;
+		if (off >= size) {
+			msg = "fan curr offset";
+			goto invalid;
+		}
+	}
+
+flow:
+	/* Flow sensor readings data size check */
+	for (i = 0; i < priv->num_flow_sensors; i++) {
+		off = priv->flow_sensors_start_offset + i * AQC_SENSOR_SIZE + 1;
+		if (off >= size) {
+			msg = "flow sensors start offset";
+			goto invalid;
+		}
+	}
+
+	if (priv->power_cycle_count_offset != 0) {
+		off = priv->power_cycle_count_offset + 3;
+		if (off >= size) {
+			msg = "power cycle count offset";
+			goto invalid;
+		}
+	}
+
+	return true;
+invalid:
+	pr_debug("data size (%d) is less than the %s, %s\n",
+		 size, msg, __func__);
+	return false;
+}
+
 static int aqc_raw_event(struct hid_device *hdev, struct hid_report *report, u8 *data, int size)
 {
 	int i, j, sensor_value;
@@ -1334,6 +1435,9 @@ static int aqc_raw_event(struct hid_device *hdev, struct hid_report *report, u8
 
 	priv = hid_get_drvdata(hdev);
 
+	if (!aqc_raw_data_valid(priv, 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 +
-- 
2.43.0

Re: [PATCH v4] hwmon: (aquacomputer_d5next) valid the data size before reading the sensor data

[Greg KH]

On Sat, Aug 22, 2026 at 07:34:35PM +0800, 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 too small when reading
> the data, resulting in [1] during the read process.

What is "user-forged sensor data"?

> 
> Add a data size check, if the size is less than that required for the
> specific data item to be read, abort the sensor data read operation.

This patch does much more than that.

> 
> [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: syzbot+9ee5f5dc18673d6b2f37@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=9ee5f5dc18673d6b2f37
> Tested-by: syzbot+9ee5f5dc18673d6b2f37@syzkaller.appspotmail.com
> Signed-off-by: Edward Adam Davis <eadavis@qq.com>
> ---

Was the Assisted-by: tag forgotten?


> v1 -> v2: change to check the data item and update comments
> v2 -> v3: check all sub items and update subject
> v3 -> v4: add speed and flow check
> 
>  drivers/hwmon/aquacomputer_d5next.c | 104 ++++++++++++++++++++++++++++
>  1 file changed, 104 insertions(+)
> 
> diff --git a/drivers/hwmon/aquacomputer_d5next.c b/drivers/hwmon/aquacomputer_d5next.c
> index 1ca70e726298..89c9fc0c77e5 100644
> --- a/drivers/hwmon/aquacomputer_d5next.c
> +++ b/drivers/hwmon/aquacomputer_d5next.c
> @@ -1324,6 +1324,107 @@ static const struct hwmon_chip_info aqc_chip_info = {
>  	.info = aqc_info,
>  };
>  
> +/* aqc_raw_data_valid()
> + * Does not support special-case sensor readings data size check

Why not?

> + */
> +static bool aqc_raw_data_valid(struct aqc_data *priv, int size)
> +{
> +	int off, fan_off, i;
> +	char *msg;
> +
> +	if (!priv)
> +		return false;
> +
> +	/* +1 for get_unaligned_be16(), it reads 2 bytes */
> +	off = priv->serial_number_start_offset + SERIAL_PART_OFFSET + 1;
> +	if (off >= size) {
> +		msg = "serial number start offset";

That's a lot of debugging code being added, why?  Who is going to use
that?



> +		goto invalid;
> +	}
> +
> +	off = priv->firmware_version_offset + 1;
> +	if (off >= size) {
> +		msg = "firmware version offset";
> +		goto invalid;
> +	}
> +
> +	/* Physical temperature sensor readings data size check*/
> +	for (i = 0; i < priv->num_temp_sensors; i++) {
> +		off = priv->temp_sensor_start_offset + i * AQC_SENSOR_SIZE + 1;
> +
> +		if (off >= size) {
> +			msg = "temp sensor start offset";
> +			goto invalid;
> +		}
> +	}
> +
> +	/* Virtual temperature sensor readings data size check*/
> +	for (i = 0; i < priv->num_virtual_temp_sensors; i++) {
> +		off = priv->virtual_temp_sensor_start_offset +
> +		      i * AQC_SENSOR_SIZE + 1;
> +
> +		if (off >= size) {
> +			msg = "virtual temp sensor start offset";
> +			goto invalid;
> +		}
> +	}
> +
> +	/* Fan speed and related readings data size check */
> +	if (!priv->fan_structure)
> +		goto flow;
> +
> +	for (i = 0; i < priv->num_fans; i++) {
> +		fan_off = priv->fan_sensor_offsets[i] + 1;
> +		off = fan_off + priv->fan_structure->speed;
> +		if (off >= size) {
> +			msg = "fan speed offset";
> +			goto invalid;
> +		}
> +
> +		off = fan_off + priv->fan_structure->power;
> +		if (off >= size) {
> +			msg = "fan power offset";
> +			goto invalid;
> +		}
> +
> +		off = fan_off + priv->fan_structure->voltage;
> +		if (off >= size) {
> +			msg = "fan voltage offset";
> +			goto invalid;
> +		}
> +
> +		off = fan_off + priv->fan_structure->curr;
> +		if (off >= size) {
> +			msg = "fan curr offset";
> +			goto invalid;
> +		}
> +	}
> +
> +flow:
> +	/* Flow sensor readings data size check */
> +	for (i = 0; i < priv->num_flow_sensors; i++) {
> +		off = priv->flow_sensors_start_offset + i * AQC_SENSOR_SIZE + 1;
> +		if (off >= size) {
> +			msg = "flow sensors start offset";
> +			goto invalid;
> +		}
> +	}
> +
> +	if (priv->power_cycle_count_offset != 0) {
> +		off = priv->power_cycle_count_offset + 3;
> +		if (off >= size) {
> +			msg = "power cycle count offset";
> +			goto invalid;
> +		}
> +	}
> +
> +	return true;
> +invalid:
> +	pr_debug("data size (%d) is less than the %s, %s\n",
> +		 size, msg, __func__);

drivers should always use dev_dbg().

also pr_debug() already has __func__ in it.  Please fix your LLM.

thanks,

greg k-h

Re: [PATCH v4] hwmon: (aquacomputer_d5next) valid the data size before reading the sensor data

[Edward Adam Davis]

On Sat, 22 Aug 2026 13:55:35 +0200, Greg KH wrote:
> On Sat, Aug 22, 2026 at 07:34:35PM +0800, 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 too small when reading
> > the data, resulting in [1] during the read process.
> 
> What is "user-forged sensor data"?
It is the data constructed within the reproducer.
> 
> > 
> > Add a data size check, if the size is less than that required for the
> > specific data item to be read, abort the sensor data read operation.
> 
> This patch does much more than that.
Got it, I will write more comments.
> 
> > 
> > [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: syzbot+9ee5f5dc18673d6b2f37@syzkaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=9ee5f5dc18673d6b2f37
> > Tested-by: syzbot+9ee5f5dc18673d6b2f37@syzkaller.appspotmail.com
> > Signed-off-by: Edward Adam Davis <eadavis@qq.com>
> > ---
> 
> Was the Assisted-by: tag forgotten?
No one has assisted me so far; also, I haven't used this type of tag before.
> 
> 
> > v1 -> v2: change to check the data item and update comments
> > v2 -> v3: check all sub items and update subject
> > v3 -> v4: add speed and flow check
> > 
> >  drivers/hwmon/aquacomputer_d5next.c | 104 ++++++++++++++++++++++++++++
> >  1 file changed, 104 insertions(+)
> > 
> > diff --git a/drivers/hwmon/aquacomputer_d5next.c b/drivers/hwmon/aquacomputer_d5next.c
> > index 1ca70e726298..89c9fc0c77e5 100644
> > --- a/drivers/hwmon/aquacomputer_d5next.c
> > +++ b/drivers/hwmon/aquacomputer_d5next.c
> > @@ -1324,6 +1324,107 @@ static const struct hwmon_chip_info aqc_chip_info = {
> >  	.info = aqc_info,
> >  };
> >  
> > +/* aqc_raw_data_valid()
> > + * Does not support special-case sensor readings data size check
> 
> Why not?
It would be more appropriate for the maintainers of these sensors to add
the relevant checks.
> 
> > + */
> > +static bool aqc_raw_data_valid(struct aqc_data *priv, int size)
> > +{
> > +	int off, fan_off, i;
> > +	char *msg;
> > +
> > +	if (!priv)
> > +		return false;
> > +
> > +	/* +1 for get_unaligned_be16(), it reads 2 bytes */
> > +	off = priv->serial_number_start_offset + SERIAL_PART_OFFSET + 1;
> > +	if (off >= size) {
> > +		msg = "serial number start offset";
> 
> That's a lot of debugging code being added, why?  Who is going to use
> that?
I am not certain that these debug messages are definitely useful;
I simply included them because I felt they might be helpful.
> 
> 
> 
> > +		goto invalid;
> > +	}
> > +
> > +	off = priv->firmware_version_offset + 1;
> > +	if (off >= size) {
> > +		msg = "firmware version offset";
> > +		goto invalid;
> > +	}
> > +
> > +	/* Physical temperature sensor readings data size check*/
> > +	for (i = 0; i < priv->num_temp_sensors; i++) {
> > +		off = priv->temp_sensor_start_offset + i * AQC_SENSOR_SIZE + 1;
> > +
> > +		if (off >= size) {
> > +			msg = "temp sensor start offset";
> > +			goto invalid;
> > +		}
> > +	}
> > +
> > +	/* Virtual temperature sensor readings data size check*/
> > +	for (i = 0; i < priv->num_virtual_temp_sensors; i++) {
> > +		off = priv->virtual_temp_sensor_start_offset +
> > +		      i * AQC_SENSOR_SIZE + 1;
> > +
> > +		if (off >= size) {
> > +			msg = "virtual temp sensor start offset";
> > +			goto invalid;
> > +		}
> > +	}
> > +
> > +	/* Fan speed and related readings data size check */
> > +	if (!priv->fan_structure)
> > +		goto flow;
> > +
> > +	for (i = 0; i < priv->num_fans; i++) {
> > +		fan_off = priv->fan_sensor_offsets[i] + 1;
> > +		off = fan_off + priv->fan_structure->speed;
> > +		if (off >= size) {
> > +			msg = "fan speed offset";
> > +			goto invalid;
> > +		}
> > +
> > +		off = fan_off + priv->fan_structure->power;
> > +		if (off >= size) {
> > +			msg = "fan power offset";
> > +			goto invalid;
> > +		}
> > +
> > +		off = fan_off + priv->fan_structure->voltage;
> > +		if (off >= size) {
> > +			msg = "fan voltage offset";
> > +			goto invalid;
> > +		}
> > +
> > +		off = fan_off + priv->fan_structure->curr;
> > +		if (off >= size) {
> > +			msg = "fan curr offset";
> > +			goto invalid;
> > +		}
> > +	}
> > +
> > +flow:
> > +	/* Flow sensor readings data size check */
> > +	for (i = 0; i < priv->num_flow_sensors; i++) {
> > +		off = priv->flow_sensors_start_offset + i * AQC_SENSOR_SIZE + 1;
> > +		if (off >= size) {
> > +			msg = "flow sensors start offset";
> > +			goto invalid;
> > +		}
> > +	}
> > +
> > +	if (priv->power_cycle_count_offset != 0) {
> > +		off = priv->power_cycle_count_offset + 3;
> > +		if (off >= size) {
> > +			msg = "power cycle count offset";
> > +			goto invalid;
> > +		}
> > +	}
> > +
> > +	return true;
> > +invalid:
> > +	pr_debug("data size (%d) is less than the %s, %s\n",
> > +		 size, msg, __func__);
> 
> drivers should always use dev_dbg().
> 
> also pr_debug() already has __func__ in it.  Please fix your LLM.
Got it.

BR,
Edward

Re: [PATCH v4] hwmon: (aquacomputer_d5next) valid the data size before reading the sensor data

[Greg KH]

On Sat, Aug 22, 2026 at 08:23:35PM +0800, Edward Adam Davis wrote:
> On Sat, 22 Aug 2026 13:55:35 +0200, Greg KH wrote:
> > On Sat, Aug 22, 2026 at 07:34:35PM +0800, 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 too small when reading
> > > the data, resulting in [1] during the read process.
> > 
> > What is "user-forged sensor data"?
> It is the data constructed within the reproducer.

What "reproducer"?

> > > Fixes: 0e35f63f7f4e ("hwmon: add driver for Aquacomputer D5 Next")
> > > Reported-by: syzbot+9ee5f5dc18673d6b2f37@syzkaller.appspotmail.com
> > > Closes: https://syzkaller.appspot.com/bug?extid=9ee5f5dc18673d6b2f37
> > > Tested-by: syzbot+9ee5f5dc18673d6b2f37@syzkaller.appspotmail.com
> > > Signed-off-by: Edward Adam Davis <eadavis@qq.com>
> > > ---
> > 
> > Was the Assisted-by: tag forgotten?
> No one has assisted me so far; also, I haven't used this type of tag before.

No LLM was used?

> > > +/* aqc_raw_data_valid()
> > > + * Does not support special-case sensor readings data size check
> > 
> > Why not?
> It would be more appropriate for the maintainers of these sensors to add
> the relevant checks.

Why?  Shouldn't that be covered here?

> > > + */
> > > +static bool aqc_raw_data_valid(struct aqc_data *priv, int size)
> > > +{
> > > +	int off, fan_off, i;
> > > +	char *msg;
> > > +
> > > +	if (!priv)
> > > +		return false;
> > > +
> > > +	/* +1 for get_unaligned_be16(), it reads 2 bytes */
> > > +	off = priv->serial_number_start_offset + SERIAL_PART_OFFSET + 1;
> > > +	if (off >= size) {
> > > +		msg = "serial number start offset";
> > 
> > That's a lot of debugging code being added, why?  Who is going to use
> > that?
> I am not certain that these debug messages are definitely useful;
> I simply included them because I felt they might be helpful.

If you didn't use them when debugging this code, why would it be needed
by anyone else?

thanks,

greg k-h

Re: [PATCH v4] hwmon: (aquacomputer_d5next) valid the data size before reading the sensor data

[Edward Adam Davis]

On Sat, 22 Aug 2026 14:45:27 +0200, Greg KH wrote:
> On Sat, Aug 22, 2026 at 08:23:35PM +0800, Edward Adam Davis wrote:
> > On Sat, 22 Aug 2026 13:55:35 +0200, Greg KH wrote:
> > > On Sat, Aug 22, 2026 at 07:34:35PM +0800, 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 too small when reading
> > > > the data, resulting in [1] during the read process.
> > >
> > > What is "user-forged sensor data"?
> > It is the data constructed within the reproducer.
> 
> What "reproducer"?
The complete reproducer:
https://syzkaller.appspot.com/x/repro.c?x=15a22549580000

Code for constructing the data:
   memcpy((void*)0x200000000500, 
           "\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"
           "\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"
           "\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"
           "\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00",
           65);
    syz_usb_ep_write(/*fd=*/r[0], /*ep=*/0x81, /*len=*/0x41,
                     /*data=*/0x200000000500);
> 
> > > > Fixes: 0e35f63f7f4e ("hwmon: add driver for Aquacomputer D5 Next")
> > > > Reported-by: syzbot+9ee5f5dc18673d6b2f37@syzkaller.appspotmail.com
> > > > Closes: https://syzkaller.appspot.com/bug?extid=9ee5f5dc18673d6b2f37
> > > > Tested-by: syzbot+9ee5f5dc18673d6b2f37@syzkaller.appspotmail.com
> > > > Signed-off-by: Edward Adam Davis <eadavis@qq.com>
> > > > ---
> > >
> > > Was the Assisted-by: tag forgotten?
> > No one has assisted me so far; also, I haven't used this type of tag before.
> 
> No LLM was used?
No.
> 
> > > > +/* aqc_raw_data_valid()
> > > > + * Does not support special-case sensor readings data size check
> > >
> > > Why not?
> > It would be more appropriate for the maintainers of these sensors to add
> > the relevant checks.
> 
> Why?  Shouldn't that be covered here?
Perhaps I didn't express myself clearly enough; it should be included here,
but it would be best if someone familiar with Special-case sensors followed
up to add the details.
> 
> > > > + */
> > > > +static bool aqc_raw_data_valid(struct aqc_data *priv, int size)
> > > > +{
> > > > +	int off, fan_off, i;
> > > > +	char *msg;
> > > > +
> > > > +	if (!priv)
> > > > +		return false;
> > > > +
> > > > +	/* +1 for get_unaligned_be16(), it reads 2 bytes */
> > > > +	off = priv->serial_number_start_offset + SERIAL_PART_OFFSET + 1;
> > > > +	if (off >= size) {
> > > > +		msg = "serial number start offset";
> > >
> > > That's a lot of debugging code being added, why?  Who is going to use
> > > that?
> > I am not certain that these debug messages are definitely useful;
> > I simply included them because I felt they might be helpful.
> 
> If you didn't use them when debugging this code, why would it be needed
> by anyone else?
That makes perfect sense; I will remove them.

BR,
Edward

[PATCH v5] hwmon: (aquacomputer_d5next) valid the data size before reading the sensor data

[Edward Adam Davis]

The user-forged sensor data is only 65 bytes long; however, aqc_raw_event()
fails to handle cases where the sensor data length is too small when reading
the data, resulting in [1] during the read process.

Add a data size check, if the size is less than that required for the
specific data item(includes: Physical/Virtual temperature sensor, Flow
sensor, Fan speed and related, etc.) to be read, abort the sensor data
read operation.

[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: syzbot+9ee5f5dc18673d6b2f37@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=9ee5f5dc18673d6b2f37
Tested-by: syzbot+9ee5f5dc18673d6b2f37@syzkaller.appspotmail.com
Signed-off-by: Edward Adam Davis <eadavis@qq.com>
---
v1 -> v2: change to check the data item and update comments
v2 -> v3: check all sub items and update subject
v3 -> v4: add speed and flow check
v4 -> v5: remove dbg msg and update comments

 drivers/hwmon/aquacomputer_d5next.c | 81 +++++++++++++++++++++++++++++
 1 file changed, 81 insertions(+)

diff --git a/drivers/hwmon/aquacomputer_d5next.c b/drivers/hwmon/aquacomputer_d5next.c
index 1ca70e726298..352b227c3015 100644
--- a/drivers/hwmon/aquacomputer_d5next.c
+++ b/drivers/hwmon/aquacomputer_d5next.c
@@ -1324,6 +1324,84 @@ static const struct hwmon_chip_info aqc_chip_info = {
 	.info = aqc_info,
 };
 
+/* aqc_raw_data_valid()
+ * Does not support special-case sensor readings data size check
+ */
+static bool aqc_raw_data_valid(struct aqc_data *priv, int size)
+{
+	int off, fan_off, i;
+
+	if (!priv)
+		return false;
+
+	/* +1 for get_unaligned_be16(), it reads 2 bytes */
+	off = priv->serial_number_start_offset + SERIAL_PART_OFFSET + 1;
+	if (off >= size)
+		goto invalid;
+
+	off = priv->firmware_version_offset + 1;
+	if (off >= size)
+		goto invalid;
+
+	/* Physical temperature sensor */
+	for (i = 0; i < priv->num_temp_sensors; i++) {
+		off = priv->temp_sensor_start_offset + i * AQC_SENSOR_SIZE + 1;
+
+		if (off >= size)
+			goto invalid;
+	}
+
+	/* Virtual temperature sensor */
+	for (i = 0; i < priv->num_virtual_temp_sensors; i++) {
+		off = priv->virtual_temp_sensor_start_offset +
+		      i * AQC_SENSOR_SIZE + 1;
+
+		if (off >= size)
+			goto invalid;
+	}
+
+	/* Fan speed and related */
+	if (!priv->fan_structure)
+		goto flow;
+
+	for (i = 0; i < priv->num_fans; i++) {
+		fan_off = priv->fan_sensor_offsets[i] + 1;
+		off = fan_off + priv->fan_structure->speed;
+		if (off >= size)
+			goto invalid;
+
+		off = fan_off + priv->fan_structure->power;
+		if (off >= size)
+			goto invalid;
+
+		off = fan_off + priv->fan_structure->voltage;
+		if (off >= size)
+			goto invalid;
+
+		off = fan_off + priv->fan_structure->curr;
+		if (off >= size)
+			goto invalid;
+	}
+
+flow:
+	/* Flow sensor */
+	for (i = 0; i < priv->num_flow_sensors; i++) {
+		off = priv->flow_sensors_start_offset + i * AQC_SENSOR_SIZE + 1;
+		if (off >= size)
+			goto invalid;
+	}
+
+	if (priv->power_cycle_count_offset != 0) {
+		off = priv->power_cycle_count_offset + 3;
+		if (off >= size)
+			goto invalid;
+	}
+
+	return true;
+invalid:
+	return false;
+}
+
 static int aqc_raw_event(struct hid_device *hdev, struct hid_report *report, u8 *data, int size)
 {
 	int i, j, sensor_value;
@@ -1334,6 +1412,9 @@ static int aqc_raw_event(struct hid_device *hdev, struct hid_report *report, u8
 
 	priv = hid_get_drvdata(hdev);
 
+	if (!aqc_raw_data_valid(priv, 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 +
-- 
2.43.0

Re: [PATCH v5] hwmon: (aquacomputer_d5next) valid the data size before reading the sensor data

[Guenter Roeck]

On 8/22/26 07:05, 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 too small when reading
> the data, resulting in [1] during the read process.
> 
> Add a data size check, if the size is less than that required for the
> specific data item(includes: Physical/Virtual temperature sensor, Flow
> sensor, Fan speed and related, etc.) to be read, abort the sensor data
> read operation.
> 
> [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: syzbot+9ee5f5dc18673d6b2f37@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=9ee5f5dc18673d6b2f37
> Tested-by: syzbot+9ee5f5dc18673d6b2f37@syzkaller.appspotmail.com
> Signed-off-by: Edward Adam Davis <eadavis@qq.com>
> ---
> v1 -> v2: change to check the data item and update comments
> v2 -> v3: check all sub items and update subject
> v3 -> v4: add speed and flow check
> v4 -> v5: remove dbg msg and update comments
> 

Your code was and remains way too complicated.

Guenter