Hi Guenter, On Mon, 23 Jan 2012 18:49:58 -0800, Guenter Roeck wrote: > When updating vrm, the value range was not limited. This could result in more or > less random vrm values if the value provided by the user was larger than 255. > Fix by limiting the range to 0..255 using the SENSORS_LIMIT macro. > > Cc: Jean Delvare <khali@xxxxxxxxxxxx> > Signed-off-by: Guenter Roeck <guenter.roeck@xxxxxxxxxxxx> > --- > drivers/hwmon/it87.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/drivers/hwmon/it87.c b/drivers/hwmon/it87.c > index 0b204e4..bd8775c 100644 > --- a/drivers/hwmon/it87.c > +++ b/drivers/hwmon/it87.c > @@ -1324,7 +1324,7 @@ static ssize_t store_vrm_reg(struct device *dev, struct device_attribute *attr, > if (kstrtoul(buf, 10, &val) < 0) > return -EINVAL; > > - data->vrm = val; > + data->vrm = SENSORS_LIMIT(val, 0, 255); > > return count; > } To be honest, I don't know what to do with this. You're replacing a silent failure with another. I'd rather return an error if the value doesn't fit. as clamping to 255 will lead to a failure in vid_from_reg() later anyway. On the other hand, we don't validate the VRM value here anyway, it could be in the 0-255 range and still lead to a failure in vid_from_reg(). We could improve this of course, but is it worth it? As documented in Documentation/hwmon/sysfs-interface, changing the vrm value manually should no longer be needed. libsensors does even ignore this attribute completely. And newer hardware tends to no longer use the VID pins at all. Actually hwmon-vid is in a pretty bad shape at the moment at least for all Intel CPUs after Conroe (2007, yeah!) for which we set the VRM to 82 i.e. Pentium II/III. And nobody complained yet as far as I know! So I'm really not sure if it is worth spending time on. It might make more sense to turn all these vrm attributes read-only (and ultimately kill them altogether.) If we really want to let the user force the vrm value if automatic detection failed, it would probably be better done as a module parameter to hwmon-vid. It is certainly less flexible, but at least we don't have to repeat it in every hwmon driver. -- Jean Delvare _______________________________________________ lm-sensors mailing list lm-sensors@xxxxxxxxxxxxxx http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
