Thread (429 messages) 429 messages, 11 authors, 22d ago

Re: [PATCH 6.12 219/403] svcrdma: Use svc_xprt_put to free listener on create failure

From: Harshit Mogalapalli <hidden>
Date: 2026-09-05 18:25:25
Also in: linux-patches

Hi Greg/Sasha,

On 04/09/26 10:30 am, Greg Kroah-Hartman wrote:
6.12-stable review patch.  If anyone has any objections, please let me know.

------------------

From: Chuck Lever <redacted>

commit e346ef7bcb137f50c49f969330ab7dcf64ea1654 upstream.

svc_rdma_create() calls kfree(cma_xprt) when
svc_rdma_create_listen_id() fails. svc_xprt_init() has already
acquired a net namespace reference via get_net_track(); kfree
bypasses svc_xprt_free() which releases it.

Replace the kfree() with svc_xprt_put() so the kref_init birth
reference drops to zero and svc_xprt_free() dispatches
svc_rdma_free() to clean up properly. sc_cm_id is still NULL
at that point; the preceding patch added the necessary NULL
guard in svc_rdma_free().

svc_xprt_free() also drops the module reference via
module_put(), but the caller _svc_xprt_create() does the same
on xpo_create failure, double-putting the single
try_module_get() it acquired. Take a compensating
__module_get() before the svc_xprt_put() to keep the count
balanced, matching the convention in svc_rdma_accept()'s error
path.
 > Fixes: 4fb8518bdac8 ("sunrpc: Tag svc_xprt with net")> Cc: 
stable@vger.kernel.org
quoted hunk ↗ jump to hunk
Acked-by: Jeff Layton <jlayton@kernel.org>
Link: https://patch.msgid.link/20260527-rdma-follow-on-v1-3-1b09bd87b6cd@oracle.com
Signed-off-by: Chuck Lever <redacted>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
  net/sunrpc/xprtrdma/svc_rdma_transport.c |    8 +++++++-
  1 file changed, 7 insertions(+), 1 deletion(-)
--- a/net/sunrpc/xprtrdma/svc_rdma_transport.c
+++ b/net/sunrpc/xprtrdma/svc_rdma_transport.c
@@ -373,7 +373,13 @@ static struct svc_xprt *svc_rdma_create(
  
  	listen_id = svc_rdma_create_listen_id(net, sa, cma_xprt);
  	if (IS_ERR(listen_id)) {
-		kfree(cma_xprt);
+		/* _svc_xprt_create() acquired one module reference and
+		 * puts it on xpo_create failure.  svc_xprt_free() puts
+		 * a second one when the kref drops to zero.  Take a
+		 * compensating reference so both puts are balanced.
+		 */
+		__module_get(cma_xprt->sc_xprt.xpt_class->xcl_owner);
+		svc_xprt_put(&cma_xprt->sc_xprt);
  		return ERR_CAST(listen_id);
  	}
  	cma_xprt->sc_cm_id = listen_id;
^^^

I have run an AI-assisted backport review and it spotted an issue.

The posted backport calls svc_xprt_put() when listener creation fails.
At that point sc_cm_id has not been assigned.  In 6.12.y the class
free callback queues __svc_rdma_free(), whose first resource access is:

     struct ib_device *device = rdma->sc_cm_id->device;

That is a NULL dereference on the newly selected error path.
The queued free also lets svc_xprt_free() drop the module reference
before the driver work is guaranteed to run.

Upstream commit: e346ef7bcb13 ("svcrdma: Use svc_xprt_put to free
listener on create failure") was applied after commit: 4488e9129737
("svcrdma: Reorder rpcrdma_rn_unregister before rdma_destroy_id")
added the NULL guard and after commit: bf94dea7fd4e ("svcrdma:
Release transport resources synchronously") made transport teardown
synchronous.  Neither behavior is present in 6.12.y.

This needs a coherent 6.12 teardown adaptation, not the visible hunk
alone.  Without those prerequisites, I would suggest dropping this 
commit.  Thoughts?


thanks,
Harshit
  
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help