[bug report] thermal/x86_pkg_temp_thermal: Add support for handling dynamic tjmax

[bug report] thermal/x86_pkg_temp_thermal: Add support for handling dynamic tjmax

[Dan Carpenter]

Hello Zhang Rui,

The patch 58374a3970a0: "thermal/x86_pkg_temp_thermal: Add support
for handling dynamic tjmax" from Dec 19, 2022, leads to the following
Smatch static checker warning:

	drivers/thermal/intel/x86_pkg_temp_thermal.c:159 sys_set_trip_temp()
	warn: no lower bound on 'temp'

drivers/thermal/intel/x86_pkg_temp_thermal.c
    122 static int
    123 sys_set_trip_temp(struct thermal_zone_device *tzd, int trip, int temp)
    124 {
    125         struct zone_device *zonedev = tzd->devdata;
    126         u32 l, h, mask, shift, intr;
    127         int tj_max, ret;
    128 
    129         tj_max = intel_tcc_get_tjmax(zonedev->cpu);
    130         if (tj_max < 0)
    131                 return tj_max;
    132         tj_max *= 1000;
    133 
    134         if (trip >= MAX_NUMBER_OF_TRIPS || temp >= tj_max)

There is an upper bound on temp, but no lower bound.  The "temp"
variable comes from sysfs via the call tree:

  trip_point_temp_store()
  -> thermal_zone_set_trip()
     -> sys_set_trip_temp()

    135                 return -EINVAL;
    136 
    137         ret = rdmsr_on_cpu(zonedev->cpu, MSR_IA32_PACKAGE_THERM_INTERRUPT,
    138                            &l, &h);
    139         if (ret < 0)
    140                 return ret;
    141 
    142         if (trip) {
    143                 mask = THERM_MASK_THRESHOLD1;
    144                 shift = THERM_SHIFT_THRESHOLD1;
    145                 intr = THERM_INT_THRESHOLD1_ENABLE;
    146         } else {
    147                 mask = THERM_MASK_THRESHOLD0;
    148                 shift = THERM_SHIFT_THRESHOLD0;
    149                 intr = THERM_INT_THRESHOLD0_ENABLE;
    150         }
    151         l &= ~mask;
    152         /*
    153         * When users space sets a trip temperature == 0, which is indication
    154         * that, it is no longer interested in receiving notifications.
    155         */
    156         if (!temp) {
    157                 l &= ~intr;
    158         } else {
--> 159                 l |= (tj_max - temp)/1000 << shift;
    160                 l |= intr;
    161         }
    162 
    163         return wrmsr_on_cpu(zonedev->cpu, MSR_IA32_PACKAGE_THERM_INTERRUPT,
    164                         l, h);
    165 }

regards,
dan carpenter

Re: [bug report] thermal/x86_pkg_temp_thermal: Add support for handling dynamic tjmax

[Zhang, Rui]

Hi, Dan,

thanks for the report.

> The patch 58374a3970a0: "thermal/x86_pkg_temp_thermal: Add support
> for handling dynamic tjmax" from Dec 19, 2022, leads to the following
> Smatch static checker warning:
> 
> 	drivers/thermal/intel/x86_pkg_temp_thermal.c:159
> sys_set_trip_temp()
> 	warn: no lower bound on 'temp'
> 
Actually, this problem always exists. I have sent a patch to fix it.

May I know why it is found only after the recent change?

thanks,
rui

Re: [bug report] thermal/x86_pkg_temp_thermal: Add support for handling dynamic tjmax

[Dan Carpenter]

On Wed, Mar 08, 2023 at 03:15:29AM +0000, Zhang, Rui wrote:
> Hi, Dan,
> 
> thanks for the report.
> 
> > The patch 58374a3970a0: "thermal/x86_pkg_temp_thermal: Add support
> > for handling dynamic tjmax" from Dec 19, 2022, leads to the following
> > Smatch static checker warning:
> > 
> > 	drivers/thermal/intel/x86_pkg_temp_thermal.c:159
> > sys_set_trip_temp()
> > 	warn: no lower bound on 'temp'
> > 
> Actually, this problem always exists. I have sent a patch to fix it.
> 
> May I know why it is found only after the recent change?

No, I just blamed the wrong commit.  There was a recent CVE we were
responding to which forced some Smatch changes.  So that's why the
warning is new.

regards,
dan carpenter