[PATCH] xprtrdma: fix race between frwr_unmap_sync() and transport disconnect

[PATCH] xprtrdma: fix race between frwr_unmap_sync() and transport disconnect

[Jorge Mora]

From: Rakesh Bhargava <Rakesh.Bhargava@netapp.com>

frwr_unmap_sync() is called from xprt_rdma_free() via ->buf_free, which
xprt_release() invokes without holding the transport send lock and
without XPRT_LOCKED. rpcrdma_xprt_disconnect() runs from xprt_autoclose()
under XPRT_LOCKED and, after draining the QP, resets every rpcrdma_req
(rpcrdma_reqs_reset) and destroys every MR (rpcrdma_mrs_destroy). Nothing
orders the two, so both can walk and list_del() the same req's
rl_registered list and the same MRs. Observed on RHEL 10.2
(6.12.0-211.7.3.el10_2) NFS/RDMA FlexFiles clients as:

  kernel BUG at lib/list_debug.c:65!
  frwr_unmap_sync+0x75 [rpcrdma]
  xprt_release+... [sunrpc]

In a vmcore, the panicking task was in frwr_unmap_sync() for r_xprt X
while another kworker was in xprt_autoclose -> rpcrdma_xprt_disconnect
-> rpcrdma_reqs_reset -> frwr_mr_release for the same r_xprt X. The
window was large because 256 transports were being torn down at once,
each blocked in mlx5_core_destroy_mkey waiting on firmware.

Other consequences of the same race are a use of ep after
rpcrdma_ep_put() frees it, and rpcrdma_flush_disconnect() dereferencing
r_xprt->rx_ep after disconnect cleared it.

Fix it by making frwr_unmap_sync() and disconnect cooperate through
rb_lock:

 - frwr_unmap_sync() reads rx_ep, checks rb_closing and that
   rl_registered is non-empty, and claims all of the req's MRs under
   rb_lock. It bumps rb_unmap_users while it still references the MRs
   and the ep, and drops it (waking rb_unmap_wq) when done.
 - If rb_closing is set, frwr_unmap_sync() returns and leaves the MRs on
   rl_registered. rb_closing is only set after rdma_disconnect(), when
   the QP is in the error state and the peer cannot reach the MRs;
   rpcrdma_reqs_reset() then releases them.
 - rpcrdma_xprt_disconnect() sets rb_closing after rdma_disconnect(),
   waits for rb_unmap_users to reach zero after draining the QP and
   before releasing reps, reqs and MRs, and clears rx_ep and rb_closing
   under rb_lock before dropping the ep reference.
 - rpcrdma_flush_disconnect() tolerates a NULL rx_ep.

Signed-off-by: Rakesh Bhargava <Rakesh.Bhargava@netapp.com>
Signed-off-by: Jorge Mora <mora@netapp.com>
---
 net/sunrpc/xprtrdma/frwr_ops.c  | 43 ++++++++++++++++++++++++++------
 net/sunrpc/xprtrdma/verbs.c     | 44 ++++++++++++++++++++++++++++++---
 net/sunrpc/xprtrdma/xprt_rdma.h | 14 +++++++++++
 3 files changed, 90 insertions(+), 11 deletions(-)

diff --git a/net/sunrpc/xprtrdma/frwr_ops.c b/net/sunrpc/xprtrdma/frwr_ops.c
index e83cef19e656..03e0e6c475b1 100644
--- a/net/sunrpc/xprtrdma/frwr_ops.c
+++ b/net/sunrpc/xprtrdma/frwr_ops.c
@@ -567,12 +567,33 @@ static void frwr_wc_localinv_wake(struct ib_cq *cq, struct ib_wc *wc)
  */
 void frwr_unmap_sync(struct rpcrdma_xprt *r_xprt, struct rpcrdma_req *req)
 {
+	struct rpcrdma_buffer *buf = &r_xprt->rx_buf;
 	struct ib_send_wr *first, **prev, *last;
-	struct rpcrdma_ep *ep = r_xprt->rx_ep;
 	const struct ib_send_wr *bad_wr;
+	struct rpcrdma_ep *ep;
 	struct rpcrdma_mr *mr;
 	int rc;
 
+	/* This function is invoked via ->buf_free, which does not
+	 * hold the transport send lock. It therefore races with
+	 * rpcrdma_xprt_disconnect(), which resets every req and
+	 * destroys every MR.
+	 *
+	 * Claim the req's MRs, and pin the transport's ep, under
+	 * rb_lock. If the disconnect path has already put the QP
+	 * into the error state, leave the MRs on rl_registered:
+	 * the peer can no longer reach them, and
+	 * rpcrdma_reqs_reset() releases them.
+	 */
+	spin_lock(&buf->rb_lock);
+	ep = r_xprt->rx_ep;
+	if (buf->rb_closing || !ep ||
+	    list_empty(&req->rl_registered)) {
+		spin_unlock(&buf->rb_lock);
+		return;
+	}
+	buf->rb_unmap_users++;
+
 	/* ORDER: Invalidate all of the MRs first
 	 *
 	 * Chain the LOCAL_INV Work Requests and post them with
@@ -598,6 +619,7 @@ void frwr_unmap_sync(struct rpcrdma_xprt *r_xprt, struct rpcrdma_req *req)
 		*prev = last;
 		prev = &last->next;
 	} while ((mr = rpcrdma_mr_pop(&req->rl_registered)));
+	spin_unlock(&buf->rb_lock);
 
 	mr = container_of(last, struct rpcrdma_mr, mr_invwr);
 
@@ -621,15 +643,22 @@ void frwr_unmap_sync(struct rpcrdma_xprt *r_xprt, struct rpcrdma_req *req)
 	 */
 	if (bad_wr != first)
 		wait_for_completion(&mr->mr_linv_done);
-	if (!rc)
-		return;
+	if (rc) {
+		/* On error, the MRs get destroyed once the QP has drained. */
+		trace_xprtrdma_post_linv_err(req, rc);
 
-	/* On error, the MRs get destroyed once the QP has drained. */
-	trace_xprtrdma_post_linv_err(req, rc);
+		/* Force a connection loss to ensure complete recovery.
+		 */
+		rpcrdma_force_disconnect(ep);
+	}
 
-	/* Force a connection loss to ensure complete recovery.
+	/* The MRs and the ep are no longer referenced. Allow
+	 * rpcrdma_xprt_disconnect() to proceed.
 	 */
-	rpcrdma_force_disconnect(ep);
+	spin_lock(&buf->rb_lock);
+	if (!--buf->rb_unmap_users)
+		wake_up(&buf->rb_unmap_wq);
+	spin_unlock(&buf->rb_lock);
 }
 
 /**
diff --git a/net/sunrpc/xprtrdma/verbs.c b/net/sunrpc/xprtrdma/verbs.c
index be335eed329d..8848cebc42b2 100644
--- a/net/sunrpc/xprtrdma/verbs.c
+++ b/net/sunrpc/xprtrdma/verbs.c
@@ -133,8 +133,14 @@ void rpcrdma_force_disconnect(struct rpcrdma_ep *ep)
  */
 void rpcrdma_flush_disconnect(struct rpcrdma_xprt *r_xprt, struct ib_wc *wc)
 {
-	if (wc->status != IB_WC_SUCCESS)
-		rpcrdma_force_disconnect(r_xprt->rx_ep);
+	struct rpcrdma_ep *ep = READ_ONCE(r_xprt->rx_ep);
+
+	/* A flushed completion can be processed after the disconnect
+	 * path has already divested the transport of its endpoint.
+	 * In that case the connection is already gone.
+	 */
+	if (wc->status != IB_WC_SUCCESS && ep)
+		rpcrdma_force_disconnect(ep);
 }
 
 /**
@@ -580,6 +586,7 @@ int rpcrdma_xprt_connect(struct rpcrdma_xprt *r_xprt)
  */
 void rpcrdma_xprt_disconnect(struct rpcrdma_xprt *r_xprt)
 {
+	struct rpcrdma_buffer *buf = &r_xprt->rx_buf;
 	struct rpcrdma_ep *ep = r_xprt->rx_ep;
 	struct rdma_cm_id *id;
 	int rc;
@@ -591,16 +598,42 @@ void rpcrdma_xprt_disconnect(struct rpcrdma_xprt *r_xprt)
 	rc = rdma_disconnect(id);
 	trace_xprtrdma_disconnect(r_xprt, rc);
 
+	/* The QP is in the error state now, so the peer can no
+	 * longer access any registered MR. From here on, a
+	 * frwr_unmap_sync() caller that has not yet claimed a req's
+	 * MRs leaves them on rl_registered: rpcrdma_reqs_reset()
+	 * releases them below.
+	 */
+	spin_lock(&buf->rb_lock);
+	buf->rb_closing = true;
+	spin_unlock(&buf->rb_lock);
+
 	rpcrdma_xprt_drain(r_xprt);
+
+	/* frwr_unmap_sync() runs without the transport send lock.
+	 * Callers that claimed MRs before rb_closing was set may
+	 * still be posting or waiting on LOCAL_INV Work Requests.
+	 * Those complete (or flush) because the QP has been
+	 * drained, but the callers still reference the MRs and the
+	 * ep, so wait for them before anything is released.
+	 */
+	wait_event(buf->rb_unmap_wq, !READ_ONCE(buf->rb_unmap_users));
+
 	rpcrdma_reps_unmap(r_xprt);
 	rpcrdma_sendctxs_destroy(r_xprt);
 	rpcrdma_reqs_reset(r_xprt);
 	rpcrdma_mrs_destroy(r_xprt);
 
+	/* Unpublish the ep before dropping the last reference to it
+	 * so that frwr_unmap_sync() never picks up a stale pointer.
+	 */
+	spin_lock(&buf->rb_lock);
+	r_xprt->rx_ep = NULL;
+	buf->rb_closing = false;
+	spin_unlock(&buf->rb_lock);
+
 	if (rpcrdma_ep_put(ep))
 		rdma_destroy_id(id);
-
-	r_xprt->rx_ep = NULL;
 }
 
 /* Fixed-size circular FIFO queue. This implementation is wait-free and
@@ -1168,6 +1201,9 @@ int rpcrdma_buffer_create(struct rpcrdma_xprt *r_xprt)
 
 	buf->rb_bc_srv_max_requests = 0;
 	spin_lock_init(&buf->rb_lock);
+	buf->rb_closing = false;
+	buf->rb_unmap_users = 0;
+	init_waitqueue_head(&buf->rb_unmap_wq);
 	INIT_LIST_HEAD(&buf->rb_mrs);
 	INIT_LIST_HEAD(&buf->rb_all_mrs);
 	INIT_WORK(&buf->rb_refresh_worker, rpcrdma_mr_refresh_worker);
diff --git a/net/sunrpc/xprtrdma/xprt_rdma.h b/net/sunrpc/xprtrdma/xprt_rdma.h
index 4cbc941e4a3e..0161317e4916 100644
--- a/net/sunrpc/xprtrdma/xprt_rdma.h
+++ b/net/sunrpc/xprtrdma/xprt_rdma.h
@@ -402,6 +402,20 @@ struct rpcrdma_buffer {
 	u32			rb_bc_max_requests;
 
 	struct work_struct	rb_refresh_worker;
+
+	/* Serializes frwr_unmap_sync(), which runs from ->buf_free
+	 * without holding the transport send lock, against
+	 * rpcrdma_xprt_disconnect(). Protected by rb_lock.
+	 *
+	 * rb_closing: the QP is in the error state and the disconnect
+	 *	path is about to reset all reqs and destroy all MRs.
+	 * rb_unmap_users: number of frwr_unmap_sync() callers that
+	 *	own MRs taken from a req's rl_registered list, or have
+	 *	LOCAL_INV Work Requests outstanding on the QP.
+	 */
+	bool			rb_closing;
+	unsigned int		rb_unmap_users;
+	wait_queue_head_t	rb_unmap_wq;
 };
 
 /*

base-commit: 18a80121193c6d19519e218434c9acf90e8d0e5c
-- 
2.47.3

Re: [PATCH] xprtrdma: fix race between frwr_unmap_sync() and transport disconnect

[Mora, Jorge]

Adding Chuck Lever.


________________________________________
From: Jorge Mora <jmora1300@gmail.com>
Sent: Saturday, October 24, 2026 10:46 AM
To: linux-nfs@vger.kernel.org
Cc: trondmy@kernel.org; anna@kernel.org; Bhargava, Rakesh; Kumar, Mohit; Mora, Jorge
Subject: [PATCH] xprtrdma: fix race between frwr_unmap_sync() and transport disconnect

EXTERNAL EMAIL - USE CAUTION when clicking links or attachments




From: Rakesh Bhargava <Rakesh.Bhargava@netapp.com>

frwr_unmap_sync() is called from xprt_rdma_free() via ->buf_free, which
xprt_release() invokes without holding the transport send lock and
without XPRT_LOCKED. rpcrdma_xprt_disconnect() runs from xprt_autoclose()
under XPRT_LOCKED and, after draining the QP, resets every rpcrdma_req
(rpcrdma_reqs_reset) and destroys every MR (rpcrdma_mrs_destroy). Nothing
orders the two, so both can walk and list_del() the same req's
rl_registered list and the same MRs. Observed on RHEL 10.2
(6.12.0-211.7.3.el10_2) NFS/RDMA FlexFiles clients as:

  kernel BUG at lib/list_debug.c:65!
  frwr_unmap_sync+0x75 [rpcrdma]
  xprt_release+... [sunrpc]

In a vmcore, the panicking task was in frwr_unmap_sync() for r_xprt X
while another kworker was in xprt_autoclose -> rpcrdma_xprt_disconnect
-> rpcrdma_reqs_reset -> frwr_mr_release for the same r_xprt X. The
window was large because 256 transports were being torn down at once,
each blocked in mlx5_core_destroy_mkey waiting on firmware.

Other consequences of the same race are a use of ep after
rpcrdma_ep_put() frees it, and rpcrdma_flush_disconnect() dereferencing
r_xprt->rx_ep after disconnect cleared it.

Fix it by making frwr_unmap_sync() and disconnect cooperate through
rb_lock:

 - frwr_unmap_sync() reads rx_ep, checks rb_closing and that
   rl_registered is non-empty, and claims all of the req's MRs under
   rb_lock. It bumps rb_unmap_users while it still references the MRs
   and the ep, and drops it (waking rb_unmap_wq) when done.
 - If rb_closing is set, frwr_unmap_sync() returns and leaves the MRs on
   rl_registered. rb_closing is only set after rdma_disconnect(), when
   the QP is in the error state and the peer cannot reach the MRs;
   rpcrdma_reqs_reset() then releases them.
 - rpcrdma_xprt_disconnect() sets rb_closing after rdma_disconnect(),
   waits for rb_unmap_users to reach zero after draining the QP and
   before releasing reps, reqs and MRs, and clears rx_ep and rb_closing
   under rb_lock before dropping the ep reference.
 - rpcrdma_flush_disconnect() tolerates a NULL rx_ep.

Signed-off-by: Rakesh Bhargava <Rakesh.Bhargava@netapp.com>
Signed-off-by: Jorge Mora <mora@netapp.com>
---
 net/sunrpc/xprtrdma/frwr_ops.c  | 43 ++++++++++++++++++++++++++------
 net/sunrpc/xprtrdma/verbs.c     | 44 ++++++++++++++++++++++++++++++---
 net/sunrpc/xprtrdma/xprt_rdma.h | 14 +++++++++++
 3 files changed, 90 insertions(+), 11 deletions(-)

diff --git a/net/sunrpc/xprtrdma/frwr_ops.c b/net/sunrpc/xprtrdma/frwr_ops.c
index e83cef19e656..03e0e6c475b1 100644
--- a/net/sunrpc/xprtrdma/frwr_ops.c
+++ b/net/sunrpc/xprtrdma/frwr_ops.c
@@ -567,12 +567,33 @@ static void frwr_wc_localinv_wake(struct ib_cq *cq, struct ib_wc *wc)
  */
 void frwr_unmap_sync(struct rpcrdma_xprt *r_xprt, struct rpcrdma_req *req)
 {
+       struct rpcrdma_buffer *buf = &r_xprt->rx_buf;
        struct ib_send_wr *first, **prev, *last;
-       struct rpcrdma_ep *ep = r_xprt->rx_ep;
        const struct ib_send_wr *bad_wr;
+       struct rpcrdma_ep *ep;
        struct rpcrdma_mr *mr;
        int rc;

+       /* This function is invoked via ->buf_free, which does not
+        * hold the transport send lock. It therefore races with
+        * rpcrdma_xprt_disconnect(), which resets every req and
+        * destroys every MR.
+        *
+        * Claim the req's MRs, and pin the transport's ep, under
+        * rb_lock. If the disconnect path has already put the QP
+        * into the error state, leave the MRs on rl_registered:
+        * the peer can no longer reach them, and
+        * rpcrdma_reqs_reset() releases them.
+        */
+       spin_lock(&buf->rb_lock);
+       ep = r_xprt->rx_ep;
+       if (buf->rb_closing || !ep ||
+           list_empty(&req->rl_registered)) {
+               spin_unlock(&buf->rb_lock);
+               return;
+       }
+       buf->rb_unmap_users++;
+
        /* ORDER: Invalidate all of the MRs first
         *
         * Chain the LOCAL_INV Work Requests and post them with
@@ -598,6 +619,7 @@ void frwr_unmap_sync(struct rpcrdma_xprt *r_xprt, struct rpcrdma_req *req)
                *prev = last;
                prev = &last->next;
        } while ((mr = rpcrdma_mr_pop(&req->rl_registered)));
+       spin_unlock(&buf->rb_lock);

        mr = container_of(last, struct rpcrdma_mr, mr_invwr);

@@ -621,15 +643,22 @@ void frwr_unmap_sync(struct rpcrdma_xprt *r_xprt, struct rpcrdma_req *req)
         */
        if (bad_wr != first)
                wait_for_completion(&mr->mr_linv_done);
-       if (!rc)
-               return;
+       if (rc) {
+               /* On error, the MRs get destroyed once the QP has drained. */
+               trace_xprtrdma_post_linv_err(req, rc);

-       /* On error, the MRs get destroyed once the QP has drained. */
-       trace_xprtrdma_post_linv_err(req, rc);
+               /* Force a connection loss to ensure complete recovery.
+                */
+               rpcrdma_force_disconnect(ep);
+       }

-       /* Force a connection loss to ensure complete recovery.
+       /* The MRs and the ep are no longer referenced. Allow
+        * rpcrdma_xprt_disconnect() to proceed.
         */
-       rpcrdma_force_disconnect(ep);
+       spin_lock(&buf->rb_lock);
+       if (!--buf->rb_unmap_users)
+               wake_up(&buf->rb_unmap_wq);
+       spin_unlock(&buf->rb_lock);
 }

 /**
diff --git a/net/sunrpc/xprtrdma/verbs.c b/net/sunrpc/xprtrdma/verbs.c
index be335eed329d..8848cebc42b2 100644
--- a/net/sunrpc/xprtrdma/verbs.c
+++ b/net/sunrpc/xprtrdma/verbs.c
@@ -133,8 +133,14 @@ void rpcrdma_force_disconnect(struct rpcrdma_ep *ep)
  */
 void rpcrdma_flush_disconnect(struct rpcrdma_xprt *r_xprt, struct ib_wc *wc)
 {
-       if (wc->status != IB_WC_SUCCESS)
-               rpcrdma_force_disconnect(r_xprt->rx_ep);
+       struct rpcrdma_ep *ep = READ_ONCE(r_xprt->rx_ep);
+
+       /* A flushed completion can be processed after the disconnect
+        * path has already divested the transport of its endpoint.
+        * In that case the connection is already gone.
+        */
+       if (wc->status != IB_WC_SUCCESS && ep)
+               rpcrdma_force_disconnect(ep);
 }

 /**
@@ -580,6 +586,7 @@ int rpcrdma_xprt_connect(struct rpcrdma_xprt *r_xprt)
  */
 void rpcrdma_xprt_disconnect(struct rpcrdma_xprt *r_xprt)
 {
+       struct rpcrdma_buffer *buf = &r_xprt->rx_buf;
        struct rpcrdma_ep *ep = r_xprt->rx_ep;
        struct rdma_cm_id *id;
        int rc;
@@ -591,16 +598,42 @@ void rpcrdma_xprt_disconnect(struct rpcrdma_xprt *r_xprt)
        rc = rdma_disconnect(id);
        trace_xprtrdma_disconnect(r_xprt, rc);

+       /* The QP is in the error state now, so the peer can no
+        * longer access any registered MR. From here on, a
+        * frwr_unmap_sync() caller that has not yet claimed a req's
+        * MRs leaves them on rl_registered: rpcrdma_reqs_reset()
+        * releases them below.
+        */
+       spin_lock(&buf->rb_lock);
+       buf->rb_closing = true;
+       spin_unlock(&buf->rb_lock);
+
        rpcrdma_xprt_drain(r_xprt);
+
+       /* frwr_unmap_sync() runs without the transport send lock.
+        * Callers that claimed MRs before rb_closing was set may
+        * still be posting or waiting on LOCAL_INV Work Requests.
+        * Those complete (or flush) because the QP has been
+        * drained, but the callers still reference the MRs and the
+        * ep, so wait for them before anything is released.
+        */
+       wait_event(buf->rb_unmap_wq, !READ_ONCE(buf->rb_unmap_users));
+
        rpcrdma_reps_unmap(r_xprt);
        rpcrdma_sendctxs_destroy(r_xprt);
        rpcrdma_reqs_reset(r_xprt);
        rpcrdma_mrs_destroy(r_xprt);

+       /* Unpublish the ep before dropping the last reference to it
+        * so that frwr_unmap_sync() never picks up a stale pointer.
+        */
+       spin_lock(&buf->rb_lock);
+       r_xprt->rx_ep = NULL;
+       buf->rb_closing = false;
+       spin_unlock(&buf->rb_lock);
+
        if (rpcrdma_ep_put(ep))
                rdma_destroy_id(id);
-
-       r_xprt->rx_ep = NULL;
 }

 /* Fixed-size circular FIFO queue. This implementation is wait-free and
@@ -1168,6 +1201,9 @@ int rpcrdma_buffer_create(struct rpcrdma_xprt *r_xprt)

        buf->rb_bc_srv_max_requests = 0;
        spin_lock_init(&buf->rb_lock);
+       buf->rb_closing = false;
+       buf->rb_unmap_users = 0;
+       init_waitqueue_head(&buf->rb_unmap_wq);
        INIT_LIST_HEAD(&buf->rb_mrs);
        INIT_LIST_HEAD(&buf->rb_all_mrs);
        INIT_WORK(&buf->rb_refresh_worker, rpcrdma_mr_refresh_worker);
diff --git a/net/sunrpc/xprtrdma/xprt_rdma.h b/net/sunrpc/xprtrdma/xprt_rdma.h
index 4cbc941e4a3e..0161317e4916 100644
--- a/net/sunrpc/xprtrdma/xprt_rdma.h
+++ b/net/sunrpc/xprtrdma/xprt_rdma.h
@@ -402,6 +402,20 @@ struct rpcrdma_buffer {
        u32                     rb_bc_max_requests;

        struct work_struct      rb_refresh_worker;
+
+       /* Serializes frwr_unmap_sync(), which runs from ->buf_free
+        * without holding the transport send lock, against
+        * rpcrdma_xprt_disconnect(). Protected by rb_lock.
+        *
+        * rb_closing: the QP is in the error state and the disconnect
+        *      path is about to reset all reqs and destroy all MRs.
+        * rb_unmap_users: number of frwr_unmap_sync() callers that
+        *      own MRs taken from a req's rl_registered list, or have
+        *      LOCAL_INV Work Requests outstanding on the QP.
+        */
+       bool                    rb_closing;
+       unsigned int            rb_unmap_users;
+       wait_queue_head_t       rb_unmap_wq;
 };

 /*

base-commit: 18a80121193c6d19519e218434c9acf90e8d0e5c
--
2.47.3

Re: [PATCH] xprtrdma: fix race between frwr_unmap_sync() and transport disconnect

[Chuck Lever]

On Sat, Oct 24, 2026, at 12:46 PM, Jorge Mora wrote:
> Other consequences of the same race are a use of ep after
> rpcrdma_ep_put() frees it, and rpcrdma_flush_disconnect() dereferencing
> r_xprt->rx_ep after disconnect cleared it.

...

>  - rpcrdma_xprt_disconnect() sets rb_closing after rdma_disconnect(),
>    waits for rb_unmap_users to reach zero after draining the QP and
>    before releasing reps, reqs and MRs, and clears rx_ep and rb_closing
>    under rb_lock before dropping the ep reference.
>  - rpcrdma_flush_disconnect() tolerates a NULL rx_ep.

Let's drop the reorder and the rpcrdma_flush_disconnect() hunk.

The only new reader of rx_ep that runs without the transport send
lock is frwr_unmap_sync(), and rb_closing already gates it. It
returns before touching the ep whenever rb_closing is set, and
rb_closing stays set until the locked section that clears rx_ep. So
keep the ep put where it is today, and clear rx_ep and rb_closing
together under rb_lock after the put. A late frwr_unmap_sync() caller
then bails on rb_closing before it can see a stale pointer.

So the rpcrdma_flush_disconnect() change addresses a problem the
patch itself introduces, not a pre-existing one. Removing both keeps
the fix to just two pieces: claiming a req's MRs under rb_lock, and
waiting for claimants before disconnect releases the MRs and the ep.
The sentence in the patch description about rpcrdma_flush_disconnect()
dereferencing a cleared rx_ep can be removed too.


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

Re: [PATCH] xprtrdma: fix race between frwr_unmap_sync() and transport disconnect

[Mora, Jorge]

Thank you Chuck.

I’ll work on version 2 and send it over as soon as it’s tested and ready.


--Jorge

________________________________________
From: Chuck Lever <cel@kernel.org>
Sent: Saturday, October 24, 2026 2:33 PM
To: Jorge Mora
Cc: linux-nfs@vger.kernel.org; trondmy@kernel.org; anna@kernel.org; Bhargava, Rakesh; Kumar, Mohit; Mora, Jorge
Subject: Re: [PATCH] xprtrdma: fix race between frwr_unmap_sync() and transport disconnect

EXTERNAL EMAIL - USE CAUTION when clicking links or attachments




On Sat, Oct 24, 2026, at 12:46 PM, Jorge Mora wrote:
> Other consequences of the same race are a use of ep after
> rpcrdma_ep_put() frees it, and rpcrdma_flush_disconnect() dereferencing
> r_xprt->rx_ep after disconnect cleared it.

...

>  - rpcrdma_xprt_disconnect() sets rb_closing after rdma_disconnect(),
>    waits for rb_unmap_users to reach zero after draining the QP and
>    before releasing reps, reqs and MRs, and clears rx_ep and rb_closing
>    under rb_lock before dropping the ep reference.
>  - rpcrdma_flush_disconnect() tolerates a NULL rx_ep.

Let's drop the reorder and the rpcrdma_flush_disconnect() hunk.

The only new reader of rx_ep that runs without the transport send
lock is frwr_unmap_sync(), and rb_closing already gates it. It
returns before touching the ep whenever rb_closing is set, and
rb_closing stays set until the locked section that clears rx_ep. So
keep the ep put where it is today, and clear rx_ep and rb_closing
together under rb_lock after the put. A late frwr_unmap_sync() caller
then bails on rb_closing before it can see a stale pointer.

So the rpcrdma_flush_disconnect() change addresses a problem the
patch itself introduces, not a pre-existing one. Removing both keeps
the fix to just two pieces: claiming a req's MRs under rb_lock, and
waiting for claimants before disconnect releases the MRs and the ep.
The sentence in the patch description about rpcrdma_flush_disconnect()
dereferencing a cleared rx_ep can be removed too.


--
Chuck Lever (Come to NFS bake-a-thon! https://urldefense.com/v3/__https://nfsv4bat.org__;!!Nhn8V6BzJA!VxUh92K-zhEMEpDyLcpzRSd3RNztFVPJieCH70PiuYNx43DJDzRIJ4rsBibjklmrLado8Qs$ )