Message ID | 20230308030711.4362-1-rui.zhang@intel.com (mailing list archive) |
---|---|
State | Mainlined, archived |
Headers | show |
Series | thermal/x86_pkg_temp_thermal: Add lower bound check for sysfs input | expand |
On Wed, Mar 8, 2023 at 4:12 AM Zhang Rui <rui.zhang@intel.com> wrote: > > When setting a trip point temperature from sysfs, there is an upper > bound check on the user input, but no lower bound check. > > As hardware register has 7 bits for a trip point temperature, the offset > to tj_max of the input temperature must be equal to/less than 0x7f. > Or else, > 1. bogus temperature is updated into the trip temperature bits. > 2. the upper bits of the register can be polluted. > > For example, > $ rdmsr 0x1b2 > 2000003 > $ echo -180000 > /sys/class/thermal/thermal_zone1/trip_point_1_temp > $ rdmsr 0x1b2 > 3980003 > > Not only the trip point temp is set to 76C on this platform (tj_max is > 100), the Power Notification (Bit 24) is also enabled erronously. > > Fix the problem by adding lower bound check for sysfs input. > > Reported-by: Dan Carpenter <error27@gmail.com> > Link: https://lore.kernel.org/all/add7a378-4d50-4ba1-81d3-a0c17db25a0b@kili.mountain/ > Signed-off-by: Zhang Rui <rui.zhang@intel.com> > --- > Note that I didn't add a Fixes tag here because this problem always > exists, and even the location of this file has been changed. > > Note that I didn't cc -stable because this patch doesn't apply to older > kernels due to recent changes. If Rafael thinks this is a stable > material, we can rebase this fix on previous kernel after this patch > hits upstream. > --- > drivers/thermal/intel/x86_pkg_temp_thermal.c | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/drivers/thermal/intel/x86_pkg_temp_thermal.c b/drivers/thermal/intel/x86_pkg_temp_thermal.c > index 1c2de84742df..128ee02596ed 100644 > --- a/drivers/thermal/intel/x86_pkg_temp_thermal.c > +++ b/drivers/thermal/intel/x86_pkg_temp_thermal.c > @@ -124,14 +124,16 @@ sys_set_trip_temp(struct thermal_zone_device *tzd, int trip, int temp) > { > struct zone_device *zonedev = tzd->devdata; > u32 l, h, mask, shift, intr; > - int tj_max, ret; > + int tj_max, val, ret; > > tj_max = intel_tcc_get_tjmax(zonedev->cpu); > if (tj_max < 0) > return tj_max; > tj_max *= 1000; > > - if (trip >= MAX_NUMBER_OF_TRIPS || temp >= tj_max) > + val = (tj_max - temp)/1000; > + > + if (trip >= MAX_NUMBER_OF_TRIPS || val < 0 || val > 0x7f) > return -EINVAL; > > ret = rdmsr_on_cpu(zonedev->cpu, MSR_IA32_PACKAGE_THERM_INTERRUPT, > @@ -156,7 +158,7 @@ sys_set_trip_temp(struct thermal_zone_device *tzd, int trip, int temp) > if (!temp) { > l &= ~intr; > } else { > - l |= (tj_max - temp)/1000 << shift; > + l |= val << shift; > l |= intr; > } > > -- Applied as 6.4 material, thanks!
diff --git a/drivers/thermal/intel/x86_pkg_temp_thermal.c b/drivers/thermal/intel/x86_pkg_temp_thermal.c index 1c2de84742df..128ee02596ed 100644 --- a/drivers/thermal/intel/x86_pkg_temp_thermal.c +++ b/drivers/thermal/intel/x86_pkg_temp_thermal.c @@ -124,14 +124,16 @@ sys_set_trip_temp(struct thermal_zone_device *tzd, int trip, int temp) { struct zone_device *zonedev = tzd->devdata; u32 l, h, mask, shift, intr; - int tj_max, ret; + int tj_max, val, ret; tj_max = intel_tcc_get_tjmax(zonedev->cpu); if (tj_max < 0) return tj_max; tj_max *= 1000; - if (trip >= MAX_NUMBER_OF_TRIPS || temp >= tj_max) + val = (tj_max - temp)/1000; + + if (trip >= MAX_NUMBER_OF_TRIPS || val < 0 || val > 0x7f) return -EINVAL; ret = rdmsr_on_cpu(zonedev->cpu, MSR_IA32_PACKAGE_THERM_INTERRUPT, @@ -156,7 +158,7 @@ sys_set_trip_temp(struct thermal_zone_device *tzd, int trip, int temp) if (!temp) { l &= ~intr; } else { - l |= (tj_max - temp)/1000 << shift; + l |= val << shift; l |= intr; }
When setting a trip point temperature from sysfs, there is an upper bound check on the user input, but no lower bound check. As hardware register has 7 bits for a trip point temperature, the offset to tj_max of the input temperature must be equal to/less than 0x7f. Or else, 1. bogus temperature is updated into the trip temperature bits. 2. the upper bits of the register can be polluted. For example, $ rdmsr 0x1b2 2000003 $ echo -180000 > /sys/class/thermal/thermal_zone1/trip_point_1_temp $ rdmsr 0x1b2 3980003 Not only the trip point temp is set to 76C on this platform (tj_max is 100), the Power Notification (Bit 24) is also enabled erronously. Fix the problem by adding lower bound check for sysfs input. Reported-by: Dan Carpenter <error27@gmail.com> Link: https://lore.kernel.org/all/add7a378-4d50-4ba1-81d3-a0c17db25a0b@kili.mountain/ Signed-off-by: Zhang Rui <rui.zhang@intel.com> --- Note that I didn't add a Fixes tag here because this problem always exists, and even the location of this file has been changed. Note that I didn't cc -stable because this patch doesn't apply to older kernels due to recent changes. If Rafael thinks this is a stable material, we can rebase this fix on previous kernel after this patch hits upstream. --- drivers/thermal/intel/x86_pkg_temp_thermal.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)