netdev - [PATCH net-next] tcp: do not allow to connect with the four-tuple symmetry socket

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <20240818042538.40195-1-kerneljasonxing@gmail.com>
Date: Sun, 18 Aug 2024 12:25:38 +0800
From: Jason Xing <kerneljasonxing@...il.com>
To: davem@...emloft.net,
	edumazet@...gle.com,
	kuba@...nel.org,
	pabeni@...hat.com,
	dsahern@...nel.org,
	ncardwell@...gle.com
Cc: netdev@...r.kernel.org,
	Jason Xing <kernelxing@...cent.com>
Subject: [PATCH net-next] tcp: do not allow to connect with the four-tuple symmetry socket

From: Jason Xing <kernelxing@...cent.com>

Four-tuple symmetry here means the socket has the same remote/local
port and ipaddr, like this, 127.0.0.1:8000 -> 127.0.0.1:8000.
$ ss -nat | grep 8000
ESTAB      0      0          127.0.0.1:8000       127.0.0.1:8000

Before this patch, one client could start a connection successfully
as above even without a listener, which means, the socket connects
to its self. Then every time other threads trying to bind/listen on
this port will encounter a failure surely, unless the thread owning
the socket exits.

It can rarely happen on the loopback device when the connect() finds
the same port as its remote port while listener is not running. It
has the side-effect on other threads. Besides, this solo flow has no
merit, no significance at all.

After this patch, the moment we try to connect with a 4-tuple symmetry
socket, we will get an error "connect: Cannot assign requested address".

Signed-off-by: Jason Xing <kernelxing@...cent.com>
---
 net/ipv4/inet_hashtables.c | 31 +++++++++++++++++++++++++++----
 1 file changed, 27 insertions(+), 4 deletions(-)

diff --git a/net/ipv4/inet_hashtables.c b/net/ipv4/inet_hashtables.c
index 9bfcfd016e18..2f8f34ee62fb 100644
--- a/net/ipv4/inet_hashtables.c
+++ b/net/ipv4/inet_hashtables.c
@@ -978,6 +978,21 @@ void inet_bhash2_reset_saddr(struct sock *sk)
 }
 EXPORT_SYMBOL_GPL(inet_bhash2_reset_saddr);
 
+/* SYMMETRY means the socket has the same local and remote port/ipaddr */
+#define INET_ADDR_SYMMETRY(sk) (inet_sk(sk)->inet_rcv_saddr == \
+				inet_sk(sk)->inet_daddr)
+#define INET_PORT_SYMMETRY(sk) (inet_sk(sk)->inet_num == \
+				ntohs(inet_sk(sk)->inet_dport))
+#define INET_PORT_SYMMETRY_MATCH(sk, port) (port == \
+					    ntohs(inet_sk(sk)->inet_dport))
+static inline int inet_tuple_symmetry(struct sock *sk)
+{
+	if (INET_ADDR_SYMMETRY(sk) && INET_PORT_SYMMETRY(sk))
+		return -EADDRNOTAVAIL;
+
+	return 0;
+}
+
 /* RFC 6056 3.3.4.  Algorithm 4: Double-Hash Port Selection Algorithm
  * Note that we use 32bit integers (vs RFC 'short integers')
  * because 2^16 is not a multiple of num_ephemeral and this
@@ -997,13 +1012,13 @@ int __inet_hash_connect(struct inet_timewait_death_row *death_row,
 			struct sock *, __u16, struct inet_timewait_sock **))
 {
 	struct inet_hashinfo *hinfo = death_row->hashinfo;
+	bool tb_created = false, symmetry_test = false;
 	struct inet_bind_hashbucket *head, *head2;
 	struct inet_timewait_sock *tw = NULL;
 	int port = inet_sk(sk)->inet_num;
 	struct net *net = sock_net(sk);
 	struct inet_bind2_bucket *tb2;
 	struct inet_bind_bucket *tb;
-	bool tb_created = false;
 	u32 remaining, offset;
 	int ret, i, low, high;
 	bool local_ports;
@@ -1011,12 +1026,18 @@ int __inet_hash_connect(struct inet_timewait_death_row *death_row,
 	u32 index;
 
 	if (port) {
-		local_bh_disable();
-		ret = check_established(death_row, sk, port, NULL);
-		local_bh_enable();
+		ret = inet_tuple_symmetry(sk);
+		if (!ret) {
+			local_bh_disable();
+			ret = check_established(death_row, sk, port, NULL);
+			local_bh_enable();
+		}
 		return ret;
 	}
 
+	if (INET_ADDR_SYMMETRY(sk))
+		symmetry_test = true;
+
 	l3mdev = inet_sk_bound_l3mdev(sk);
 
 	local_ports = inet_sk_get_local_port_range(sk, &low, &high);
@@ -1046,6 +1067,8 @@ int __inet_hash_connect(struct inet_timewait_death_row *death_row,
 			port -= remaining;
 		if (inet_is_local_reserved_port(net, port))
 			continue;
+		if (symmetry_test && INET_PORT_SYMMETRY_MATCH(sk, port))
+			continue;
 		head = &hinfo->bhash[inet_bhashfn(net, port,
 						  hinfo->bhash_size)];
 		spin_lock_bh(&head->lock);
-- 
2.37.3