Re: [bug report] hwmon: (nct6775) Add support for weighted fan control

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Dan,

On 09/05/2018 12:57 AM, Dan Carpenter wrote:
Done.  Btw, I saw another that looks legit.

Thanks ...

drivers/hwmon/nct6775.c:1687 nct6775_update_device()
error: buffer overflow 'data->FAN_PULSE_SHIFT' 6 <= 6

drivers/hwmon/nct6775.c
   1667                          data->in[i][2] = nct6775_read_value(data,
   1668                                            data->REG_IN_MINMAX[1][i]);
   1669                  }
   1670
   1671                  /* Measured fan speeds and limits */
   1672                  for (i = 0; i < ARRAY_SIZE(data->rpm); i++) {
                                     ^^^^^^^^^^^^^^^^^^^^^^^^
This is a 7 element array.

   1673                          u16 reg;
   1674
   1675                          if (!(data->has_fan & BIT(i)))
                                       ^^^^^^^^^^^^^^^^^^^^^^

I probably wouldn't have reported this originally because Smatch is bad
at these checks.  (I just haven't gotten around to it yet).  But we do
have fan7pin so it looks like BIT(6) can be set.

   1676                                  continue;
   1677
   1678                          reg = nct6775_read_value(data, data->REG_FAN[i]);
   1679                          data->rpm[i] = data->fan_from_reg(reg,
   1680                                                            data->fan_div[i]);
   1681
   1682                          if (data->has_fan_min & BIT(i))
   1683                                  data->fan_min[i] = nct6775_read_value(data,
   1684                                             data->REG_FAN_MIN[i]);
   1685                          data->fan_pulses[i] =
   1686                            (nct6775_read_value(data, data->REG_FAN_PULSES[i])
   1687                                  >> data->FAN_PULSE_SHIFT[i]) & 0x03;
                                            ^^^^^^^^^^^^^^^^^^^^^^^^
FAN_PULSE_SHIFT is either 5 or 6 elements.


Array size of xxx_REG_FAN_PULSES[] and xxx_FAN_PULSE_SHIFT[] should probably
be NUM_FAN to be on the safe side. There is a slight secondary problem, though:
data->REG_FAN_PULSES[i] should not be read if it is 0. It doesn't matter much -
it results in an unnecessary read from register 0 - but it is still undesirable.

Care to send another patch ?

Thanks,
Guenter

   1688
   1689                          nct6775_select_fan_div(dev, data, i, reg);
   1690                  }
   1691
   1692                  nct6775_update_pwm(dev);
   1693                  nct6775_update_pwm_limits(dev);
   1694

regards,
dan carpenter





[Index of Archives]     [LM Sensors]     [Linux Sound]     [ALSA Users]     [ALSA Devel]     [Linux Audio Users]     [Linux Media]     [Kernel]     [Gimp]     [Yosemite News]     [Linux Media]

  Powered by Linux