From a42a5839f400e929c489bb1b58f54596c4535167 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Thu, 3 Oct 2024 14:25:58 +0200 Subject: [PATCH 1/2] thermal: core: Reference count the zone in thermal_zone_get_by_id() There are places in the thermal netlink code where nothing prevents the 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 with the help of the cleanup.h infrastructure. Fixes: 1ce50e7d408e ("thermal: core: genetlink support for events/cmd/sampling") Cc: 6.8+ # 6.8+ Signed-off-by: Rafael J. Wysocki Reviewed-by: Lukasz Luba Link: https://patch.msgid.link/6112242.lOV4Wx5bFT@rjwysocki.net --- drivers/thermal/thermal_core.c | 1 + drivers/thermal/thermal_core.h | 3 +++ drivers/thermal/thermal_netlink.c | 9 +++------ 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c index 073d02e21352e6..c69f8123fb73a9 100644 --- a/drivers/thermal/thermal_core.c +++ b/drivers/thermal/thermal_core.c @@ -728,6 +728,7 @@ struct thermal_zone_device *thermal_zone_get_by_id(int id) 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; } diff --git a/drivers/thermal/thermal_core.h b/drivers/thermal/thermal_core.h index 50b858aa173a07..a64d39b1c86b23 100644 --- a/drivers/thermal/thermal_core.h +++ b/drivers/thermal/thermal_core.h @@ -194,6 +194,9 @@ int for_each_thermal_governor(int (*cb)(struct thermal_governor *, void *), struct thermal_zone_device *thermal_zone_get_by_id(int id); +DEFINE_CLASS(thermal_zone_get_by_id, struct thermal_zone_device *, + if (_T) put_device(&_T->device), thermal_zone_get_by_id(id), int id) + static inline bool cdev_is_power_actor(struct thermal_cooling_device *cdev) { return cdev->ops->get_requested_power && cdev->ops->state2power && diff --git a/drivers/thermal/thermal_netlink.c b/drivers/thermal/thermal_netlink.c index 97157c45363019..f3c58c708969c2 100644 --- a/drivers/thermal/thermal_netlink.c +++ b/drivers/thermal/thermal_netlink.c @@ -443,7 +443,6 @@ static int thermal_genl_cmd_tz_get_trip(struct param *p) { struct sk_buff *msg = p->msg; const struct thermal_trip_desc *td; - struct thermal_zone_device *tz; struct nlattr *start_trip; int id; @@ -452,7 +451,7 @@ static int thermal_genl_cmd_tz_get_trip(struct param *p) id = nla_get_u32(p->attrs[THERMAL_GENL_ATTR_TZ_ID]); - tz = thermal_zone_get_by_id(id); + CLASS(thermal_zone_get_by_id, tz)(id); if (!tz) return -EINVAL; @@ -488,7 +487,6 @@ static int thermal_genl_cmd_tz_get_trip(struct param *p) static int thermal_genl_cmd_tz_get_temp(struct param *p) { struct sk_buff *msg = p->msg; - struct thermal_zone_device *tz; int temp, ret, id; if (!p->attrs[THERMAL_GENL_ATTR_TZ_ID]) @@ -496,7 +494,7 @@ static int thermal_genl_cmd_tz_get_temp(struct param *p) id = nla_get_u32(p->attrs[THERMAL_GENL_ATTR_TZ_ID]); - tz = thermal_zone_get_by_id(id); + CLASS(thermal_zone_get_by_id, tz)(id); if (!tz) return -EINVAL; @@ -514,7 +512,6 @@ static int thermal_genl_cmd_tz_get_temp(struct param *p) static int thermal_genl_cmd_tz_get_gov(struct param *p) { struct sk_buff *msg = p->msg; - struct thermal_zone_device *tz; int id, ret = 0; if (!p->attrs[THERMAL_GENL_ATTR_TZ_ID]) @@ -522,7 +519,7 @@ static int thermal_genl_cmd_tz_get_gov(struct param *p) id = nla_get_u32(p->attrs[THERMAL_GENL_ATTR_TZ_ID]); - tz = thermal_zone_get_by_id(id); + CLASS(thermal_zone_get_by_id, tz)(id); if (!tz) return -EINVAL; From 827a07525c099f54d3b15110408824541ec66b3c Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Thu, 3 Oct 2024 14:27:28 +0200 Subject: [PATCH 2/2] thermal: core: Free tzp copy along with the thermal zone 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+ # 6.8+ Signed-off-by: Rafael J. Wysocki Reviewed-by: Lukasz Luba Link: https://patch.msgid.link/4623516.LvFx2qVVIh@rjwysocki.net --- drivers/thermal/thermal_core.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c index c69f8123fb73a9..8f03985f971c30 100644 --- a/drivers/thermal/thermal_core.c +++ b/drivers/thermal/thermal_core.c @@ -1606,14 +1606,12 @@ void thermal_zone_device_unregister(struct thermal_zone_device *tz) 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);