Re: [PATCH v2] thermal: int340x: Protect trip temperature from dynamic update

[Rafael J. Wysocki]

On Mon, Jan 23, 2023 at 6:26 PM Srinivas Pandruvada
<srinivas.pandruvada@xxxxxxxxxxxxxxx> wrote:
>
> Trip temperatures are read using ACPI methods and stored in the memory
> during zone initializtion and when the firmware sends a notification for
> change. This trip temperature is returned when the thermal core calls via
> callback get_trip_temp().
>
> But it is possible that while updating the memory copy of the trips when
> the firmware sends a notification for change, thermal core is reading the
> trip temperature via the callback get_trip_temp(). This may return invalid
> trip temperature.
>
> To address this add a mutex to protect the invalid temperature reads in
> the callback get_trip_temp() and int340x_thermal_read_trips().
>
> Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@xxxxxxxxxxxxxxx>
> Cc: stable@xxxxxxxxxxxxxxx # 5.0+
> ---
> v2:
> - rebased on linux-next

So I've rebased it back onto 6.2-rc5 and pushed the result into the
thermal-intel-fixes branch.  Please see if it looks good to you and
let me know.

I'd prefer to push it for 6.2-rc.

> - Add ret variable and remove return as suugested by Rafael

Thanks!

>  .../int340x_thermal/int340x_thermal_zone.c     | 18 +++++++++++++++---
>  .../int340x_thermal/int340x_thermal_zone.h     |  1 +
>  2 files changed, 16 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c b/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
> index 228f44260b27..5fda1e67b793 100644
> --- a/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
> +++ b/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
> @@ -41,7 +41,9 @@ static int int340x_thermal_get_trip_temp(struct thermal_zone_device *zone,
>                                          int trip, int *temp)
>  {
>         struct int34x_thermal_zone *d = zone->devdata;
> -       int i;
> +       int i, ret = 0;
> +
> +       mutex_lock(&d->trip_mutex);
>
>         if (trip < d->aux_trip_nr)
>                 *temp = d->aux_trips[trip];
> @@ -60,10 +62,12 @@ static int int340x_thermal_get_trip_temp(struct thermal_zone_device *zone,
>                         }
>                 }
>                 if (i == INT340X_THERMAL_MAX_ACT_TRIP_COUNT)
> -                       return -EINVAL;
> +                       ret = -EINVAL;
>         }
>
> -       return 0;
> +       mutex_unlock(&d->trip_mutex);
> +
> +       return ret;
>  }
>
>  static int int340x_thermal_get_trip_type(struct thermal_zone_device *zone,
> @@ -165,6 +169,8 @@ int int340x_thermal_read_trips(struct int34x_thermal_zone *int34x_zone)
>         int trip_cnt = int34x_zone->aux_trip_nr;
>         int i;
>
> +       mutex_lock(&int34x_zone->trip_mutex);
> +
>         int34x_zone->crt_trip_id = -1;
>         if (!int340x_thermal_get_trip_config(int34x_zone->adev->handle, "_CRT",
>                                              &int34x_zone->crt_temp))
> @@ -192,6 +198,8 @@ int int340x_thermal_read_trips(struct int34x_thermal_zone *int34x_zone)
>                 int34x_zone->act_trips[i].valid = true;
>         }
>
> +       mutex_unlock(&int34x_zone->trip_mutex);
> +
>         return trip_cnt;
>  }
>  EXPORT_SYMBOL_GPL(int340x_thermal_read_trips);
> @@ -215,6 +223,8 @@ struct int34x_thermal_zone *int340x_thermal_zone_add(struct acpi_device *adev,
>         if (!int34x_thermal_zone)
>                 return ERR_PTR(-ENOMEM);
>
> +       mutex_init(&int34x_thermal_zone->trip_mutex);
> +
>         int34x_thermal_zone->adev = adev;
>
>         int34x_thermal_zone->ops = kmemdup(&int340x_thermal_zone_ops,
> @@ -277,6 +287,7 @@ struct int34x_thermal_zone *int340x_thermal_zone_add(struct acpi_device *adev,
>  err_trip_alloc:
>         kfree(int34x_thermal_zone->ops);
>  err_ops_alloc:
> +       mutex_destroy(&int34x_thermal_zone->trip_mutex);
>         kfree(int34x_thermal_zone);
>         return ERR_PTR(ret);
>  }
> @@ -289,6 +300,7 @@ void int340x_thermal_zone_remove(struct int34x_thermal_zone
>         acpi_lpat_free_conversion_table(int34x_thermal_zone->lpat_table);
>         kfree(int34x_thermal_zone->aux_trips);
>         kfree(int34x_thermal_zone->ops);
> +       mutex_destroy(&int34x_thermal_zone->trip_mutex);
>         kfree(int34x_thermal_zone);
>  }
>  EXPORT_SYMBOL_GPL(int340x_thermal_zone_remove);
> diff --git a/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.h b/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.h
> index e28ab1ba5e06..6610a9cc441b 100644
> --- a/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.h
> +++ b/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.h
> @@ -32,6 +32,7 @@ struct int34x_thermal_zone {
>         struct thermal_zone_device_ops *ops;
>         void *priv_data;
>         struct acpi_lpat_conversion_table *lpat_table;
> +       struct mutex trip_mutex;
>  };
>
>  struct int34x_thermal_zone *int340x_thermal_zone_add(struct acpi_device *,
> --
> 2.31.1
>