thermal: gov_power_allocator: Fix NULL pointer dereference in update_tz()

[PATCH] thermal: gov_power_allocator: Fix NULL pointer dereference in update_tz()

Posted by Sumeet Pawnikar 1 month ago

power_allocator_update_tz() unconditionally derives the trip descriptor of
params->trip_max as below,
  const struct thermal_trip_desc *td = trip_to_trip_desc(params->trip_max);
and then walks td->thermal_instances. However, params->trip_max is allowed
to be NULL and in that case the list walk dereferences a bogus pointer
derived from NULL which oops the kernel.

get_governor_trips() picks trip_switch_on and trip_max out of the trip
table of the zone. When the zone has neither a passive nor an active trip
point, last_active is NULL and params->trip_max is left NULL. This is an
explicitly supported configuration, as documented in the function
get_governor_trips() as below,
  If there are no passive or active trip points, then the governor won't
  do anything. In fact, its throttle function won't be called at all.

Return early from power_allocator_update_tz() when params->trip_max is NULL
and only compute the trip descriptor after that check. There is nothing to
update in that case anyway, with no trip_max and there are no thermal
instances for the governor to account for, num_actors stays zero and
total_weight is irrelevant.

Fixes: 912e97c67cc3 ("thermal: gov_power_allocator: Move memory allocation out of throttle()")

Signed-off-by: Sumeet Pawnikar <sumeet4linux@gmail.com>
---
 drivers/thermal/gov_power_allocator.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/thermal/gov_power_allocator.c b/drivers/thermal/gov_power_allocator.c
index 37f2e22a999e..b5c254187628 100644
--- a/drivers/thermal/gov_power_allocator.c
+++ b/drivers/thermal/gov_power_allocator.c
@@ -660,10 +660,15 @@ static void power_allocator_update_tz(struct thermal_zone_device *tz,
 				      enum thermal_notify_event reason)
 {
 	struct power_allocator_params *params = tz->governor_data;
-	const struct thermal_trip_desc *td = trip_to_trip_desc(params->trip_max);
+	const struct thermal_trip_desc *td;
 	struct thermal_instance *instance;
 	int num_actors = 0;
 
+	if (!params->trip_max)
+		return;
+
+	td = trip_to_trip_desc(params->trip_max);
+
 	switch (reason) {
 	case THERMAL_TZ_BIND_CDEV:
 	case THERMAL_TZ_UNBIND_CDEV:
-- 
2.43.0

Re: [PATCH] thermal: gov_power_allocator: Fix NULL pointer dereference in update_tz()

Posted by Lukasz Luba 3 weeks ago


On 8/22/26 12:42, Sumeet Pawnikar wrote:
> power_allocator_update_tz() unconditionally derives the trip descriptor of
> params->trip_max as below,
>    const struct thermal_trip_desc *td = trip_to_trip_desc(params->trip_max);
> and then walks td->thermal_instances. However, params->trip_max is allowed
> to be NULL and in that case the list walk dereferences a bogus pointer
> derived from NULL which oops the kernel.
> 
> get_governor_trips() picks trip_switch_on and trip_max out of the trip
> table of the zone. When the zone has neither a passive nor an active trip
> point, last_active is NULL and params->trip_max is left NULL. This is an
> explicitly supported configuration, as documented in the function
> get_governor_trips() as below,
>    If there are no passive or active trip points, then the governor won't
>    do anything. In fact, its throttle function won't be called at all.
> 
> Return early from power_allocator_update_tz() when params->trip_max is NULL
> and only compute the trip descriptor after that check. There is nothing to
> update in that case anyway, with no trip_max and there are no thermal
> instances for the governor to account for, num_actors stays zero and
> total_weight is irrelevant.
> 
> Fixes: 912e97c67cc3 ("thermal: gov_power_allocator: Move memory allocation out of throttle()")
> 
> Signed-off-by: Sumeet Pawnikar <sumeet4linux@gmail.com>
> ---
>   drivers/thermal/gov_power_allocator.c | 7 ++++++-
>   1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/thermal/gov_power_allocator.c b/drivers/thermal/gov_power_allocator.c
> index 37f2e22a999e..b5c254187628 100644
> --- a/drivers/thermal/gov_power_allocator.c
> +++ b/drivers/thermal/gov_power_allocator.c
> @@ -660,10 +660,15 @@ static void power_allocator_update_tz(struct thermal_zone_device *tz,
>   				      enum thermal_notify_event reason)
>   {
>   	struct power_allocator_params *params = tz->governor_data;
> -	const struct thermal_trip_desc *td = trip_to_trip_desc(params->trip_max);
> +	const struct thermal_trip_desc *td;
>   	struct thermal_instance *instance;
>   	int num_actors = 0;
>   
> +	if (!params->trip_max)
> +		return;
> +
> +	td = trip_to_trip_desc(params->trip_max);
> +
>   	switch (reason) {
>   	case THERMAL_TZ_BIND_CDEV:
>   	case THERMAL_TZ_UNBIND_CDEV:


LGTM,

Reviewed-by: Lukasz Luba <lukasz.luba@arm.com>

Re: [PATCH] thermal: gov_power_allocator: Fix NULL pointer dereference in update_tz()

Posted by Rafael J. Wysocki (Intel) 2 weeks, 2 days ago

On Mon, Sep 7, 2026 at 9:51 AM Lukasz Luba <lukasz.luba@arm.com> wrote:
>
>
>
> On 8/22/26 12:42, Sumeet Pawnikar wrote:
> > power_allocator_update_tz() unconditionally derives the trip descriptor of
> > params->trip_max as below,
> >    const struct thermal_trip_desc *td = trip_to_trip_desc(params->trip_max);
> > and then walks td->thermal_instances. However, params->trip_max is allowed
> > to be NULL and in that case the list walk dereferences a bogus pointer
> > derived from NULL which oops the kernel.
> >
> > get_governor_trips() picks trip_switch_on and trip_max out of the trip
> > table of the zone. When the zone has neither a passive nor an active trip
> > point, last_active is NULL and params->trip_max is left NULL. This is an
> > explicitly supported configuration, as documented in the function
> > get_governor_trips() as below,
> >    If there are no passive or active trip points, then the governor won't
> >    do anything. In fact, its throttle function won't be called at all.
> >
> > Return early from power_allocator_update_tz() when params->trip_max is NULL
> > and only compute the trip descriptor after that check. There is nothing to
> > update in that case anyway, with no trip_max and there are no thermal
> > instances for the governor to account for, num_actors stays zero and
> > total_weight is irrelevant.
> >
> > Fixes: 912e97c67cc3 ("thermal: gov_power_allocator: Move memory allocation out of throttle()")
> >
> > Signed-off-by: Sumeet Pawnikar <sumeet4linux@gmail.com>
> > ---
> >   drivers/thermal/gov_power_allocator.c | 7 ++++++-
> >   1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/thermal/gov_power_allocator.c b/drivers/thermal/gov_power_allocator.c
> > index 37f2e22a999e..b5c254187628 100644
> > --- a/drivers/thermal/gov_power_allocator.c
> > +++ b/drivers/thermal/gov_power_allocator.c
> > @@ -660,10 +660,15 @@ static void power_allocator_update_tz(struct thermal_zone_device *tz,
> >                                     enum thermal_notify_event reason)
> >   {
> >       struct power_allocator_params *params = tz->governor_data;
> > -     const struct thermal_trip_desc *td = trip_to_trip_desc(params->trip_max);
> > +     const struct thermal_trip_desc *td;
> >       struct thermal_instance *instance;
> >       int num_actors = 0;
> >
> > +     if (!params->trip_max)
> > +             return;
> > +
> > +     td = trip_to_trip_desc(params->trip_max);
> > +
> >       switch (reason) {
> >       case THERMAL_TZ_BIND_CDEV:
> >       case THERMAL_TZ_UNBIND_CDEV:
>
> Reviewed-by: Lukasz Luba <lukasz.luba@arm.com>

Applied as 7.4 material, thanks!