[PATCH 0/3] rpcrdma: Fix device removal races and endpoint cleanup

[PATCH 0/3] rpcrdma: Fix device removal races and endpoint cleanup

[Li Xiasong]

This series fixes races and endpoint cleanup issues in the RPC-over-RDMA
device removal paths.

Notification registration is not serialized with device removal. Removal
can free the per-device private data while registration is still using it,
or finish walking the notification array before a new registration is
published. In addition, the notification walk does not hold references to
the objects containing the notifications, allowing concurrent unregister
and teardown to free an object before its removal callback is invoked.

On the client side, the device removal callback only forces a transport
disconnect. A connect worker waiting for re_connect_status to change can
remain asleep, preventing endpoint cleanup. An established endpoint can
also retain its connection reference because CMA suppresses subsequent
DISCONNECTED callbacks after entering the device removal state.

Patch 1 serializes notification registration with the transition to device
removal and protects access to the per-device private data.

Patch 2 uses the same mutex to serialize notification reference acquisition
with unregister, and holds a reference across each removal callback.
Callbacks and reference release run outside the mutex to allow teardown
to unregister notifications.

Patch 3 records device removal in the endpoint connection status and wakes
a waiting connect worker. For an established connection, atomic state
transitions ensure that the first path to leave the established state
releases the connection reference exactly once. It also handles connection
establishment racing with removal and propagates final endpoint release
to CMA where needed.

The patches are ordered by dependency: patch 2 uses the serialization
introduced by patch 1, and patch 3 relies on the callback lifetime
protection added by patch 2.

Li Xiasong (3):
  rpcrdma: Serialize notification registration with device removal
  rpcrdma: Pin notifications during removal callbacks
  xprtrdma: Wake connect worker on device removal

 include/linux/sunrpc/rdma_rn.h           |  6 ++-
 net/sunrpc/xprtrdma/ib_client.c          | 56 +++++++++++++++++----
 net/sunrpc/xprtrdma/svc_rdma_transport.c | 19 +++++++-
 net/sunrpc/xprtrdma/verbs.c              | 62 +++++++++++++++++++-----
 4 files changed, 119 insertions(+), 24 deletions(-)

-- 
2.34.1

[PATCH 1/3] rpcrdma: Serialize notification registration with device removal

[Li Xiasong]

rpcrdma_rn_register() checks RPCRDMA_RD_F_REMOVING before publishing the
notification and taking a reference on the rpcrdma_device. Nothing
serializes these operations with rpcrdma_remove_one(). Device removal
can therefore free the private data after the check but before xa_alloc()
or kref_get(), resulting in a use-after-free. It can also finish its
notification walk before a new registration is published, leaving the
removal path waiting for a registrant that was never notified.

Use a module-wide mutex to protect the client-data lookup, removal-state
check, notification publication and reference acquisition against the
transition to REMOVING. Clear client-data under the same mutex before
destroying the XArray and freeing the private data, so a later lookup
cannot return a stale pointer. Keep notification callbacks and completion
waits outside the critical sections.

Fixes: 7e86845a0346 ("rpcrdma: Implement generic device removal")
Signed-off-by: Li Xiasong <lixiasong1@huawei.com>
---
 net/sunrpc/xprtrdma/ib_client.c | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/net/sunrpc/xprtrdma/ib_client.c b/net/sunrpc/xprtrdma/ib_client.c
index 188f7a13397f..acd5db729539 100644
--- a/net/sunrpc/xprtrdma/ib_client.c
+++ b/net/sunrpc/xprtrdma/ib_client.c
@@ -9,6 +9,7 @@
 #include <linux/types.h>
 #include <linux/kref.h>
 #include <linux/completion.h>
+#include <linux/mutex.h>
 
 #include <linux/sunrpc/svc_rdma.h>
 #include <linux/sunrpc/rdma_rn.h>
@@ -28,6 +29,7 @@ struct rpcrdma_device {
 #define RPCRDMA_RD_F_REMOVING	(0)
 
 static struct ib_client rpcrdma_ib_client;
+static DEFINE_MUTEX(rpcrdma_device_mutex);
 
 /*
  * Listeners have no associated device, so we never register them.
@@ -61,10 +63,13 @@ int rpcrdma_rn_register(struct ib_device *device,
 			struct rpcrdma_notification *rn,
 			void (*done)(struct rpcrdma_notification *rn))
 {
-	struct rpcrdma_device *rd = rpcrdma_get_client_data(device);
+	struct rpcrdma_device *rd;
+	int ret = -ENETUNREACH;
 
+	mutex_lock(&rpcrdma_device_mutex);
+	rd = rpcrdma_get_client_data(device);
 	if (!rd || test_bit(RPCRDMA_RD_F_REMOVING, &rd->rd_flags))
-		return -ENETUNREACH;
+		goto out_unlock;
 
 	/*
 	 * Arm rn_done before xa_alloc() publishes @rn: once @rn is
@@ -78,11 +83,15 @@ int rpcrdma_rn_register(struct ib_device *device,
 	rn->rn_done = done;
 	if (xa_alloc(&rd->rd_xa, &rn->rn_index, rn, xa_limit_32b, GFP_KERNEL) < 0) {
 		rn->rn_done = NULL;
-		return -ENOMEM;
+		ret = -ENOMEM;
+		goto out_unlock;
 	}
 	kref_get(&rd->rd_kref);
 	trace_rpcrdma_client_register(device, rn);
-	return 0;
+	ret = 0;
+out_unlock:
+	mutex_unlock(&rpcrdma_device_mutex);
+	return ret;
 }
 
 static void rpcrdma_rn_release(struct kref *kref)
@@ -173,7 +182,9 @@ static void rpcrdma_remove_one(struct ib_device *device,
 
 	trace_rpcrdma_client_remove_one(device);
 
+	mutex_lock(&rpcrdma_device_mutex);
 	set_bit(RPCRDMA_RD_F_REMOVING, &rd->rd_flags);
+	mutex_unlock(&rpcrdma_device_mutex);
 	xa_for_each(&rd->rd_xa, index, rn)
 		rn->rn_done(rn);
 
@@ -187,8 +198,11 @@ static void rpcrdma_remove_one(struct ib_device *device,
 	}
 
 	trace_rpcrdma_client_remove_one_done(device);
+	mutex_lock(&rpcrdma_device_mutex);
+	ib_set_client_data(device, &rpcrdma_ib_client, NULL);
 	xa_destroy(&rd->rd_xa);
 	kfree(rd);
+	mutex_unlock(&rpcrdma_device_mutex);
 }
 
 static struct ib_client rpcrdma_ib_client = {
-- 
2.34.1

Re: [PATCH 1/3] rpcrdma: Serialize notification registration with device removal

[sashiko-bot]

> rpcrdma_rn_register() checks RPCRDMA_RD_F_REMOVING before publishing the
> notification and taking a reference on the rpcrdma_device. Nothing
> serializes these operations with rpcrdma_remove_one(). Device removal
> can therefore free the private data after the check but before xa_alloc()
> or kref_get(), resulting in a use-after-free. It can also finish its
> notification walk before a new registration is published, leaving the
> removal path waiting for a registrant that was never notified.
> 
> Use a module-wide mutex to protect the client-data lookup, removal-state
> check, notification publication and reference acquisition against the
> transition to REMOVING. Clear client-data under the same mutex before
> destroying the XArray and freeing the private data, so a later lookup
> cannot return a stale pointer. Keep notification callbacks and completion
> waits outside the critical sections.
> 
> Fixes: 7e86845a0346 ("rpcrdma: Implement generic device removal")
> Signed-off-by: Li Xiasong <lixiasong1@huawei.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261023125033.528835-1-lixiasong1@huawei.com?part=1

Re: [PATCH 1/3] rpcrdma: Serialize notification registration with device removal

[Chuck Lever]

On 10/23/26 8:50 AM, Li Xiasong wrote:
> Use a module-wide mutex to protect the client-data lookup, removal-state
> check, notification publication and reference acquisition against the
> transition to REMOVING.

I thought a mutex might be a little heavy-weight, but registration and
unregistration happen once per connection, and NVMe has used a mutex
for this purpose since 2016 without complaints.


> +	mutex_lock(&rpcrdma_device_mutex);
> +	rd = rpcrdma_get_client_data(device);

The kernel-doc comments for rpcrdma_rn_register() and
rpcrdma_rn_unregister() should now carry a "Context:" section
noting that both take rpcrdma_device_mutex and may sleep. The
unregister path is reached from rpcrdma_ep_destroy(), which can run
from the CM event handler, so callers need to be able to see that
requirement without reading the function body.

Otherwise this one looks good to me.


-- 
Chuck Lever (Come to NFS bake-a-thon! https://nfsv4bat.org)

[PATCH 2/3] rpcrdma: Pin notifications during removal callbacks

[Li Xiasong]

rpcrdma_remove_one() obtains notification pointers from rd_xa and
invokes their callbacks without preventing a concurrent unregister from
freeing the object that embeds the notification. The XArray internal RCU
protection keeps its nodes valid, but does not extend the lifetime of
stored objects.

Add get and put operations to each notification. Acquire a reference
while holding rpcrdma_device_mutex, then drop the mutex around the
callback and release the reference afterwards. Serialize unregister
with the same mutex so either removal pins the object first or unregister
removes it before the walk can find it.

Use kref_get_unless_zero() so a notification whose owner has already
entered its final release is skipped. The removal path then waits for
that owner to unregister and release the rpcrdma_device reference.

Fixes: 7e86845a0346 ("rpcrdma: Implement generic device removal")
Signed-off-by: Li Xiasong <lixiasong1@huawei.com>
---
 include/linux/sunrpc/rdma_rn.h           |  6 ++++-
 net/sunrpc/xprtrdma/ib_client.c          | 34 +++++++++++++++++++-----
 net/sunrpc/xprtrdma/svc_rdma_transport.c | 19 ++++++++++++-
 net/sunrpc/xprtrdma/verbs.c              | 21 ++++++++++++++-
 4 files changed, 71 insertions(+), 9 deletions(-)

diff --git a/include/linux/sunrpc/rdma_rn.h b/include/linux/sunrpc/rdma_rn.h
index 7d032ca057af..6122af607120 100644
--- a/include/linux/sunrpc/rdma_rn.h
+++ b/include/linux/sunrpc/rdma_rn.h
@@ -13,12 +13,16 @@
  */
 struct rpcrdma_notification {
 	void			(*rn_done)(struct rpcrdma_notification *rn);
+	bool			(*rn_get)(struct rpcrdma_notification *rn);
+	void			(*rn_put)(struct rpcrdma_notification *rn);
 	u32			rn_index;
 };
 
 int rpcrdma_rn_register(struct ib_device *device,
 			struct rpcrdma_notification *rn,
-			void (*done)(struct rpcrdma_notification *rn));
+			void (*done)(struct rpcrdma_notification *rn),
+			bool (*get)(struct rpcrdma_notification *rn),
+			void (*put)(struct rpcrdma_notification *rn));
 void rpcrdma_rn_unregister(struct ib_device *device,
 			   struct rpcrdma_notification *rn);
 int rpcrdma_ib_client_register(void);
diff --git a/net/sunrpc/xprtrdma/ib_client.c b/net/sunrpc/xprtrdma/ib_client.c
index acd5db729539..651ed6637000 100644
--- a/net/sunrpc/xprtrdma/ib_client.c
+++ b/net/sunrpc/xprtrdma/ib_client.c
@@ -48,6 +48,8 @@ static struct rpcrdma_device *rpcrdma_get_client_data(struct ib_device *device)
  * @device: device to monitor
  * @rn: notification object that wishes to be notified
  * @done: callback to notify caller of device removal
+ * @get: callback to acquire a notification object reference
+ * @put: callback to release a notification object reference
  *
  * Returns zero on success. The callback in rn_done is guaranteed
  * to be invoked when the device is removed, unless this notification
@@ -61,7 +63,9 @@ static struct rpcrdma_device *rpcrdma_get_client_data(struct ib_device *device)
  */
 int rpcrdma_rn_register(struct ib_device *device,
 			struct rpcrdma_notification *rn,
-			void (*done)(struct rpcrdma_notification *rn))
+			void (*done)(struct rpcrdma_notification *rn),
+			bool (*get)(struct rpcrdma_notification *rn),
+			void (*put)(struct rpcrdma_notification *rn))
 {
 	struct rpcrdma_device *rd;
 	int ret = -ENETUNREACH;
@@ -81,8 +85,12 @@ int rpcrdma_rn_register(struct ib_device *device,
 	 * value would unregister an @rn that was never inserted.
 	 */
 	rn->rn_done = done;
+	rn->rn_get = get;
+	rn->rn_put = put;
 	if (xa_alloc(&rd->rd_xa, &rn->rn_index, rn, xa_limit_32b, GFP_KERNEL) < 0) {
 		rn->rn_done = NULL;
+		rn->rn_get = NULL;
+		rn->rn_put = NULL;
 		ret = -ENOMEM;
 		goto out_unlock;
 	}
@@ -115,10 +123,12 @@ static void rpcrdma_rn_release(struct kref *kref)
 void rpcrdma_rn_unregister(struct ib_device *device,
 			   struct rpcrdma_notification *rn)
 {
-	struct rpcrdma_device *rd = rpcrdma_get_client_data(device);
+	struct rpcrdma_device *rd;
 
+	mutex_lock(&rpcrdma_device_mutex);
+	rd = rpcrdma_get_client_data(device);
 	if (!rd)
-		return;
+		goto out_unlock;
 
 	/*
 	 * rn_done is the registration sentinel: rpcrdma_rn_register
@@ -132,12 +142,16 @@ void rpcrdma_rn_unregister(struct ib_device *device,
 	 * rd_kref.
 	 */
 	if (!rn->rn_done)
-		return;
+		goto out_unlock;
 	rn->rn_done = NULL;
+	rn->rn_get = NULL;
+	rn->rn_put = NULL;
 
 	trace_rpcrdma_client_unregister(device, rn);
 	xa_erase(&rd->rd_xa, rn->rn_index);
 	kref_put(&rd->rd_kref, rpcrdma_rn_release);
+out_unlock:
+	mutex_unlock(&rpcrdma_device_mutex);
 }
 
 /**
@@ -178,15 +192,23 @@ static void rpcrdma_remove_one(struct ib_device *device,
 {
 	struct rpcrdma_device *rd = client_data;
 	struct rpcrdma_notification *rn;
+	void (*done)(struct rpcrdma_notification *rn);
 	unsigned long index;
 
 	trace_rpcrdma_client_remove_one(device);
 
 	mutex_lock(&rpcrdma_device_mutex);
 	set_bit(RPCRDMA_RD_F_REMOVING, &rd->rd_flags);
+	xa_for_each(&rd->rd_xa, index, rn) {
+		if (!rn->rn_get(rn))
+			continue;
+		done = rn->rn_done;
+		mutex_unlock(&rpcrdma_device_mutex);
+		done(rn);
+		rn->rn_put(rn);
+		mutex_lock(&rpcrdma_device_mutex);
+	}
 	mutex_unlock(&rpcrdma_device_mutex);
-	xa_for_each(&rd->rd_xa, index, rn)
-		rn->rn_done(rn);
 
 	/*
 	 * Wait only if there are still outstanding notification
diff --git a/net/sunrpc/xprtrdma/svc_rdma_transport.c b/net/sunrpc/xprtrdma/svc_rdma_transport.c
index 927269598ac2..93beef3a2c69 100644
--- a/net/sunrpc/xprtrdma/svc_rdma_transport.c
+++ b/net/sunrpc/xprtrdma/svc_rdma_transport.c
@@ -424,6 +424,22 @@ static void svc_rdma_xprt_done(struct rpcrdma_notification *rn)
 	svc_xprt_close(&rdma->sc_xprt);
 }
 
+static bool svc_rdma_xprt_get(struct rpcrdma_notification *rn)
+{
+	struct svcxprt_rdma *rdma = container_of(rn, struct svcxprt_rdma,
+						 sc_rn);
+
+	return kref_get_unless_zero(&rdma->sc_xprt.xpt_ref);
+}
+
+static void svc_rdma_xprt_put(struct rpcrdma_notification *rn)
+{
+	struct svcxprt_rdma *rdma = container_of(rn, struct svcxprt_rdma,
+						 sc_rn);
+
+	svc_xprt_put(&rdma->sc_xprt);
+}
+
 /*
  * This is the xpo_recvfrom function for listening endpoints. Its
  * purpose is to accept incoming connections. The CMA callback handler
@@ -464,7 +480,8 @@ static struct svc_xprt *svc_rdma_accept(struct svc_xprt *xprt)
 	dev = newxprt->sc_cm_id->device;
 	newxprt->sc_port_num = newxprt->sc_cm_id->port_num;
 
-	if (rpcrdma_rn_register(dev, &newxprt->sc_rn, svc_rdma_xprt_done))
+	if (rpcrdma_rn_register(dev, &newxprt->sc_rn, svc_rdma_xprt_done,
+				svc_rdma_xprt_get, svc_rdma_xprt_put))
 		goto errout;
 
 	newxprt->sc_max_req_size = svcrdma_max_req_size;
diff --git a/net/sunrpc/xprtrdma/verbs.c b/net/sunrpc/xprtrdma/verbs.c
index be335eed329d..9b00e71c80a7 100644
--- a/net/sunrpc/xprtrdma/verbs.c
+++ b/net/sunrpc/xprtrdma/verbs.c
@@ -297,6 +297,22 @@ static void rpcrdma_ep_removal_done(struct rpcrdma_notification *rn)
 	xprt_force_disconnect(ep->re_xprt);
 }
 
+static bool rpcrdma_ep_removal_get(struct rpcrdma_notification *rn)
+{
+	struct rpcrdma_ep *ep = container_of(rn, struct rpcrdma_ep, re_rn);
+
+	return kref_get_unless_zero(&ep->re_kref);
+}
+
+static void rpcrdma_ep_removal_put(struct rpcrdma_notification *rn)
+{
+	struct rpcrdma_ep *ep = container_of(rn, struct rpcrdma_ep, re_rn);
+	struct rdma_cm_id *id = ep->re_id;
+
+	if (rpcrdma_ep_put(ep))
+		rdma_destroy_id(id);
+}
+
 static struct rdma_cm_id *rpcrdma_create_id(struct rpcrdma_xprt *r_xprt,
 					    struct rpcrdma_ep *ep)
 {
@@ -337,7 +353,10 @@ static struct rdma_cm_id *rpcrdma_create_id(struct rpcrdma_xprt *r_xprt,
 		goto out;
 
 	ep->re_id = id;
-	rc = rpcrdma_rn_register(id->device, &ep->re_rn, rpcrdma_ep_removal_done);
+	rc = rpcrdma_rn_register(id->device, &ep->re_rn,
+				 rpcrdma_ep_removal_done,
+				 rpcrdma_ep_removal_get,
+				 rpcrdma_ep_removal_put);
 	if (rc)
 		goto out;
 
-- 
2.34.1

Re: [PATCH 2/3] rpcrdma: Pin notifications during removal callbacks

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] xprtrdma: use-after-free of ep->re_xprt in rpcrdma_ep_removal_done()
--

commit a86275f4d3e636fe6cee2fb36e6c7d9cb58af726
Author: Li Xiasong <lixiasong1@huawei.com>

rpcrdma: Pin notifications during removal callbacks

This commit adds reference-counting get and put callbacks to
rpcrdma_notification and serializes rpcrdma_rn_unregister() with
rpcrdma_device_mutex so device removal callbacks can safely run outside the
mutex.

[ ... ]

> diff --git a/net/sunrpc/xprtrdma/verbs.c b/net/sunrpc/xprtrdma/verbs.c
> index be335eed329d1..9b00e71c80a7a 100644
> --- a/net/sunrpc/xprtrdma/verbs.c
> +++ b/net/sunrpc/xprtrdma/verbs.c
> @@ -297,6 +297,22 @@ static void rpcrdma_ep_removal_done(struct rpcrdma_notification *rn)
>  	xprt_force_disconnect(ep->re_xprt);
>  }
>  
> +static bool rpcrdma_ep_removal_get(struct rpcrdma_notification *rn)
> +{
> +	struct rpcrdma_ep *ep = container_of(rn, struct rpcrdma_ep, re_rn);
> +
> +	return kref_get_unless_zero(&ep->re_kref);
> +}
> +
> +static void rpcrdma_ep_removal_put(struct rpcrdma_notification *rn)
> +{
> +	struct rpcrdma_ep *ep = container_of(rn, struct rpcrdma_ep, re_rn);
> +	struct rdma_cm_id *id = ep->re_id;
> +
> +	if (rpcrdma_ep_put(ep))
> +		rdma_destroy_id(id);
> +}

[Severity: High]
Can pinning only ep->re_kref in rpcrdma_ep_removal_get() lead to a
use-after-free of ep->re_xprt in rpcrdma_ep_removal_done()?

ep->re_xprt is an unreferenced pointer to &r_xprt->rx_xprt. When
rpcrdma_remove_one() acquires a reference on ep->re_kref via
rpcrdma_ep_removal_get() and drops rpcrdma_device_mutex before calling
rpcrdma_ep_removal_done(), a concurrent transport teardown in
xprt_destroy() -> xprt_destroy_cb() -> xprt_rdma_destroy() can run:

net/sunrpc/xprtrdma/verbs.c:rpcrdma_xprt_disconnect() {
    ...
	if (rpcrdma_ep_put(ep))
		rdma_destroy_id(id);

	r_xprt->rx_ep = NULL;
}

Because rpcrdma_remove_one() is holding the extra reference on ep->re_kref,
rpcrdma_ep_put(ep) does not reach zero and returns 0 without calling
rpcrdma_ep_destroy() or rpcrdma_rn_unregister(). rpcrdma_xprt_disconnect()
then returns immediately to xprt_rdma_destroy():

net/sunrpc/xprtrdma/transport.c:xprt_rdma_destroy() {
    ...
	rpcrdma_xprt_disconnect(r_xprt);
    ...
	xprt_rdma_free_addresses(xprt);
	xprt_free(xprt);
}

xprt_rdma_destroy() frees xprt->address_strings[] and schedules xprt for
destruction via kfree_rcu() while rpcrdma_remove_one() is executing in
sleepable process context outside any RCU read-side section. When
rpcrdma_ep_removal_done() then calls xprt_force_disconnect(ep->re_xprt), it
accesses the freed address_strings[] in trace_xprt_disconnect_force(), locks
xprt->transport_lock, and schedules xprt->task_cleanup on the freed
rpc_xprt.

The follow-up change ("xprtrdma: Wake connect worker on device removal")
still leaves this window open when rpcrdma_ep_removal_done() observes
xchg(&ep->re_connect_status, -ENODEV) == 1 and transitions
ep->re_force_disconnect from 0 to 1 via rpcrdma_force_disconnect(ep) before
rpcrdma_xprt_drain() flushes the receive queue (or when no receive WRs are
posted), as rpcrdma_xprt_disconnect() will still return with ep->re_kref > 0
while rpcrdma_force_disconnect() proceeds to call
xprt_force_disconnect(ep->re_xprt).

Should rpcrdma_ep_removal_get() also acquire a reference on ep->re_xprt (for
example via xprt_get_unless_zero(), or by ensuring transport teardown waits
for in-flight removal callbacks) before rpcrdma_ep_removal_done()
dereferences ep->re_xprt?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261023125033.528835-1-lixiasong1@huawei.com?part=2

Re: [PATCH 2/3] rpcrdma: Pin notifications during removal callbacks

[Chuck Lever]

On 10/23/26 8:50 AM, Li Xiasong wrote:
> +static bool rpcrdma_ep_removal_get(struct rpcrdma_notification *rn)
> +{
> +	struct rpcrdma_ep *ep = container_of(rn, struct rpcrdma_ep, re_rn);
> +
> +	return kref_get_unless_zero(&ep->re_kref);
> +}

Before this series the whole ep could be freed under the walk;
that's now fixed. But now that there is a get/put pair, it seems
natural to close the remaining gap here. rpcrdma_ep_removal_get()
could also take xprt_get(ep->re_xprt), and rpcrdma_ep_removal_put()
could drop it with xprt_put(). xprt_put() defers the destroy to a
work item, so dropping the last transport reference from inside the
removal walk does not recurse into xprt_rdma_destroy() while the
mutex is held.


> diff --git a/include/linux/sunrpc/rdma_rn.h b/include/linux/sunrpc/rdma_rn.h

> +	bool			(*rn_get)(struct rpcrdma_notification *rn);
> +	void			(*rn_put)(struct rpcrdma_notification *rn);

Let's add @rn_get and @rn_put entries to the struct's kdoc comment.

The kernel-doc for rpcrdma_rn_register() should also spell out the
contract for @get: it has to pin the object that embeds @rn so that
the owner cannot call rpcrdma_rn_unregister() while the pin is
held. rpcrdma_remove_one() depends on that when it calls
rn->rn_put() after dropping the mutex. Both registrants in this
patch satisfy it, since they unregister only from their final kref
release, but a future registrant has no way to know the new rule.


-- 
Chuck Lever (Come to NFS bake-a-thon! https://nfsv4bat.org)

[PATCH 3/3] xprtrdma: Wake connect worker on device removal

[Li Xiasong]

The RDMA device removal notification replaced handling of
RDMA_CM_EVENT_DEVICE_REMOVAL in the CM event handler. The removal
callback now only forces the transport to disconnect. If the connect
worker is waiting for re_connect_status to become non-zero, it cannot
release the endpoint until a CM event wakes it.

Set re_connect_status to -ENODEV and wake the connect worker when the
endpoint is not established. For an established endpoint, release its
reference from the first event that changes the connection state. RDMA
CMA does not send a later DISCONNECTED event after entering
RDMA_CM_DEVICE_REMOVAL, and ADDR_CHANGE changes the status before its
DISCONNECTED event arrives. Use atomic state transitions so these paths
release the reference only once.

Return the endpoint put result from ESTABLISHED error paths so CMA can
destroy the CM ID when the last reference is released. Initialize
re_connect_wait before registering the removal notification.

Fixes: 3f4eb9ff9234 ("xprtrdma: Handle device removal outside of the CM event handler")
Signed-off-by: Li Xiasong <lixiasong1@huawei.com>
---
 net/sunrpc/xprtrdma/verbs.c | 41 +++++++++++++++++++++++++++----------
 1 file changed, 30 insertions(+), 11 deletions(-)

diff --git a/net/sunrpc/xprtrdma/verbs.c b/net/sunrpc/xprtrdma/verbs.c
index 9b00e71c80a7..ff65bb9e28ff 100644
--- a/net/sunrpc/xprtrdma/verbs.c
+++ b/net/sunrpc/xprtrdma/verbs.c
@@ -229,6 +229,7 @@ static int
 rpcrdma_cm_event_handler(struct rdma_cm_id *id, struct rdma_cm_event *event)
 {
 	struct rpcrdma_ep *ep = id->context;
+	int status;
 
 	might_sleep();
 
@@ -251,16 +252,21 @@ rpcrdma_cm_event_handler(struct rdma_cm_id *id, struct rdma_cm_event *event)
 		case 0:
 			goto wake_connect_worker;
 		case 1:
-			/* The later DISCONNECTED event balances the
-			 * ESTABLISHED get; do not put here.
-			 */
 			rpcrdma_force_disconnect(ep);
-			return 0;
+			return rpcrdma_ep_put(ep);
 		}
 		return 0;
 	case RDMA_CM_EVENT_ESTABLISHED:
 		rpcrdma_ep_get(ep);
-		ep->re_connect_status = 1;
+		switch (cmpxchg(&ep->re_connect_status, 0, 1)) {
+		case 0:
+			break;
+		case -ENODEV:
+			rpcrdma_force_disconnect(ep);
+			return rpcrdma_ep_put(ep);
+		default:
+			return rpcrdma_ep_put(ep);
+		}
 		rpcrdma_update_cm_private(ep, &event->param.conn);
 		trace_xprtrdma_inline_thresh(ep);
 		wake_up_all(&ep->re_connect_wait);
@@ -279,9 +285,11 @@ rpcrdma_cm_event_handler(struct rdma_cm_id *id, struct rdma_cm_event *event)
 		wake_up_all(&ep->re_connect_wait);
 		return 0;
 	case RDMA_CM_EVENT_DISCONNECTED:
-		ep->re_connect_status = -ECONNABORTED;
+		status = xchg(&ep->re_connect_status, -ECONNABORTED);
 		rpcrdma_force_disconnect(ep);
-		return rpcrdma_ep_put(ep);
+		if (status == 1)
+			return rpcrdma_ep_put(ep);
+		return 0;
 	default:
 		break;
 	}
@@ -294,7 +302,18 @@ static void rpcrdma_ep_removal_done(struct rpcrdma_notification *rn)
 	struct rpcrdma_ep *ep = container_of(rn, struct rpcrdma_ep, re_rn);
 
 	trace_xprtrdma_device_removal(ep->re_id);
-	xprt_force_disconnect(ep->re_xprt);
+	switch (xchg(&ep->re_connect_status, -ENODEV)) {
+	case 0:
+		wake_up_all(&ep->re_connect_wait);
+		break;
+	case 1:
+		rpcrdma_force_disconnect(ep);
+		/* DEVICE_REMOVAL prevents a later DISCONNECTED event. */
+		rpcrdma_ep_put(ep);
+		break;
+	default:
+		break;
+	}
 }
 
 static bool rpcrdma_ep_removal_get(struct rpcrdma_notification *rn)
@@ -420,6 +439,7 @@ static int rpcrdma_ep_create(struct rpcrdma_xprt *r_xprt)
 		return -ENOTCONN;
 	ep->re_xprt = &r_xprt->rx_xprt;
 	kref_init(&ep->re_kref);
+	init_waitqueue_head(&ep->re_connect_wait);
 
 	id = rpcrdma_create_id(r_xprt, ep);
 	if (IS_ERR(id)) {
@@ -447,7 +467,6 @@ static int rpcrdma_ep_create(struct rpcrdma_xprt *r_xprt)
 
 	ep->re_send_batch = ep->re_max_requests >> 3;
 	ep->re_send_count = ep->re_send_batch;
-	init_waitqueue_head(&ep->re_connect_wait);
 
 	ep->re_attr.send_cq = ib_alloc_cq_any(device, r_xprt,
 					      ep->re_attr.cap.max_send_wr,
@@ -514,8 +533,8 @@ static int rpcrdma_ep_create(struct rpcrdma_xprt *r_xprt)
 	return 0;
 
 out_destroy:
-	rpcrdma_ep_put(ep);
-	rdma_destroy_id(id);
+	if (rpcrdma_ep_put(ep))
+		rdma_destroy_id(id);
 	return rc;
 }
 
-- 
2.34.1

Re: [PATCH 3/3] xprtrdma: Wake connect worker on device removal

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Critical] xprtrdma: use-after-free of ep in rpcrdma_cm_event_handler() on teardown
- [High] xprtrdma: ep kref leak and removal hang on UNREACHABLE or REJECTED event
- [High] xprtrdma: use-after-free or spurious disconnect of re_xprt on ESTABLISHED
--

commit ab99aeccfc2a88d68d44010bf5a53935dda9cc76
Author: Li Xiasong <lixiasong1@huawei.com>

xprtrdma: Wake connect worker on device removal

This commit updates rpcrdma_cm_event_handler() and rpcrdma_ep_removal_done()
to use atomic state transitions on ep->re_connect_status so that device
removal wakes the connect worker when not yet established and the endpoint
reference taken on RDMA_CM_EVENT_ESTABLISHED is released once on state
change.

> diff --git a/net/sunrpc/xprtrdma/verbs.c b/net/sunrpc/xprtrdma/verbs.c
> index 9b00e71c80a7a..ff65bb9e28ff6 100644
> --- a/net/sunrpc/xprtrdma/verbs.c
> +++ b/net/sunrpc/xprtrdma/verbs.c
> @@ -229,6 +229,7 @@ static int
>  rpcrdma_cm_event_handler(struct rdma_cm_id *id, struct rdma_cm_event *event)
>  {
>  	struct rpcrdma_ep *ep = id->context;
> +	int status;
>  
>  	might_sleep();
>  
> @@ -251,16 +252,21 @@ rpcrdma_cm_event_handler(struct rdma_cm_id *id, struct rdma_cm_event *event)
>  		case 0:
>  			goto wake_connect_worker;
>  		case 1:
> -			/* The later DISCONNECTED event balances the
> -			 * ESTABLISHED get; do not put here.
> -			 */
>  			rpcrdma_force_disconnect(ep);
> -			return 0;
> +			return rpcrdma_ep_put(ep);
>  		}
>  		return 0;
>  	case RDMA_CM_EVENT_ESTABLISHED:
>  		rpcrdma_ep_get(ep);
> -		ep->re_connect_status = 1;
> +		switch (cmpxchg(&ep->re_connect_status, 0, 1)) {
> +		case 0:
> +			break;
> +		case -ENODEV:
> +			rpcrdma_force_disconnect(ep);
> +			return rpcrdma_ep_put(ep);

[Severity: High]
Can calling rpcrdma_force_disconnect(ep) here cause a use-after-free on
ep->re_xprt or a spurious disconnect on a subsequent connection attempt?

When ep->re_connect_status transitions from 0 to -ENODEV in
RDMA_CM_EVENT_ADDR_CHANGE or rpcrdma_ep_removal_done(), the endpoint was
not yet established and ep->re_connect_wait is woken so that
rpcrdma_xprt_connect() returns -ENODEV to xprt_rdma_connect_worker():

net/sunrpc/xprtrdma/transport.c:xprt_rdma_connect_worker() {
	...
	rc = rpcrdma_xprt_connect(r_xprt);
	xprt_clear_connecting(xprt);
	if (!rc) {
		...
	} else
		rpcrdma_xprt_disconnect(r_xprt);
	xprt_unlock_connect(xprt, r_xprt);
}

If RDMA_CM_EVENT_ESTABLISHED races with that teardown and takes a temporary
reference via rpcrdma_ep_get(ep) before rpcrdma_xprt_disconnect() calls
rpcrdma_ep_put(ep), rpcrdma_xprt_disconnect() sees a non-zero kref, clears
r_xprt->rx_ep = NULL, and returns:

net/sunrpc/xprtrdma/verbs.c:rpcrdma_xprt_disconnect() {
	...
	if (rpcrdma_ep_put(ep))
		rdma_destroy_id(id);

	r_xprt->rx_ep = NULL;
}

Once xprt_rdma_connect_worker() finishes, a concurrent xprt_rdma_destroy()
can complete cancel_delayed_work_sync(&r_xprt->rx_connect_worker), observe
r_xprt->rx_ep == NULL, and free xprt via xprt_free(xprt) (or if not
destroyed, a new connection attempt can start on xprt) before
RDMA_CM_EVENT_ESTABLISHED executes rpcrdma_force_disconnect(ep) ->
xprt_force_disconnect(ep->re_xprt).

> +		default:
> +			return rpcrdma_ep_put(ep);
> +		}
>  		rpcrdma_update_cm_private(ep, &event->param.conn);
>  		trace_xprtrdma_inline_thresh(ep);
>  		wake_up_all(&ep->re_connect_wait);
> @@ -279,9 +285,11 @@ rpcrdma_cm_event_handler(struct rdma_cm_id *id, struct rdma_cm_event *event)
>  		wake_up_all(&ep->re_connect_wait);
>  		return 0;
>  	case RDMA_CM_EVENT_DISCONNECTED:
> -		ep->re_connect_status = -ECONNABORTED;
> +		status = xchg(&ep->re_connect_status, -ECONNABORTED);
>  		rpcrdma_force_disconnect(ep);
> -		return rpcrdma_ep_put(ep);
> +		if (status == 1)
> +			return rpcrdma_ep_put(ep);
> +		return 0;

[Severity: High]
What happens if RDMA_CM_EVENT_UNREACHABLE or RDMA_CM_EVENT_REJECTED arrives
after RDMA_CM_EVENT_ESTABLISHED has already set ep->re_connect_status to 1?

In rpcrdma_cm_event_handler(), RDMA_CM_EVENT_CONNECT_ERROR,
RDMA_CM_EVENT_UNREACHABLE, and RDMA_CM_EVENT_REJECTED still overwrite
ep->re_connect_status unconditionally without checking whether the previous
value was 1:

net/sunrpc/xprtrdma/verbs.c:rpcrdma_cm_event_handler() {
	...
	case RDMA_CM_EVENT_CONNECT_ERROR:
		ep->re_connect_status = -ENOTCONN;
		goto wake_connect_worker;
	case RDMA_CM_EVENT_UNREACHABLE:
		ep->re_connect_status = -ENETUNREACH;
		goto wake_connect_worker;
	case RDMA_CM_EVENT_REJECTED:
		ep->re_connect_status = -ECONNREFUSED;
		if (event->status == IB_CM_REJ_STALE_CONN)
			ep->re_connect_status = -ENOTCONN;
wake_connect_worker:
		wake_up_all(&ep->re_connect_wait);
		return 0;
}

In RDMA CMA, both events can be delivered on a client CM ID after
RDMA_CM_EVENT_ESTABLISHED:

1. On RoCE, rdma_resolve_route() inserts id_priv into id_table via
   cma_add_id_to_tree(), where it remains in state RDMA_CM_CONNECT until
   _destroy_id(). On a NETEVENT_NEIGH_UPDATE event,
   cma_netevent_callback() queues id_priv->id.net_work and
   cma_netevent_work_handler() delivers RDMA_CM_EVENT_UNREACHABLE unless
   the state is RDMA_CM_DESTROYING or RDMA_CM_DEVICE_REMOVAL.

2. On IB/RoCE, after cma_ib_handler() processes IB_CM_REP_RECEIVED via
   cma_rep_recv() -> ib_send_cm_rtu() and delivers
   RDMA_CM_EVENT_ESTABLISHED, cm_rej_handler() still accepts a peer REJ MAD
   while in IB_CM_ESTABLISHED (with lap_state == IB_CM_LAP_UNINIT) or
   IB_CM_DREQ_SENT, and cma_ib_handler() delivers RDMA_CM_EVENT_REJECTED.

If either event overwrites ep->re_connect_status from 1 to a negative
errno, neither that event nor a subsequent RDMA_CM_EVENT_DISCONNECTED,
RDMA_CM_EVENT_ADDR_CHANGE, or rpcrdma_ep_removal_done() will observe
status == 1.

Wouldn't that skip rpcrdma_ep_put(ep) (and skip rpcrdma_force_disconnect(ep)
in RDMA_CM_EVENT_ADDR_CHANGE and rpcrdma_ep_removal_done()), leaking
ep->re_kref and causing rpcrdma_remove_one() to hang indefinitely at
wait_for_completion(&rd->rd_done)?

>  	default:
>  		break;
>  	}
> @@ -294,7 +302,18 @@ static void rpcrdma_ep_removal_done(struct rpcrdma_notification *rn)
>  	struct rpcrdma_ep *ep = container_of(rn, struct rpcrdma_ep, re_rn);
>  
>  	trace_xprtrdma_device_removal(ep->re_id);
> -	xprt_force_disconnect(ep->re_xprt);
> +	switch (xchg(&ep->re_connect_status, -ENODEV)) {
> +	case 0:
> +		wake_up_all(&ep->re_connect_wait);
> +		break;
> +	case 1:
> +		rpcrdma_force_disconnect(ep);
> +		/* DEVICE_REMOVAL prevents a later DISCONNECTED event. */
> +		rpcrdma_ep_put(ep);
> +		break;
> +	default:
> +		break;
> +	}
>  }

[Severity: Critical]
Does dropping the ESTABLISHED reference in RDMA_CM_EVENT_ADDR_CHANGE and
rpcrdma_ep_removal_done() (or waking the connect worker on case 0 while CM
events can still fire) allow ep to be freed while rpcrdma_cm_event_handler()
is still using id->context?

In rpcrdma_xprt_disconnect(), rpcrdma_ep_put(ep) is called before
rdma_destroy_id(id):

net/sunrpc/xprtrdma/verbs.c:rpcrdma_xprt_disconnect() {
	...
	id = ep->re_id;
	rc = rdma_disconnect(id);
	trace_xprtrdma_disconnect(r_xprt, rc);

	rpcrdma_xprt_drain(r_xprt);
	rpcrdma_reps_unmap(r_xprt);
	rpcrdma_sendctxs_destroy(r_xprt);
	rpcrdma_reqs_reset(r_xprt);
	rpcrdma_mrs_destroy(r_xprt);

	if (rpcrdma_ep_put(ep))
		rdma_destroy_id(id);
}

When rpcrdma_ep_put(ep) drops the last reference on ep->re_kref,
rpcrdma_ep_destroy() runs immediately and calls kfree(ep) before
rdma_destroy_id(id) acquires id_priv->handler_mutex to synchronize with
rpcrdma_cm_event_handler().

Prior to this change, commit af9b65b29af3 ("xprtrdma: Fix ep kref imbalance
on ADDR_CHANGE") kept the ESTABLISHED reference on RDMA_CM_EVENT_ADDR_CHANGE
until RDMA_CM_EVENT_DISCONNECTED arrived. Neither RDMA_CM_EVENT_ADDR_CHANGE
nor rpcrdma_ep_removal_done() stops further CM events from firing on id:

1. RDMA_CM_EVENT_ADDR_CHANGE leaves id_priv->state as RDMA_CM_CONNECT.

2. During device removal, disable_device() invokes ib_client->remove
   callbacks in reverse registration order. Because rpcrdma_ib_client is
   registered after cma_client, rpcrdma_remove_one() runs and waits in
   wait_for_completion(&rd->rd_done) before cma_remove_one() ->
   cma_process_remove() transitions id_priv->state to
   RDMA_CM_DEVICE_REMOVAL.

Once RDMA_CM_EVENT_ADDR_CHANGE or rpcrdma_ep_removal_done() drops the
ESTABLISHED reference (or wakes the connect worker from case 0 while a
connection attempt is still in flight), rpcrdma_xprt_disconnect() calls
rdma_disconnect(id) (which triggers RDMA_CM_EVENT_DISCONNECTED), drains the
QP, and drops the final ep->re_kref reference in rpcrdma_ep_put(ep),
freeing ep before rdma_destroy_id(id) is reached.

Won't a concurrent rpcrdma_cm_event_handler() invocation (such as
RDMA_CM_EVENT_DISCONNECTED triggered by rdma_disconnect(id), or a racing
RDMA_CM_EVENT_ESTABLISHED on the case 0 path) then dereference id->context
after ep has already been freed?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261023125033.528835-1-lixiasong1@huawei.com?part=3

Re: [PATCH 3/3] xprtrdma: Wake connect worker on device removal

[Chuck Lever]

On 10/23/26 8:50 AM, Li Xiasong wrote:
>  	case RDMA_CM_EVENT_ESTABLISHED:
>  		rpcrdma_ep_get(ep);
> -		ep->re_connect_status = 1;
> +		switch (cmpxchg(&ep->re_connect_status, 0, 1)) {

>  	case RDMA_CM_EVENT_UNREACHABLE:
>  		ep->re_connect_status = -ENETUNREACH;
>  		goto wake_connect_worker;

>  	case RDMA_CM_EVENT_DISCONNECTED:
> -		ep->re_connect_status = -ECONNABORTED;
> +		status = xchg(&ep->re_connect_status, -ECONNABORTED);
>  		rpcrdma_force_disconnect(ep);
> -		return rpcrdma_ep_put(ep);
> +		if (status == 1)
> +			return rpcrdma_ep_put(ep);
> +		return 0;

UNREACHABLE can arrive on an established connection. On RoCE,
cma_netevent_work_handler() delivers RDMA_CM_EVENT_UNREACHABLE to
every cm_id whose cached neighbour hardware address changed, and it
filters only the DESTROYING and DEVICE_REMOVAL states. A connected
cm_id in RDMA_CM_CONNECT passes that filter.

When that happens, the UNREACHABLE case above overwrites status 1
with -ENETUNREACH without releasing the ESTABLISHED reference. The
later DISCONNECTED event then sees status != 1 and skips its put.
The ep and its cm_id are never freed.

Before this patch, DISCONNECTED put unconditionally, so this path
did not leak. Therefore UNREACHABLE needs the same xchg treatment as
ADDR_CHANGE: when the previous status was 1, force a disconnect and
return rpcrdma_ep_put(ep). CONNECT_ERROR and REJECTED cannot arrive
after ESTABLISHED, so those can stay as plain stores, though making
all three consistent would be easier to reason about.


> +	case 1:
> +		rpcrdma_force_disconnect(ep);
> +		/* DEVICE_REMOVAL prevents a later DISCONNECTED event. */
> +		rpcrdma_ep_put(ep);
> +		break;

I don't think this comment is accurate, and the patch description
repeats the same rationale. Leon or Jason can correct me if I'm
wrong.

ib_core removes clients in LIFO order, and xprtrdma registers its
client after rdma_cm. So rpcrdma_remove_one() runs before rdma_cm's
removal handler, and it does not return until every ep on the device
has unregistered. No xprtrdma cm_id ever reaches the
RDMA_CM_DEVICE_REMOVAL state while its ep exists. That is why the
DEVICE_REMOVAL CM event was taken out of this handler in the first
place.

After the forced disconnect here, rpcrdma_xprt_disconnect() calls
rdma_disconnect(), and the DREP or the DREQ timeout still produces
an RDMA_CM_EVENT_DISCONNECTED. The put above is correct, but only
because the DISCONNECTED case now sees -ENODEV from its xchg and
skips its own put. Someone who trusts this comment could later
remove that guard and double-put.

Let's reword both the comment and the description to state the
actual invariant: the first event that moves re_connect_status away
from 1 owns the ESTABLISHED reference.

Also, moving the ESTABLISHED put to ADDR_CHANGE and to the removal
callback means the final rpcrdma_ep_put() for a connected ep can
now happen in rpcrdma_xprt_disconnect(), which frees the ep and
only then calls rdma_destroy_id(). The DISCONNECTED event that
rdma_disconnect() provokes is dispatched while the cm_id is still
in RDMA_CM_CONNECT. If it lands between kfree(ep) and
rdma_destroy_id() taking the handler mutex,
rpcrdma_cm_event_handler() runs with id->context pointing at freed
memory.

Before this patch a connected ep's last put was always inside the
DISCONNECTED handler itself, so CMA ordered the free and the cm_id
destruction for us. The window is narrow, but I don't see what
closes it now. Am I missing something?


-- 
Chuck Lever (Come to NFS bake-a-thon! https://nfsv4bat.org)