6.8-stable review patch. If anyone has any objections, please let me know.
------------------
From: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
[ Upstream commit c7f7c37271787a7f77d7eedc132b0b419a76b4c8 ]
With the current thermal zone locking arrangement in the debugfs code,
user space can open the "mitigations" file for a thermal zone before
the zone's debugfs pointer is set which will result in a NULL pointer
dereference in tze_seq_start().
Moreover, thermal_debug_tz_remove() is not called under the thermal
zone lock, so it can run in parallel with the other functions accessing
the thermal zone's struct thermal_debugfs object. Then, it may clear
tz->debugfs after one of those functions has checked it and the
struct thermal_debugfs object may be freed prematurely.
To address the first problem, pass a pointer to the thermal zone's
struct thermal_debugfs object to debugfs_create_file() in
thermal_debug_tz_add() and make tze_seq_start(), tze_seq_next(),
tze_seq_stop(), and tze_seq_show() retrieve it from s->private
instead of a pointer to the thermal zone object. This will ensure
that tz_debugfs will be valid across the "mitigations" file accesses
until thermal_debugfs_remove_id() called by thermal_debug_tz_remove()
removes that file.
To address the second problem, use tz->lock in thermal_debug_tz_remove()
around the tz->debugfs value check (in case the same thermal zone is
removed at the same time in two different threads) and its reset to NULL.
Fixes: 7ef01f228c9f ("thermal/debugfs: Add thermal debugfs information for mitigation episodes")
Cc :6.8+ <stable@xxxxxxxxxxxxxxx> # 6.8+
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
Reviewed-by: Lukasz Luba <lukasz.luba@xxxxxxx>
Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>
---
drivers/thermal/thermal_debugfs.c | 34 ++++++++++++++++++++-----------
1 file changed, 22 insertions(+), 12 deletions(-)
diff --git a/drivers/thermal/thermal_debugfs.c b/drivers/thermal/thermal_debugfs.c
index 8d66b3a96be1a..1fe2914a8853b 100644
--- a/drivers/thermal/thermal_debugfs.c
+++ b/drivers/thermal/thermal_debugfs.c
@@ -139,11 +139,13 @@ struct tz_episode {
* we keep track of the current position in the history array.
*
* @tz_episodes: a list of thermal mitigation episodes
+ * @tz: thermal zone this object belongs to
* @trips_crossed: an array of trip points crossed by id
* @nr_trips: the number of trip points currently being crossed
*/
struct tz_debugfs {
struct list_head tz_episodes;
+ struct thermal_zone_device *tz;
int *trips_crossed;
int nr_trips;
};
@@ -716,8 +718,7 @@ void thermal_debug_update_temp(struct thermal_zone_device *tz)
static void *tze_seq_start(struct seq_file *s, loff_t *pos)
{
- struct thermal_zone_device *tz = s->private;
- struct thermal_debugfs *thermal_dbg = tz->debugfs;
+ struct thermal_debugfs *thermal_dbg = s->private;
struct tz_debugfs *tz_dbg = &thermal_dbg->tz_dbg;
mutex_lock(&thermal_dbg->lock);
@@ -727,8 +728,7 @@ static void *tze_seq_start(struct seq_file *s, loff_t *pos)
static void *tze_seq_next(struct seq_file *s, void *v, loff_t *pos)
{
- struct thermal_zone_device *tz = s->private;
- struct thermal_debugfs *thermal_dbg = tz->debugfs;
+ struct thermal_debugfs *thermal_dbg = s->private;
struct tz_debugfs *tz_dbg = &thermal_dbg->tz_dbg;
return seq_list_next(v, &tz_dbg->tz_episodes, pos);
@@ -736,15 +736,15 @@ static void *tze_seq_next(struct seq_file *s, void *v, loff_t *pos)
static void tze_seq_stop(struct seq_file *s, void *v)
{
- struct thermal_zone_device *tz = s->private;
- struct thermal_debugfs *thermal_dbg = tz->debugfs;
+ struct thermal_debugfs *thermal_dbg = s->private;
mutex_unlock(&thermal_dbg->lock);
}
static int tze_seq_show(struct seq_file *s, void *v)
{
- struct thermal_zone_device *tz = s->private;
+ struct thermal_debugfs *thermal_dbg = s->private;
+ struct thermal_zone_device *tz = thermal_dbg->tz_dbg.tz;
struct thermal_trip *trip;
struct tz_episode *tze;
const char *type;
@@ -810,6 +810,8 @@ void thermal_debug_tz_add(struct thermal_zone_device *tz)
tz_dbg = &thermal_dbg->tz_dbg;
+ tz_dbg->tz = tz;
+
tz_dbg->trips_crossed = kzalloc(sizeof(int) * tz->num_trips, GFP_KERNEL);
if (!tz_dbg->trips_crossed) {
thermal_debugfs_remove_id(thermal_dbg);
@@ -818,20 +820,30 @@ void thermal_debug_tz_add(struct thermal_zone_device *tz)
INIT_LIST_HEAD(&tz_dbg->tz_episodes);
- debugfs_create_file("mitigations", 0400, thermal_dbg->d_top, tz, &tze_fops);
+ debugfs_create_file("mitigations", 0400, thermal_dbg->d_top,
+ thermal_dbg, &tze_fops);
tz->debugfs = thermal_dbg;
}
void thermal_debug_tz_remove(struct thermal_zone_device *tz)
{
- struct thermal_debugfs *thermal_dbg = tz->debugfs;
+ struct thermal_debugfs *thermal_dbg;
struct tz_episode *tze, *tmp;
struct tz_debugfs *tz_dbg;
int *trips_crossed;
- if (!thermal_dbg)
+ mutex_lock(&tz->lock);
+
+ thermal_dbg = tz->debugfs;
+ if (!thermal_dbg) {
+ mutex_unlock(&tz->lock);
return;
+ }
+
+ tz->debugfs = NULL;
+
+ mutex_unlock(&tz->lock);
tz_dbg = &thermal_dbg->tz_dbg;
@@ -844,8 +856,6 @@ void thermal_debug_tz_remove(struct thermal_zone_device *tz)
kfree(tze);
}
- tz->debugfs = NULL;
-
mutex_unlock(&thermal_dbg->lock);
thermal_debugfs_remove_id(thermal_dbg);
--
2.43.0
