[PATCH] RDMA/rtrs: Fix potential use-after-free on client sysfs setup failure

[PATCH] RDMA/rtrs: Fix potential use-after-free on client sysfs setup failure

[Guangshuo Li]

rtrs_clt_create_path_files() drops the path reference on failure, but
its callers still close connections and free the path. This can access
freed memory and free the path twice.

Keep the caller's path reference until its cleanup completes. Move
per-CPU stats cleanup to the path release callback and let the embedded
stats kobject hold a path reference until its own release callback, so
delayed kobject release cannot outlive the enclosing allocation. Keep
direct cleanup for failures before the path kobject is initialized.

This issue was found by our static analysis tool under development.

Fixes: 215378b838df ("RDMA/rtrs: client: sysfs interface functions")
Cc: stable@vger.kernel.org
Assisted-by: OpenAI:gpt-6-astra
Signed-off-by: Guangshuo Li <liguangshuo@mail.sdu.edu.cn>
---
Build-tested the changed objects with x86_64 allmodconfig (W=1):
GCC with CONFIG_DEBUG_KOBJECT_RELEASE=n/y, and Clang with it enabled.
A host harness executing extracted cleanup functions passed 198
failure/teardown cases, including delayed release (also under ASan/UBSan).
No kernel runtime or RDMA hardware testing was performed.

 drivers/infiniband/ulp/rtrs/rtrs-clt-sysfs.c | 10 +++++++---
 drivers/infiniband/ulp/rtrs/rtrs-clt.c       | 14 ++++++++++----
 2 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/drivers/infiniband/ulp/rtrs/rtrs-clt-sysfs.c b/drivers/infiniband/ulp/rtrs/rtrs-clt-sysfs.c
index f8b833bd81ad..44edc405f400 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-clt-sysfs.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-clt-sysfs.c
@@ -22,6 +22,7 @@ static void rtrs_clt_path_release(struct kobject *kobj)
 
 	clt_path = container_of(kobj, struct rtrs_clt_path, kobj);
 
+	free_percpu(clt_path->stats->pcpu_stats);
 	free_path(clt_path);
 }
 
@@ -32,11 +33,13 @@ static struct kobj_type ktype_sess = {
 
 static void rtrs_clt_path_stats_release(struct kobject *kobj)
 {
+	struct rtrs_clt_path *clt_path;
 	struct rtrs_clt_stats *stats;
 
 	stats = container_of(kobj, struct rtrs_clt_stats, kobj_stats);
 
-	free_percpu(stats->pcpu_stats);
+	clt_path = container_of_const(stats, struct rtrs_clt_path, stats[0]);
+	kobject_put(&clt_path->kobj);
 }
 
 static struct kobj_type ktype_stats = {
@@ -422,6 +425,7 @@ static const struct attribute_group rtrs_clt_path_attr_group = {
 	.attrs = rtrs_clt_path_attrs,
 };
 
+/* The caller retains the initial path reference on all exits. */
 int rtrs_clt_create_path_files(struct rtrs_clt_path *clt_path)
 {
 	struct rtrs_clt_sess *clt = clt_path->clt;
@@ -438,7 +442,6 @@ int rtrs_clt_create_path_files(struct rtrs_clt_path *clt_path)
 				   "%s", str);
 	if (err) {
 		pr_err("kobject_init_and_add: %pe\n", ERR_PTR(err));
-		kobject_put(&clt_path->kobj);
 		return err;
 	}
 	err = sysfs_create_group(&clt_path->kobj, &rtrs_clt_path_attr_group);
@@ -446,6 +449,8 @@ int rtrs_clt_create_path_files(struct rtrs_clt_path *clt_path)
 		pr_err("sysfs_create_group(): %pe\n", ERR_PTR(err));
 		goto put_kobj;
 	}
+	/* Keep the path alive until its embedded stats kobject is released. */
+	kobject_get(&clt_path->kobj);
 	err = kobject_init_and_add(&clt_path->stats->kobj_stats, &ktype_stats,
 				   &clt_path->kobj, "stats");
 	if (err) {
@@ -470,7 +475,6 @@ int rtrs_clt_create_path_files(struct rtrs_clt_path *clt_path)
 	sysfs_remove_group(&clt_path->kobj, &rtrs_clt_path_attr_group);
 put_kobj:
 	kobject_del(&clt_path->kobj);
-	kobject_put(&clt_path->kobj);
 
 	return err;
 }
diff --git a/drivers/infiniband/ulp/rtrs/rtrs-clt.c b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
index bf3a0377da3f..547313136598 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-clt.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
@@ -2880,8 +2880,7 @@ struct rtrs_clt_sess *rtrs_clt_open(struct rtrs_clt_ops *ops,
 		if (err) {
 			list_del_rcu(&clt_path->s.entry);
 			rtrs_clt_close_conns(clt_path, true);
-			free_percpu(clt_path->stats->pcpu_stats);
-			free_path(clt_path);
+			kobject_put(&clt_path->kobj);
 			goto close_all_path;
 		}
 	}
@@ -3161,7 +3160,7 @@ int rtrs_clt_create_path_from_sysfs(struct rtrs_clt_sess *clt,
 
 	err = init_path(clt_path);
 	if (err)
-		goto close_path;
+		goto close_path_free;
 
 	err = rtrs_clt_create_path_files(clt_path);
 	if (err)
@@ -3169,12 +3168,19 @@ int rtrs_clt_create_path_from_sysfs(struct rtrs_clt_sess *clt,
 
 	return 0;
 
-close_path:
+close_path_free:
 	rtrs_clt_remove_path_from_arr(clt_path);
 	rtrs_clt_close_conns(clt_path, true);
 	free_percpu(clt_path->stats->pcpu_stats);
 	free_path(clt_path);
 
+	return err;
+
+close_path:
+	rtrs_clt_remove_path_from_arr(clt_path);
+	rtrs_clt_close_conns(clt_path, true);
+	kobject_put(&clt_path->kobj);
+
 	return err;
 }
 

base-commit: d89e1d186ec9271bf78e62771779e9fc03bf0fc3

Re: [PATCH] RDMA/rtrs: Fix potential use-after-free on client sysfs setup failure

[sashiko-bot]

> rtrs_clt_create_path_files() drops the path reference on failure, but
> its callers still close connections and free the path. This can access
> freed memory and free the path twice.
> 
> Keep the caller's path reference until its cleanup completes. Move
> per-CPU stats cleanup to the path release callback and let the embedded
> stats kobject hold a path reference until its own release callback, so
> delayed kobject release cannot outlive the enclosing allocation. Keep
> direct cleanup for failures before the path kobject is initialized.
> 
> This issue was found by our static analysis tool under development.
> 
> Fixes: 215378b838df ("RDMA/rtrs: client: sysfs interface functions")
> Cc: stable@vger.kernel.org
> Assisted-by: OpenAI:gpt-6-astra
> Signed-off-by: Guangshuo Li <liguangshuo@mail.sdu.edu.cn>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/ddd08640709b149408b616c3eec8fee847af757e.1793455791.git.liguangshuo@mail.sdu.edu.cn?part=1