Messages in this thread | | | From | Pali Rohár <> | Subject | Re: [PATCH v2 1/2] i8k: Autodetect maximal fan speed and fan RPM multiplier | Date | Fri, 19 Dec 2014 19:51:25 +0100 |
| |
On Friday 19 December 2014 19:32:37 Guenter Roeck wrote: > > -static int i8k_fan_mult; > > -static int i8k_pwm_mult; > > -static int i8k_fan_max = I8K_FAN_HIGH; > > +static int i8k_fan_mult[2]; > > +static int i8k_pwm_mult[2]; > > +static int i8k_fan_max[2]; > > The rationale for this change is not explained in the commit > log. > > Do you have any indication that those values would ever be > different for the two fans, ie that you actually need arrays > here ? >
I do not know... But if we decide to use only single value for multiplier and max value which fan to use for autodetection?
> > @@ -271,8 +274,25 @@ static int i8k_get_fan_speed(int fan) > > > > { > > > > struct smm_regs regs = { .eax = I8K_SMM_GET_SPEED, }; > > > > + if (fan < 0 || fan >= ARRAY_SIZE(i8k_fan_mult)) > > + return -EINVAL; > > + > > This range check (and probably others) is still unnecessary. > > Guenter >
No, it is necessary. Function i8k_get_fan_speed is called from ioctl callback with value which comes from userspace. If userspace specify fan out of that array we can get kernel panic.
> > regs.ebx = fan & 0xff; > > > > - return i8k_smm(®s) ? : (regs.eax & 0xffff) * > > i8k_fan_mult; + return i8k_smm(®s) ? : (regs.eax & > > 0xffff) * i8k_fan_mult[fan]; +}
-- Pali Rohár pali.rohar@gmail.com [unhandled content-type:application/pgp-signature] | |