[PATCH v1 0/2] thermal: core: Fix potential use-after-free issues

[PATCH v1 0/2] thermal: core: Fix potential use-after-free issues

[Rafael J. Wysocki]

Hi Everyone,

These fix potential use-after-free issues in the thermal netlink code
and in the thermal core (during thermal zone unregistration).

They have been found by code inspection, but nevertheless they should be
addressed ASAP IMV.

Thanks!

[PATCH v1 1/2] thermal: core: Reference count the zone in thermal_zone_get_by_id()

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

There are places in the thermal netlink code where nothing prevents
a thermal zone object from going away while being accessed after it
has been returned by thermal_zone_get_by_id().

To address this, make thermal_zone_get_by_id() get a reference on the
thermal zone device object to be returned with the help of get_device(),
under thermal_list_lock, and adjust all of its callers to this change.

Fixes: 1ce50e7d408e ("thermal: core: genetlink support for events/cmd/sampling")
Cc: 6.8+ <stable@vger.kernel.org> # 6.8+
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/thermal/thermal_core.c    |    1 +
 drivers/thermal/thermal_core.h    |    5 +++++
 drivers/thermal/thermal_netlink.c |   22 ++++++++++++++--------
 3 files changed, 20 insertions(+), 8 deletions(-)

Index: linux-pm/drivers/thermal/thermal_core.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_core.c
+++ linux-pm/drivers/thermal/thermal_core.c
@@ -728,6 +728,7 @@ struct thermal_zone_device *thermal_zone
 	mutex_lock(&thermal_list_lock);
 	list_for_each_entry(tz, &thermal_tz_list, node) {
 		if (tz->id == id) {
+			get_device(&tz->device);
 			match = tz;
 			break;
 		}
Index: linux-pm/drivers/thermal/thermal_core.h
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_core.h
+++ linux-pm/drivers/thermal/thermal_core.h
@@ -194,6 +194,11 @@ int for_each_thermal_governor(int (*cb)(
 
 struct thermal_zone_device *thermal_zone_get_by_id(int id);
 
+static inline void thermal_zone_put(struct thermal_zone_device *tz)
+{
+	put_device(&tz->device);
+}
+
 static inline bool cdev_is_power_actor(struct thermal_cooling_device *cdev)
 {
 	return cdev->ops->get_requested_power && cdev->ops->state2power &&
Index: linux-pm/drivers/thermal/thermal_netlink.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_netlink.c
+++ linux-pm/drivers/thermal/thermal_netlink.c
@@ -445,7 +445,7 @@ static int thermal_genl_cmd_tz_get_trip(
 	const struct thermal_trip_desc *td;
 	struct thermal_zone_device *tz;
 	struct nlattr *start_trip;
-	int id;
+	int id, ret = -EMSGSIZE;
 
 	if (!p->attrs[THERMAL_GENL_ATTR_TZ_ID])
 		return -EINVAL;
@@ -458,7 +458,7 @@ static int thermal_genl_cmd_tz_get_trip(
 
 	start_trip = nla_nest_start(msg, THERMAL_GENL_ATTR_TZ_TRIP);
 	if (!start_trip)
-		return -EMSGSIZE;
+		goto out_put;
 
 	mutex_lock(&tz->lock);
 
@@ -470,19 +470,20 @@ static int thermal_genl_cmd_tz_get_trip(
 		    nla_put_u32(msg, THERMAL_GENL_ATTR_TZ_TRIP_TYPE, trip->type) ||
 		    nla_put_u32(msg, THERMAL_GENL_ATTR_TZ_TRIP_TEMP, trip->temperature) ||
 		    nla_put_u32(msg, THERMAL_GENL_ATTR_TZ_TRIP_HYST, trip->hysteresis))
-			goto out_cancel_nest;
+			goto out_unlock;
 	}
 
-	mutex_unlock(&tz->lock);
-
 	nla_nest_end(msg, start_trip);
 
-	return 0;
+	ret = 0;
 
-out_cancel_nest:
+out_unlock:
 	mutex_unlock(&tz->lock);
 
-	return -EMSGSIZE;
+out_put:
+	thermal_zone_put(tz);
+
+	return ret;
 }
 
 static int thermal_genl_cmd_tz_get_temp(struct param *p)
@@ -501,6 +502,9 @@ static int thermal_genl_cmd_tz_get_temp(
 		return -EINVAL;
 
 	ret = thermal_zone_get_temp(tz, &temp);
+
+	thermal_zone_put(tz);
+
 	if (ret)
 		return ret;
 
@@ -535,6 +539,8 @@ static int thermal_genl_cmd_tz_get_gov(s
 
 	mutex_unlock(&tz->lock);
 
+	thermal_zone_put(tz);
+
 	return ret;
 }
 

[PATCH v1 2/2] thermal: core: Free tzp copy along with the thermal zone

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

The object pointed to by tz->tzp may still be accessed after being
freed in thermal_zone_device_unregister(), so move the freeing of it
to the point after the removal completion has been completed at which
it cannot be accessed any more.

Fixes: 3d439b1a2ad3 ("thermal/core: Alloc-copy-free the thermal zone parameters structure")
Cc: 6.8+ <stable@vger.kernel.org> # 6.8+
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/thermal/thermal_core.c |    4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

Index: linux-pm/drivers/thermal/thermal_core.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_core.c
+++ linux-pm/drivers/thermal/thermal_core.c
@@ -1606,14 +1606,12 @@ void thermal_zone_device_unregister(stru
 	ida_destroy(&tz->ida);
 
 	device_del(&tz->device);
-
-	kfree(tz->tzp);
-
 	put_device(&tz->device);
 
 	thermal_notify_tz_delete(tz);
 
 	wait_for_completion(&tz->removal);
+	kfree(tz->tzp);
 	kfree(tz);
 }
 EXPORT_SYMBOL_GPL(thermal_zone_device_unregister);