[PATCH 2/3] RDMA/rtrs-srv: Balance the session device reference in sysfs setup

[Yuqiang Sun <izaiahsun@gmail.com>]

The session device is initialized in get_or_create_srv(), and the
reference that device_initialize() takes is meant to be dropped last, by
free_srv().  Each path takes one more reference, which
rtrs_srv_destroy_once_sysfs_root_folders() drops when the path goes
away.  On the failure paths of the path sysfs setup this does not add
up:

- rtrs_srv_create_once_sysfs_root_folders() calls put_device() when
  device_add() or the creation of the "paths" kobject fails, then
  unlocks srv->paths_mutex;
- rtrs_srv_create_path_files() calls
  rtrs_srv_destroy_once_sysfs_root_folders() when a later step fails,
  but the per-path reference is only taken by process_info_req() after
  rtrs_srv_create_path_files() succeeds.

Both drop the base reference, so rtrs_srv_dev_release() frees the
session while it is still locked, on ctx->srv_list and used by its
paths: the mutex_unlock() writes to freed memory, and the path's close
work later uses and puts the freed session again.  In addition,
srv->dev_ref stays incremented when the first helper fails, so the next
path of the session skips the setup although the device and the "paths"
kobject do not exist.

With one allocation failure injected into device_add() (failslab
restricted to rtrs_srv_create_path_files, into which the helper is
inlined), KASAN reports while a client connects:

  BUG: KASAN: slab-use-after-free in __mutex_unlock_slowpath
  Call Trace:
   mutex_unlock
   rtrs_srv_create_path_files
   rtrs_srv_info_req_done
   __ib_process_cq
  Freed by task 25:
   kfree
   device_release
   kobject_put
   rtrs_srv_create_path_files
   rtrs_srv_info_req_done

and with the failure injected into kobject_init_and_add() of the path
kobject instead, once the previous patch keeps srv_path alive:

  BUG: KASAN: slab-use-after-free in rtrs_srv_close_work
  Freed by task 88:
   kfree
   device_release
   kobject_put
   rtrs_srv_create_path_files
   rtrs_srv_info_req_done

This runs in the server's completion worker when a path sends its info
request.  Besides allocation failures in the sysfs setup, device_add()
also fails when the client sends an empty pathname, which the next
patch rejects.

Take the per-path reference in rtrs_srv_create_once_sysfs_root_folders()
on success, instead of in process_info_req(), so that the put in
rtrs_srv_destroy_once_sysfs_root_folders() is balanced on every path,
and do not put the device when the helper itself fails.  Undo the
dev_ref increment on every failure, including dev_set_name().  The
device is still released, together with its name, by the put_device()
in free_srv().

The problem was found while checking at runtime, with the fault
injection described above, a report of the kbound static analysis
prototype for the same function, 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) by failing,
with failslab, each allocation of dev_set_name(), device_add() and the
"paths" kobject, and the path kobject, during a client's connection:
before this series KASAN reports the use-after-free above, with this
series nothing is reported, also with CONFIG_DEBUG_KOBJECT_RELEASE.
Mapping, I/O and unmapping without fault injection still work.  Not
tested on RDMA hardware.

Fixes: baa5b28b7a47 ("RDMA/rtrs-srv: Replace device_register with device_initialize and device_add")
Fixes: e2853c49477d ("RDMA/rtrs-srv-sysfs: fix missing put_device")
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 | 21 +++++++++++++-------
 drivers/infiniband/ulp/rtrs/rtrs-srv.c       |  1 -
 2 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/drivers/infiniband/ulp/rtrs/rtrs-srv-sysfs.c b/drivers/infiniband/ulp/rtrs/rtrs-srv-sysfs.c
index 45ee07a98ca8..8de3963ca791 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-srv-sysfs.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-srv-sysfs.c
@@ -162,12 +162,12 @@ static int rtrs_srv_create_once_sysfs_root_folders(struct rtrs_srv_path *srv_pat
 		/*
 		 * Device needs to be registered only on the first session
 		 */
-		goto unlock;
+		goto get;
 	}
 	srv->dev.class = &rtrs_dev_class;
 	err = dev_set_name(&srv->dev, "%s", srv_path->s.sessname);
 	if (err)
-		goto unlock;
+		goto dec_ref;
 
 	/*
 	 * Suppress user space notification until
@@ -177,20 +177,27 @@ static int rtrs_srv_create_once_sysfs_root_folders(struct rtrs_srv_path *srv_pat
 	err = device_add(&srv->dev);
 	if (err) {
 		pr_err("device_add(): %pe\n", ERR_PTR(err));
-		put_device(&srv->dev);
-		goto unlock;
+		goto dec_ref;
 	}
 	srv->kobj_paths = kobject_create_and_add("paths", &srv->dev.kobj);
 	if (!srv->kobj_paths) {
 		err = -ENOMEM;
 		pr_err("kobject_create_and_add(): %pe\n", ERR_PTR(err));
 		device_del(&srv->dev);
-		put_device(&srv->dev);
-		goto unlock;
+		goto dec_ref;
 	}
 	dev_set_uevent_suppress(&srv->dev, false);
 	kobject_uevent(&srv->dev.kobj, KOBJ_ADD);
-unlock:
+get:
+	/* Dropped in rtrs_srv_destroy_once_sysfs_root_folders() */
+	get_device(&srv->dev);
+	mutex_unlock(&srv->paths_mutex);
+
+	return 0;
+
+dec_ref:
+	/* The reference from device_initialize() is dropped by free_srv() */
+	srv->dev_ref--;
 	mutex_unlock(&srv->paths_mutex);
 
 	return err;
diff --git a/drivers/infiniband/ulp/rtrs/rtrs-srv.c b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
index 641a7bf5e02c..56f0ab5a7619 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-srv.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
@@ -879,7 +879,6 @@ static int process_info_req(struct rtrs_srv_con *con,
 	if (err)
 		goto iu_free;
 	kobject_get(&srv_path->kobj);
-	get_device(&srv_path->srv->dev);
 	err = rtrs_srv_change_state(srv_path, RTRS_SRV_CONNECTED);
 	if (!err) {
 		rtrs_err(s, "rtrs_srv_change_state() failed\n");
-- 
2.53.0