[PATCH 1/3] RDMA/rtrs-srv: Do not free the path on path file creation failure

[Yuqiang Sun <izaiahsun@gmail.com>]

When rtrs_srv_create_path_files() fails after kobject_init_and_add() of
srv_path->kobj, its error path ends with kobject_put(&srv_path->kobj),
and the release, rtrs_srv_release(), frees srv_path.  The caller does
not expect that: process_info_req() goes on to free its tx IU through
srv_path->s.dev, and rtrs_srv_info_req_done() then closes the path,
whose close work tears down the connections, puts the session and
finally frees srv_path again in free_path().

Commit 5b7437339011 ("RDMA/rtrs: Fix use-after-free in path file
creation cleanup") reordered the cleanup inside
rtrs_srv_create_path_files() so that the helper runs before the put, but
the final put still frees srv_path under the caller.  The same pattern
exists one level down: rtrs_srv_create_stats_files() puts
srv_path->stats->kobj_stats on failure, whose release frees
srv_path->stats, which free_path() frees again.

With one allocation failure injected into kobject_init_and_add() of the
path kobject (failslab restricted to rtrs_srv_create_path_files), KASAN
reports while a client connects:

  BUG: KASAN: slab-use-after-free in rtrs_srv_info_req_done+0xe43/0x1740
  Read of size 8 by task kworker/0:1H/84
  Call Trace:
   rtrs_srv_info_req_done
   __ib_process_cq
   ib_cq_poll_work
  Freed by task 84:
   kfree
   kobject_put
   rtrs_srv_create_path_files
   rtrs_srv_info_req_done

These paths run in the server's completion worker when a client sends
its info request, and are reached when allocating or creating the sysfs
entries for the new path fails.

Keep srv_path and its stats owned by the caller on failure: the error
paths only delete what they added, and free_path(), which runs in every
case, drops the kobjects that were initialized but never made it into
sysfs, so their releases free the structures; otherwise it frees them
as before.  This keeps the kobject names from leaking, which is what the
puts in the error paths were added for.

The problem was found with kbound, a static analysis prototype that
checks whether callers rely on an object after a callee returns a value
for which the callee may have released it, on v7.2.12; it is present in
the rdma for-next branch.

Build-tested with x86_64 and arm64 allmodconfig (W=1, sparse): no new
warnings.  Run-tested under KVM (KASAN, lockdep, kmemleak), failing the
path or the stats kobject allocation with failslab during a client's
connection: before this series KASAN reports use-after-free in
rtrs_srv_info_req_done() and rtrs_srv_close_work() and a general
protection fault follows; with this series nothing is reported, also
with CONFIG_DEBUG_KOBJECT_RELEASE.  Failures of the two
sysfs_create_group() calls were not injected.  With this patch alone,
the close work still uses the session freed as described in the next
patch.  Not tested on RDMA hardware.

Fixes: 424774c9f3fa ("RDMA/rtrs: Call kobject_put in the failure path")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-5-5 kbound
Signed-off-by: Yuqiang Sun <izaiahsun@gmail.com>
---
 drivers/infiniband/ulp/rtrs/rtrs-srv-sysfs.c | 16 +++++++---------
 drivers/infiniband/ulp/rtrs/rtrs-srv.c       | 14 +++++++++++++-
 2 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/drivers/infiniband/ulp/rtrs/rtrs-srv-sysfs.c b/drivers/infiniband/ulp/rtrs/rtrs-srv-sysfs.c
index 9dd9141c86a5..45ee07a98ca8 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-srv-sysfs.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-srv-sysfs.c
@@ -238,25 +238,24 @@ static int rtrs_srv_create_stats_files(struct rtrs_srv_path *srv_path)
 				   &srv_path->kobj, "stats");
 	if (err) {
 		rtrs_err(s, "kobject_init_and_add(): %pe\n", ERR_PTR(err));
-		kobject_put(&srv_path->stats->kobj_stats);
 		return err;
 	}
 	err = sysfs_create_group(&srv_path->stats->kobj_stats,
 				 &rtrs_srv_stats_attr_group);
 	if (err) {
 		rtrs_err(s, "sysfs_create_group(): %pe\n", ERR_PTR(err));
-		goto err;
+		kobject_del(&srv_path->stats->kobj_stats);
+		return err;
 	}
 
 	return 0;
-
-err:
-	kobject_del(&srv_path->stats->kobj_stats);
-	kobject_put(&srv_path->stats->kobj_stats);
-
-	return err;
 }
 
+/*
+ * On failure, the caller still owns srv_path and frees it in free_path(),
+ * which also drops the kobjects initialized here.  Do not put them here:
+ * their releases free srv_path and srv_path->stats.
+ */
 int rtrs_srv_create_path_files(struct rtrs_srv_path *srv_path)
 {
 	struct rtrs_srv_sess *srv = srv_path->srv;
@@ -296,7 +295,6 @@ int rtrs_srv_create_path_files(struct rtrs_srv_path *srv_path)
 	kobject_del(&srv_path->kobj);
 destroy_root:
 	rtrs_srv_destroy_once_sysfs_root_folders(srv_path);
-	kobject_put(&srv_path->kobj);
 
 	return err;
 }
diff --git a/drivers/infiniband/ulp/rtrs/rtrs-srv.c b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
index 5314599d29fe..641a7bf5e02c 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-srv.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
@@ -1584,11 +1584,23 @@ static void free_path(struct rtrs_srv_path *srv_path)
 	if (srv_path->kobj.state_in_sysfs) {
 		kobject_del(&srv_path->kobj);
 		kobject_put(&srv_path->kobj);
+		return;
+	}
+
+	/*
+	 * rtrs_srv_create_path_files() may have failed after initializing
+	 * these kobjects; then their releases free the structures.
+	 */
+	if (srv_path->stats->kobj_stats.state_initialized) {
+		kobject_put(&srv_path->stats->kobj_stats);
 	} else {
 		free_percpu(srv_path->stats->rdma_stats);
 		kfree(srv_path->stats);
-		kfree(srv_path);
 	}
+	if (srv_path->kobj.state_initialized)
+		kobject_put(&srv_path->kobj);
+	else
+		kfree(srv_path);
 }
 
 static void rtrs_srv_close_work(struct work_struct *work)

base-commit: c3e8e9451e6f1b69f685b75e0fbf14f3d0fb2539
-- 
2.53.0