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

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: [email protected]
> > > > Closes: https://syzkaller.appspot.com/bug?extid=9ee5f5dc18673d6b2f37
> > > > Tested-by: [email protected]
> > > > Signed-off-by: Edward Adam Davis <[email protected]>
> > > > ---
> > >
> > > 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