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