Netdev List
 help / color / mirror / Atom feed
From: Kuniyuki Iwashima <kuniyu@amazon.com>
To: "David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Allison Henderson <allison.henderson@oracle.com>
Cc: Sowmini Varadhan <sowmini.varadhan@oracle.com>,
	Kuniyuki Iwashima <kuniyu@amazon.com>,
	Kuniyuki Iwashima <kuni1840@gmail.com>, <netdev@vger.kernel.org>,
	<linux-rdma@vger.kernel.org>, <rds-devel@oss.oracle.com>
Subject: [PATCH v1 net 1/2] Revert "tcp: Clean up kernel listener's reqsk in inet_twsk_purge()"
Date: Fri, 23 Feb 2024 09:24:47 -0800	[thread overview]
Message-ID: <20240223172448.94084-2-kuniyu@amazon.com> (raw)
In-Reply-To: <20240223172448.94084-1-kuniyu@amazon.com>

This reverts commit 740ea3c4a0b2e326b23d7cdf05472a0e92aa39bc.

The change actually fixed a use-after-free of struct net by kernel
listener's reqsk in per-netns ehash.

However, the fix was incomplete, as the same issue exists for the
global ehash.

We should have fixed it on the RDS side without slowing down netns
dismantle for the normal TCP use case.

The next patch fixes the issue on the RDS side.

Fixes: 740ea3c4a0b2 ("tcp: Clean up kernel listener's reqsk in inet_twsk_purge()")
Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
---
 net/ipv4/inet_timewait_sock.c | 15 +--------------
 net/ipv4/tcp_minisocks.c      |  9 ++++-----
 2 files changed, 5 insertions(+), 19 deletions(-)

diff --git a/net/ipv4/inet_timewait_sock.c b/net/ipv4/inet_timewait_sock.c
index 5befa4de5b24..e7a1698c6b22 100644
--- a/net/ipv4/inet_timewait_sock.c
+++ b/net/ipv4/inet_timewait_sock.c
@@ -277,21 +277,8 @@ void inet_twsk_purge(struct inet_hashinfo *hashinfo, int family)
 		rcu_read_lock();
 restart:
 		sk_nulls_for_each_rcu(sk, node, &head->chain) {
-			if (sk->sk_state != TCP_TIME_WAIT) {
-				/* A kernel listener socket might not hold refcnt for net,
-				 * so reqsk_timer_handler() could be fired after net is
-				 * freed.  Userspace listener and reqsk never exist here.
-				 */
-				if (unlikely(sk->sk_state == TCP_NEW_SYN_RECV &&
-					     hashinfo->pernet)) {
-					struct request_sock *req = inet_reqsk(sk);
-
-					inet_csk_reqsk_queue_drop_and_put(req->rsk_listener, req);
-				}
-
+			if (sk->sk_state != TCP_TIME_WAIT)
 				continue;
-			}
-
 			tw = inet_twsk(sk);
 			if ((tw->tw_family != family) ||
 				refcount_read(&twsk_net(tw)->ns.count))
diff --git a/net/ipv4/tcp_minisocks.c b/net/ipv4/tcp_minisocks.c
index 9e85f2a0bddd..baecfa4c70ef 100644
--- a/net/ipv4/tcp_minisocks.c
+++ b/net/ipv4/tcp_minisocks.c
@@ -394,14 +394,13 @@ void tcp_twsk_purge(struct list_head *net_exit_list, int family)
 	struct net *net;
 
 	list_for_each_entry(net, net_exit_list, exit_list) {
+		/* The last refcount is decremented in tcp_sk_exit_batch() */
+		if (refcount_read(&net->ipv4.tcp_death_row.tw_refcount) == 1)
+			continue;
+
 		if (net->ipv4.tcp_death_row.hashinfo->pernet) {
-			/* Even if tw_refcount == 1, we must clean up kernel reqsk */
 			inet_twsk_purge(net->ipv4.tcp_death_row.hashinfo, family);
 		} else if (!purged_once) {
-			/* The last refcount is decremented in tcp_sk_exit_batch() */
-			if (refcount_read(&net->ipv4.tcp_death_row.tw_refcount) == 1)
-				continue;
-
 			inet_twsk_purge(&tcp_hashinfo, family);
 			purged_once = true;
 		}
-- 
2.30.2


  reply	other threads:[~2024-02-23 17:25 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-23 17:24 [PATCH v1 net 0/2] rds: Fix use-after-free of net by tcp reqsk timer Kuniyuki Iwashima
2024-02-23 17:24 ` Kuniyuki Iwashima [this message]
2024-02-23 17:24 ` [PATCH v1 net 2/2] rds: tcp: Fix use-after-free of net in reqsk_timer_handler() Kuniyuki Iwashima
2024-02-23 18:09   ` Eric Dumazet
2024-02-23 18:28     ` Kuniyuki Iwashima
2024-02-26 19:14       ` Kuniyuki Iwashima
2024-02-26 19:22         ` Allison Henderson
2024-02-26 19:38           ` Kuniyuki Iwashima
2024-02-26 19:52             ` Eric Dumazet
2024-02-26 20:00               ` Kuniyuki Iwashima
2024-02-26 19:40           ` Eric Dumazet
2024-02-26 19:48             ` Kuniyuki Iwashima
2024-02-26 20:00               ` Eric Dumazet
2024-02-26 20:05                 ` Kuniyuki Iwashima

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=20240223172448.94084-2-kuniyu@amazon.com \
    --to=kuniyu@amazon.com \
    --cc=allison.henderson@oracle.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=kuba@kernel.org \
    --cc=kuni1840@gmail.com \
    --cc=linux-rdma@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=rds-devel@oss.oracle.com \
    --cc=sowmini.varadhan@oracle.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