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

On Thu, Jan 19, 2023 at 8:29 PM Srinivas Pandruvada srinivas.pandruvada@linux.intel.com 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@linux.intel.com Cc: stable@vger.kernel.org # 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