netdev - [PATCH net-next 1/2] datagram: Rehash sockets only if local address changed for their family

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-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20241204221254.3537932-2-sbrivio@redhat.com>
Date: Wed,  4 Dec 2024 23:12:53 +0100
From: Stefano Brivio <sbrivio@...hat.com>
To: Willem de Bruijn <willemdebruijn.kernel@...il.com>
Cc: Eric Dumazet <edumazet@...gle.com>,
	netdev@...r.kernel.org,
	Kuniyuki Iwashima <kuniyu@...zon.com>,
	Mike Manning <mvrmanning@...il.com>,
	David Gibson <david@...son.dropbear.id.au>,
	Paul Holzinger <pholzing@...hat.com>,
	Philo Lu <lulie@...ux.alibaba.com>,
	Cambda Zhu <cambda@...ux.alibaba.com>,
	Fred Chen <fred.cc@...baba-inc.com>,
	Yubing Qiu <yubing.qiuyubing@...baba-inc.com>
Subject: [PATCH net-next 1/2] datagram: Rehash sockets only if local address changed for their family

It makes no sense to rehash an IPv4 socket when we change
sk_v6_rcv_saddr, or to rehash an IPv6 socket as inet_rcv_saddr is set:
the secondary hash (including the local address) won't change, because
ipv4_portaddr_hash() and ipv6_portaddr_hash() only take the address
matching the socket family.

Currently, dual-stack AF_INET6 sockets (without IPV6_V6ONLY) are
redundantly rehashed if connect() is issued on them with an IPv4
address: __ip6_datagram_connect() calls __ip4_datagram_connect().
This case can be reproduced with:

  socat UDP6-LISTEN:1337,null-eof STDOUT & { sleep 1; : | socat STDIN UDP4:0:1337,shut-null; }

and, in general, the assumptions __ip4_datagram_connect() <-> AF_INET
__ip6_datagram_connect() <-> AF_INET6 don't necessarily hold.

This is a mere optimisation at the moment, but, in the next patch,
we'll change the rehash operation into an operation that also sets the
receive address, and we want this new operation to be called only with
addresses corresponding to the socket family, for simplicity.

v1:
  - explain in which case sk_family isn't AF_INET, in
    __ip4_datagram_connect() (Willem de Bruijn)
  - rebase onto net-next

Signed-off-by: Stefano Brivio <sbrivio@...hat.com>
---
 net/ipv4/datagram.c | 2 +-
 net/ipv6/datagram.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/datagram.c b/net/ipv4/datagram.c
index cc6d0bd7b0a9..d52333e921f3 100644
--- a/net/ipv4/datagram.c
+++ b/net/ipv4/datagram.c
@@ -65,7 +65,7 @@ int __ip4_datagram_connect(struct sock *sk, struct sockaddr *uaddr, int addr_len
 		inet->inet_saddr = fl4->saddr;	/* Update source address */
 	if (!inet->inet_rcv_saddr) {
 		inet->inet_rcv_saddr = fl4->saddr;
-		if (sk->sk_prot->rehash)
+		if (sk->sk_prot->rehash && sk->sk_family == AF_INET)
 			sk->sk_prot->rehash(sk);
 	}
 	inet->inet_daddr = fl4->daddr;
diff --git a/net/ipv6/datagram.c b/net/ipv6/datagram.c
index fff78496803d..5c28a11128c7 100644
--- a/net/ipv6/datagram.c
+++ b/net/ipv6/datagram.c
@@ -211,7 +211,7 @@ int __ip6_datagram_connect(struct sock *sk, struct sockaddr *uaddr,
 		    ipv6_mapped_addr_any(&sk->sk_v6_rcv_saddr)) {
 			ipv6_addr_set_v4mapped(inet->inet_rcv_saddr,
 					       &sk->sk_v6_rcv_saddr);
-			if (sk->sk_prot->rehash)
+			if (sk->sk_prot->rehash && sk->sk_family == AF_INET6)
 				sk->sk_prot->rehash(sk);
 		}

-- 
2.40.1