linux-kernel - Re: [PATCH] thermal: Increase maximum number of trip points

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAJZ5v0iU374GLjVsYSqCThjX0+1ZAxGzaHyGaJbYHpdNnknjpw@mail.gmail.com>
Date:   Fri, 30 Sep 2022 19:52:07 +0200
From:   "Rafael J. Wysocki" <rafael@...nel.org>
To:     Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Sumeet Pawnikar <sumeet.r.pawnikar@...el.com>,
        daniel.lezcano@...aro.org
Cc:     srinivas.pandruvada@...ux.intel.com, rui.zhang@...el.com,
        linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] thermal: Increase maximum number of trip points

On Wed, Sep 28, 2022 at 12:52 PM Andy Shevchenko
<andriy.shevchenko@...ux.intel.com> wrote:
>
> On Tue, Sep 27, 2022 at 09:17:09PM +0530, Sumeet Pawnikar wrote:
> > On one of the Chrome system, if we define more than 12 trip points,
> > probe for thermal sensor fails with
> > "int3403 thermal: probe of INTC1046:03 failed with error -22"
> > and throws an error as
> > "thermal_sys: Error: Incorrect number of thermal trips".
> >
> > The thermal_zone_device_register() interface needs maximum
> > number of trip points supported in a zone as an argument.
> > This number can't exceed THERMAL_MAX_TRIPS, which is currently
> > set to 12. To address this issue, THERMAL_MAX_TRIPS value
> > has to be increased.
> >
> > This interface also has an argument to specify a mask of trips
> > which are writable. This mask is defined as an int.
> > This mask sets the ceiling for increasing maximum number of
> > supported trips. With the current implementation, maximum number
> > of trips can be supported is 31.
> >
> > Also, THERMAL_MAX_TRIPS macro is used in one place only.
> > So, remove THERMAL_MAX_TRIPS macro and compare num_trips
> > directly with using a macro BITS_PER_TYPE(int)-1.
>
> FWIW,
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>

Applied as 6.1 material, thanks!

Daniel, please let me know if you have any concerns.

> > Signed-off-by: Sumeet Pawnikar <sumeet.r.pawnikar@...el.com>
> > ---
> >  drivers/thermal/thermal_core.c | 15 ++++++++++++++-
> >  include/linux/thermal.h        |  2 --
> >  2 files changed, 14 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> > index 50d50cec7774..589dd82fe10c 100644
> > --- a/drivers/thermal/thermal_core.c
> > +++ b/drivers/thermal/thermal_core.c
> > @@ -1212,7 +1212,20 @@ thermal_zone_device_register_with_trips(const char *type, struct thermal_trip *t
> >               return ERR_PTR(-EINVAL);
> >       }
> >
> > -     if (num_trips > THERMAL_MAX_TRIPS || num_trips < 0 || mask >> num_trips) {
> > +     /*
> > +      * Max trip count can't exceed 31 as the "mask >> num_trips" condition.
> > +      * For example, shifting by 32 will result in compiler warning:
> > +      * warning: right shift count >= width of type [-Wshift-count- overflow]
> > +      *
> > +      * Also "mask >> num_trips" will always be true with 32 bit shift.
> > +      * E.g. mask = 0x80000000 for trip id 31 to be RW. Then
> > +      * mask >> 32 = 0x80000000
> > +      * This will result in failure for the below condition.
> > +      *
> > +      * Check will be true when the bit 31 of the mask is set.
> > +      * 32 bit shift will cause overflow of 4 byte integer.
> > +      */
> > +     if (num_trips > (BITS_PER_TYPE(int) - 1) || num_trips < 0 || mask >> num_trips) {
> >               pr_err("Incorrect number of thermal trips\n");
> >               return ERR_PTR(-EINVAL);
> >       }
> > diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> > index 1386c713885d..c05f5c78a0f2 100644
> > --- a/include/linux/thermal.h
> > +++ b/include/linux/thermal.h
> > @@ -17,8 +17,6 @@
> >  #include <linux/workqueue.h>
> >  #include <uapi/linux/thermal.h>
> >
> > -#define THERMAL_MAX_TRIPS    12
> > -
> >  /* invalid cooling state */
> >  #define THERMAL_CSTATE_INVALID -1UL
> >
> > --
> > 2.17.1
> >
>
> --
> With Best Regards,
> Andy Shevchenko
>
>