[PATCH] PM: EM: Fix use-after-free of perf domain in netlink doit handlers

Fan Wu posted 1 patch 2 days, 7 hours ago
kernel/power/em_netlink.c   | 122 +++++++++++++++++++++++-------------
kernel/power/em_netlink.h   |   8 ++-
kernel/power/energy_model.c |   8 ++-
3 files changed, 87 insertions(+), 51 deletions(-)
[PATCH] PM: EM: Fix use-after-free of perf domain in netlink doit handlers
Posted by Fan Wu 2 days, 7 hours ago
The doit handlers dropped em_pd_list_mutex before dereferencing the
perf domain, so a concurrent device unregister could free it while the
reply was being built.

Encode the reply from em_perf_domain_for_id() callbacks, which run
under the mutex, and allocate the skb outside of it, like the dumpit
path already does.

This issue was found by an in-house static analysis tool.

Fixes: 380ff27af25e ("PM: EM: Add dump to get-perf-domains in the EM YNL spec")
Fixes: f2d2946eaa5c ("PM: EM: Implement em_nl_get_pd_table_doit()")
Cc: stable@vger.kernel.org
Cc: Lukasz Luba <lukasz.luba@arm.com>
Assisted-by: Codex:gpt-5.6
Co-developed-by: Song Li <songl@zju.edu.cn>
Signed-off-by: Song Li <songl@zju.edu.cn>
Signed-off-by: Fan Wu <fanwu01@zju.edu.cn>
---
 kernel/power/em_netlink.c   | 122 +++++++++++++++++++++++-------------
 kernel/power/em_netlink.h   |   8 ++-
 kernel/power/energy_model.c |   8 ++-
 3 files changed, 87 insertions(+), 51 deletions(-)

diff --git a/kernel/power/em_netlink.c b/kernel/power/em_netlink.c
index 4d4fd29bd2be..1f68d456a713 100644
--- a/kernel/power/em_netlink.c
+++ b/kernel/power/em_netlink.c
@@ -95,41 +95,62 @@ static int __em_nl_get_pd_for_dump(struct em_perf_domain *pd, void *data)
 	return ret;
 }
 
+struct em_nl_doit_ctx {
+	struct genl_info *info;
+	int cmd;
+	struct sk_buff *msg;
+};
+
+static int __em_nl_get_pd_doit_fill(struct em_perf_domain *pd, void *data)
+{
+	struct em_nl_doit_ctx *ctx = data;
+	void *hdr;
+
+	hdr = genlmsg_put_reply(ctx->msg, ctx->info, &dev_energymodel_nl_family, 0,
+				ctx->cmd);
+	if (!hdr)
+		return -EMSGSIZE;
+
+	if (__em_nl_get_pd(pd, ctx->msg)) {
+		genlmsg_cancel(ctx->msg, hdr);
+		return -EMSGSIZE;
+	}
+
+	genlmsg_end(ctx->msg, hdr);
+	return 0;
+}
+
 int dev_energymodel_nl_get_perf_domains_doit(struct sk_buff *skb,
-					      struct genl_info *info)
+					     struct genl_info *info)
 {
-	int id, ret = -EMSGSIZE, msg_sz = 0;
-	int cmd = info->genlhdr->cmd;
-	struct em_perf_domain *pd;
+	struct em_nl_doit_ctx ctx = {
+		.info = info,
+		.cmd = info->genlhdr->cmd,
+	};
 	struct sk_buff *msg;
-	void *hdr;
+	int id, ret, msg_sz = 0;
 
 	if (!info->attrs[DEV_ENERGYMODEL_A_PERF_DOMAIN_PERF_DOMAIN_ID])
 		return -EINVAL;
 
 	id = nla_get_u32(info->attrs[DEV_ENERGYMODEL_A_PERF_DOMAIN_PERF_DOMAIN_ID]);
-	pd = em_perf_domain_get_by_id(id);
-	if (!pd)
-		return -EINVAL;
 
-	__em_nl_get_pd_size(pd, &msg_sz);
+	/* Encode under em_pd_list_mutex, like the dumpit path. */
+	ret = em_perf_domain_for_id(id, __em_nl_get_pd_size, &msg_sz);
+	if (ret)
+		return ret;
+
 	msg = genlmsg_new(msg_sz, GFP_KERNEL);
 	if (!msg)
 		return -ENOMEM;
 
-	hdr = genlmsg_put_reply(msg, info, &dev_energymodel_nl_family, 0, cmd);
-	if (!hdr)
-		goto out_free_msg;
-
-	ret = __em_nl_get_pd(pd, msg);
+	ctx.msg = msg;
+	ret = em_perf_domain_for_id(id, __em_nl_get_pd_doit_fill, &ctx);
 	if (ret)
-		goto out_cancel_msg;
-	genlmsg_end(msg, hdr);
+		goto out_free_msg;
 
 	return genlmsg_reply(msg, info);
 
-out_cancel_msg:
-	genlmsg_cancel(msg, hdr);
 out_free_msg:
 	nlmsg_free(msg);
 	return ret;
@@ -148,19 +169,6 @@ int dev_energymodel_nl_get_perf_domains_dumpit(struct sk_buff *skb,
 	return for_each_em_perf_domain(__em_nl_get_pd_for_dump, &ctx);
 }
 
-static struct em_perf_domain *__em_nl_get_pd_table_id(struct nlattr **attrs)
-{
-	struct em_perf_domain *pd;
-	int id;
-
-	if (!attrs[DEV_ENERGYMODEL_A_PERF_TABLE_PERF_DOMAIN_ID])
-		return NULL;
-
-	id = nla_get_u32(attrs[DEV_ENERGYMODEL_A_PERF_TABLE_PERF_DOMAIN_ID]);
-	pd = em_perf_domain_get_by_id(id);
-	return pd;
-}
-
 static int __em_nl_get_pd_table_size(const struct em_perf_domain *pd)
 {
 	int id_sz, ps_sz;
@@ -245,34 +253,58 @@ int __em_nl_get_pd_table(struct sk_buff *msg, const struct em_perf_domain *pd)
 	return -EMSGSIZE;
 }
 
+static int __em_nl_get_pd_table_size_cb(struct em_perf_domain *pd, void *data)
+{
+	*(int *)data = __em_nl_get_pd_table_size(pd);
+	return 0;
+}
+
+static int __em_nl_get_pd_table_doit_fill(struct em_perf_domain *pd,
+					  void *data)
+{
+	struct em_nl_doit_ctx *ctx = data;
+	void *hdr;
+
+	hdr = genlmsg_put_reply(ctx->msg, ctx->info, &dev_energymodel_nl_family, 0,
+				ctx->cmd);
+	if (!hdr)
+		return -EMSGSIZE;
+
+	if (__em_nl_get_pd_table(ctx->msg, pd))
+		return -EMSGSIZE;
+
+	genlmsg_end(ctx->msg, hdr);
+	return 0;
+}
+
 int dev_energymodel_nl_get_perf_table_doit(struct sk_buff *skb,
-					    struct genl_info *info)
+					   struct genl_info *info)
 {
-	int cmd = info->genlhdr->cmd;
-	int msg_sz, ret = -EMSGSIZE;
-	struct em_perf_domain *pd;
+	struct em_nl_doit_ctx ctx = {
+		.info = info,
+		.cmd = info->genlhdr->cmd,
+	};
 	struct sk_buff *msg;
-	void *hdr;
+	int id, ret, msg_sz;
 
-	pd = __em_nl_get_pd_table_id(info->attrs);
-	if (!pd)
+	if (!info->attrs[DEV_ENERGYMODEL_A_PERF_TABLE_PERF_DOMAIN_ID])
 		return -EINVAL;
 
-	msg_sz = __em_nl_get_pd_table_size(pd);
+	id = nla_get_u32(info->attrs[DEV_ENERGYMODEL_A_PERF_TABLE_PERF_DOMAIN_ID]);
+
+	ret = em_perf_domain_for_id(id, __em_nl_get_pd_table_size_cb, &msg_sz);
+	if (ret)
+		return ret;
 
 	msg = genlmsg_new(msg_sz, GFP_KERNEL);
 	if (!msg)
 		return -ENOMEM;
 
-	hdr = genlmsg_put_reply(msg, info, &dev_energymodel_nl_family, 0, cmd);
-	if (!hdr)
-		goto out_free_msg;
-
-	ret = __em_nl_get_pd_table(msg, pd);
+	ctx.msg = msg;
+	ret = em_perf_domain_for_id(id, __em_nl_get_pd_table_doit_fill, &ctx);
 	if (ret)
 		goto out_free_msg;
 
-	genlmsg_end(msg, hdr);
 	return genlmsg_reply(msg, info);
 
 out_free_msg:
diff --git a/kernel/power/em_netlink.h b/kernel/power/em_netlink.h
index 583d7f1c3939..bc98a3c278b9 100644
--- a/kernel/power/em_netlink.h
+++ b/kernel/power/em_netlink.h
@@ -12,7 +12,8 @@
 #if defined(CONFIG_ENERGY_MODEL) && defined(CONFIG_NET)
 int for_each_em_perf_domain(int (*cb)(struct em_perf_domain*, void *),
 			    void *data);
-struct em_perf_domain *em_perf_domain_get_by_id(int id);
+int em_perf_domain_for_id(int id, int (*cb)(struct em_perf_domain *, void *),
+			  void *data);
 void em_notify_pd_created(const struct em_perf_domain *pd);
 void em_notify_pd_deleted(const struct em_perf_domain *pd);
 void em_notify_pd_updated(const struct em_perf_domain *pd);
@@ -24,9 +25,10 @@ int for_each_em_perf_domain(int (*cb)(struct em_perf_domain*, void *),
 	return -EINVAL;
 }
 static inline
-struct em_perf_domain *em_perf_domain_get_by_id(int id)
+int em_perf_domain_for_id(int id, int (*cb)(struct em_perf_domain *, void *),
+			  void *data)
 {
-	return NULL;
+	return -EINVAL;
 }
 
 static inline void em_notify_pd_created(const struct em_perf_domain *pd) {}
diff --git a/kernel/power/energy_model.c b/kernel/power/energy_model.c
index e610cf8e9a06..a76089e2ce8c 100644
--- a/kernel/power/energy_model.c
+++ b/kernel/power/energy_model.c
@@ -1031,7 +1031,9 @@ int for_each_em_perf_domain(int (*cb)(struct em_perf_domain*, void *),
 	return 0;
 }
 
-struct em_perf_domain *em_perf_domain_get_by_id(int id)
+/* Run @cb on the matching domain with em_pd_list_mutex held. */
+int em_perf_domain_for_id(int id, int (*cb)(struct em_perf_domain *, void *),
+			  void *data)
 {
 	struct em_perf_domain *pd;
 
@@ -1040,9 +1042,9 @@ struct em_perf_domain *em_perf_domain_get_by_id(int id)
 
 	list_for_each_entry(pd, &em_pd_list, node) {
 		if (pd->id == id)
-			return pd;
+			return cb(pd, data);
 	}
 
-	return NULL;
+	return -EINVAL;
 }
 #endif