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 <[email protected]>
Link: https://lore.kernel.org/all/[email protected]/
Signed-off-by: Zhang Rui <[email protected]>
---
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;
}
--
2.25.1
On Wed, Mar 8, 2023 at 4:12 AM Zhang Rui <[email protected]> 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 <[email protected]>
> Link: https://lore.kernel.org/all/[email protected]/
> Signed-off-by: Zhang Rui <[email protected]>
> ---
> 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!