On Thu, Jan 19, 2023 at 8:29 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+
One nit below.
> ---
> .../intel/int340x_thermal/int340x_thermal_zone.c | 16 +++++++++++++++-
> .../intel/int340x_thermal/int340x_thermal_zone.h | 1 +
> 2 files changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c b/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
> index 62c0aa5d0783..fd9080640e03 100644
> --- a/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
> +++ b/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
> @@ -49,6 +49,8 @@ static int int340x_thermal_get_trip_temp(struct thermal_zone_device *zone,
I would add a new local var:
int ret = 0;
> if (d->override_ops && d->override_ops->get_trip_temp)
> return d->override_ops->get_trip_temp(zone, trip, temp);
>
> + mutex_lock(&d->trip_mutex);
> +
> if (trip < d->aux_trip_nr)
> *temp = d->aux_trips[trip];
> else if (trip == d->crt_trip_id)
> @@ -65,10 +67,14 @@ static int int340x_thermal_get_trip_temp(struct thermal_zone_device *zone,
> break;
> }
> }
> - if (i == INT340X_THERMAL_MAX_ACT_TRIP_COUNT)
> + if (i == INT340X_THERMAL_MAX_ACT_TRIP_COUNT) {
> + mutex_unlock(&d->trip_mutex);
> return -EINVAL;
And then do
ret = -EINVAL;
instead of the above.
> + }
> }
>
> + mutex_unlock(&d->trip_mutex);
> +
> return 0;
And
return ret;
here.
> }
>
> @@ -180,6 +186,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))
> @@ -207,6 +215,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);
> @@ -230,6 +240,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->override_ops = override_ops;
>
> @@ -281,6 +293,7 @@ struct int34x_thermal_zone *int340x_thermal_zone_add(struct acpi_device *adev,
> acpi_lpat_free_conversion_table(int34x_thermal_zone->lpat_table);
> kfree(int34x_thermal_zone->aux_trips);
> err_trip_alloc:
> + mutex_destroy(&int34x_thermal_zone->trip_mutex);
> kfree(int34x_thermal_zone);
> return ERR_PTR(ret);
> }
> @@ -292,6 +305,7 @@ void int340x_thermal_zone_remove(struct int34x_thermal_zone
> thermal_zone_device_unregister(int34x_thermal_zone->zone);
> acpi_lpat_free_conversion_table(int34x_thermal_zone->lpat_table);
> kfree(int34x_thermal_zone->aux_trips);
> + 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 3b4971df1b33..8f9872afd0d3 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 *override_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.38.1
>
