Linux RDMA and InfiniBand development
 help / color / mirror / Atom feed
From: Allison Henderson <achender@kernel.org>
To: netdev@vger.kernel.org, linux-rdma@vger.kernel.org,
	pabeni@redhat.com, edumazet@google.com, kuba@kernel.org,
	horms@kernel.org
Cc: achender@kernel.org
Subject: [PATCH net-next v7 02/12] net/rds: undo conn_alloc() the same way on every __rds_conn_create() exit
Date: Sat, 26 Sep 2026 23:14:38 -0700	[thread overview]
Message-ID: <20260927061448.167862-3-achender@kernel.org> (raw)
In-Reply-To: <20260927061448.167862-1-achender@kernel.org>

trans->conn_alloc() may allocate transport data for every path of a
multipath connection - rds_tcp_conn_alloc() does - which is why the
lost-creation-race exit of __rds_conn_create() loops over all npaths
when it frees the connection it just built.  The passive-connection
exit right above it frees only path 0.

That is not a leak today: a passive twin is only created for an IB
loopback connection (an incoming TCP connect to a local address is
refused with -EOPNOTSUPP before it gets here), and the IB transport is
not multipath, so npaths is 1 on that exit.  But the two exits express
the same "undo conn_alloc()" step in two different ways, and the
following patches add another exit of the same kind.  Move the loop
into a helper and use it everywhere, so that the step cannot silently
diverge if a multipath transport ever grows a passive twin.

Assisted-by: Claude-Code:claude-fable-5
Signed-off-by: Allison Henderson <achender@kernel.org>
---
 net/rds/connection.c | 31 ++++++++++++++++++-------------
 1 file changed, 18 insertions(+), 13 deletions(-)

diff --git a/net/rds/connection.c b/net/rds/connection.c
index c752a8623cfc..95ff50f31d1e 100644
--- a/net/rds/connection.c
+++ b/net/rds/connection.c
@@ -161,6 +161,22 @@ static void __rds_conn_path_init(struct rds_connection *conn,
 	cp->cp_flags = 0;
 }
 
+/* Undo trans->conn_alloc(): it may have allocated transport data for
+ * every path of a multipath connection, not just for path 0.
+ */
+static void rds_conn_free_transport_data(struct rds_connection *conn,
+					 int npaths)
+{
+	struct rds_conn_path *cp;
+	int i;
+
+	for (i = 0; i < npaths; i++) {
+		cp = &conn->c_path[i];
+		if (cp->cp_transport_data)
+			conn->c_trans->conn_free(cp->cp_transport_data);
+	}
+}
+
 /*
  * There is only every one 'conn' for a given pair of addresses in the
  * system at a time.  They contain messages to be retransmitted and so
@@ -322,7 +338,7 @@ static struct rds_connection *__rds_conn_create(struct net *net,
 	if (parent) {
 		/* Creating passive conn */
 		if (parent->c_passive) {
-			trans->conn_free(conn->c_path[0].cp_transport_data);
+			rds_conn_free_transport_data(conn, npaths);
 			free_cp = conn->c_path;
 			kmem_cache_free(rds_conn_slab, conn);
 			conn = parent->c_passive;
@@ -338,18 +354,7 @@ static struct rds_connection *__rds_conn_create(struct net *net,
 		found = rds_conn_lookup(net, head, laddr, faddr, trans,
 					tos, dev_if);
 		if (found) {
-			struct rds_conn_path *cp;
-			int i;
-
-			for (i = 0; i < npaths; i++) {
-				cp = &conn->c_path[i];
-				/* The ->conn_alloc invocation may have
-				 * allocated resource for all paths, so all
-				 * of them may have to be freed here.
-				 */
-				if (cp->cp_transport_data)
-					trans->conn_free(cp->cp_transport_data);
-			}
+			rds_conn_free_transport_data(conn, npaths);
 			free_cp = conn->c_path;
 			kmem_cache_free(rds_conn_slab, conn);
 			conn = found;
-- 
2.25.1


  parent reply	other threads:[~2026-09-27  6:14 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-27  6:14 [PATCH net-next v7 00/12] net/rds: make connection lifetime reference-counted Allison Henderson
2026-09-27  6:14 ` [PATCH net-next v7 01/12] net/rds: ib: don't enable interrupts in rds_ib_conn_free() Allison Henderson
2026-09-28  6:14   ` sashiko-bot
2026-09-27  6:14 ` Allison Henderson [this message]
2026-09-28  6:15   ` [PATCH net-next v7 02/12] net/rds: undo conn_alloc() the same way on every __rds_conn_create() exit sashiko-bot
2026-09-27  6:14 ` [PATCH net-next v7 03/12] net/rds: guard every work-requeueing site with rds_destroy_pending() Allison Henderson
2026-09-28  6:15   ` sashiko-bot
2026-10-01  6:16   ` netdev-bot+sashiko
2026-09-27  6:14 ` [PATCH net-next v7 04/12] net/rds: make rds_destroy_pending() report a connection's own destroy Allison Henderson
2026-09-28  6:15   ` sashiko-bot
2026-10-01  6:16   ` netdev-bot+sashiko
2026-09-27  6:14 ` [PATCH net-next v7 05/12] net/rds: split connection destroy into quiesce and kref-governed free Allison Henderson
2026-09-28  6:15   ` sashiko-bot
2026-10-01  6:16   ` netdev-bot+sashiko
2026-09-27  6:14 ` [PATCH net-next v7 06/12] net/rds: wait for connections to be freed on transport unload Allison Henderson
2026-09-28  6:15   ` sashiko-bot
2026-10-01  6:16   ` netdev-bot+sashiko
2026-09-27  6:14 ` [PATCH net-next v7 07/12] net/rds: unlink transport nodes before a possibly deferred connection free Allison Henderson
2026-09-28  6:15   ` sashiko-bot
2026-10-01  6:16   ` netdev-bot+sashiko
2026-09-27  6:14 ` [PATCH net-next v7 08/12] net/rds: hold a connection reference from struct rds_incoming Allison Henderson
2026-09-28  6:15   ` sashiko-bot
2026-10-01  6:16   ` netdev-bot+sashiko
2026-09-27  6:14 ` [PATCH net-next v7 09/12] net/rds: take cp_lock to purge cp_send_queue in the quiesce Allison Henderson
2026-09-28  6:15   ` sashiko-bot
2026-10-01  6:16   ` netdev-bot+sashiko
2026-09-27  6:14 ` [PATCH net-next v7 10/12] net/rds: hold connection references in lookup, sockets and c_passive Allison Henderson
2026-09-28  6:15   ` sashiko-bot
2026-10-01  6:16   ` netdev-bot+sashiko
2026-09-27  6:14 ` [PATCH net-next v7 11/12] net/rds: pin the connection across RDMA-CM event handling Allison Henderson
2026-09-28  6:15   ` sashiko-bot
2026-10-01  6:16   ` netdev-bot+sashiko
2026-09-27  6:14 ` [PATCH net-next v7 12/12] net/rds: drop rds_conn_count in favor of t_conn_count Allison Henderson
2026-09-28  6:15   ` sashiko-bot
2026-10-01  6:16   ` netdev-bot+sashiko
2026-10-02 19:37 ` [PATCH net-next v7 00/12] net/rds: make connection lifetime reference-counted Jakub Kicinski
2026-10-02 21:26   ` Allison Henderson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260927061448.167862-3-achender@kernel.org \
    --to=achender@kernel.org \
    --cc=edumazet@google.com \
    --cc=horms@kernel.org \
    --cc=kuba@kernel.org \
    --cc=linux-rdma@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox