All of lore.kernel.org
 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 v6 02/12] net/rds: undo conn_alloc() the same way on every __rds_conn_create() exit
Date: Tue, 22 Sep 2026 01:54:00 -0700	[thread overview]
Message-ID: <20260922085410.391323-3-achender@kernel.org> (raw)
In-Reply-To: <20260922085410.391323-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 b6c4beb50eaf..a96569a3ee9a 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
@@ -316,7 +332,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;
@@ -332,18 +348,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-22  8:54 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-22  8:53 [PATCH net-next v6 00/12] net/rds: make connection lifetime reference-counted Allison Henderson
2026-09-22  8:53 ` [PATCH net-next v6 01/12] net/rds: ib: don't enable interrupts in rds_ib_conn_free() Allison Henderson
2026-09-23  8:54   ` sashiko-bot
2026-09-22  8:54 ` Allison Henderson [this message]
2026-09-23  8:54   ` [PATCH net-next v6 02/12] net/rds: undo conn_alloc() the same way on every __rds_conn_create() exit sashiko-bot
2026-09-22  8:54 ` [PATCH net-next v6 03/12] net/rds: guard every work-requeueing site with rds_destroy_pending() Allison Henderson
2026-09-23  8:54   ` sashiko-bot
2026-09-22  8:54 ` [PATCH net-next v6 04/12] net/rds: make rds_destroy_pending() report a connection's own destroy Allison Henderson
2026-09-23  8:54   ` sashiko-bot
2026-09-26  9:48   ` netdev-bot+sashiko
2026-09-22  8:54 ` [PATCH net-next v6 05/12] net/rds: split connection destroy into quiesce and kref-governed free Allison Henderson
2026-09-23  8:54   ` sashiko-bot
2026-09-26  9:48   ` netdev-bot+sashiko
2026-09-22  8:54 ` [PATCH net-next v6 06/12] net/rds: wait for connections to be freed on transport unload Allison Henderson
2026-09-23  8:54   ` sashiko-bot
2026-09-26  9:48   ` netdev-bot+sashiko
2026-09-22  8:54 ` [PATCH net-next v6 07/12] net/rds: unlink transport nodes before a possibly deferred connection free Allison Henderson
2026-09-23  8:54   ` sashiko-bot
2026-09-26  9:48   ` netdev-bot+sashiko
2026-09-22  8:54 ` [PATCH net-next v6 08/12] net/rds: hold connection references in lookup, sockets and c_passive Allison Henderson
2026-09-23  8:54   ` sashiko-bot
2026-09-26  9:48   ` netdev-bot+sashiko
2026-09-22  8:54 ` [PATCH net-next v6 09/12] net/rds: take cp_lock to purge cp_send_queue in the quiesce Allison Henderson
2026-09-23  8:54   ` sashiko-bot
2026-09-26  9:48   ` netdev-bot+sashiko
2026-09-22  8:54 ` [PATCH net-next v6 10/12] net/rds: pin the connection across RDMA-CM event handling Allison Henderson
2026-09-23  8:54   ` sashiko-bot
2026-09-26  9:48   ` netdev-bot+sashiko
2026-09-22  8:54 ` [PATCH net-next v6 11/12] net/rds: drop rds_conn_count in favor of t_conn_count Allison Henderson
2026-09-23  8:54   ` sashiko-bot
2026-09-22  8:54 ` [PATCH net-next v6 12/12] net/rds: hold a connection reference from struct rds_incoming Allison Henderson
2026-09-23  8:54   ` sashiko-bot
  -- strict thread matches above, loose matches on Subject: below --
2026-09-22 16:43 [PATCH net-next v6 00/12] net/rds: make connection lifetime reference-counted Allison Henderson
2026-09-22 16:43 ` [PATCH net-next v6 02/12] net/rds: undo conn_alloc() the same way on every __rds_conn_create() exit 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=20260922085410.391323-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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.