[PATCH net v2 0/2] tcp: Fix the SO_REUSEPORT bind cache admitting a conflicting socket

[PATCH net v2 0/2] tcp: Fix the SO_REUSEPORT bind cache admitting a conflicting socket

[Juan Agustin Niccolini]

The SO_REUSEPORT bind cache (tb->fast_*) can let a socket skip a bind
conflict check that it would fail, so an unprivileged user can bind and
listen on a port that another user is listening on.

Patch 1 fixes it. Patch 2 adds the reproducer as a selftest.

v2:
 - Shorten the changelog (Eric Dumazet)
 - Add reproducer as selftest (Eric Dumazet)
v1: https://lore.kernel.org/all/20261110080008.26112-1-juanagusnicco@gmail.com/

Juan Agustin Niccolini (2):
  tcp: Fix the SO_REUSEPORT bind cache admitting a conflicting socket
  selftests: net: Add SO_REUSEPORT bind cache test to bind_wildcard.c

 include/net/inet_hashtables.h               |  2 +
 net/ipv4/inet_connection_sock.c             | 15 ++++--
 tools/testing/selftests/net/bind_wildcard.c | 54 +++++++++++++++++++++
 3 files changed, 68 insertions(+), 3 deletions(-)


base-commit: 19a227903e65c7a9c179156485660a4e017575fe
-- 
2.43.0

[PATCH net v2 1/2] tcp: Fix the SO_REUSEPORT bind cache admitting a conflicting socket

[Juan Agustin Niccolini]

When the port is in the FASTREUSEPORT_STRICT state, inet_csk_get_port()
skips the bind conflict check if sk_reuseport_match() finds the new
socket's address covered by the socket cached in tb->fast_*.

This is only safe if both sockets match on everything
inet_bind_conflict() reads. The cache misses some of it:

- It does not record sk_bound_dev_if or sk_reuse.
- It records the family and IPV6_V6ONLY but never compares them with
  the new socket, so a cached v6only [::] (or 0.0.0.0) socket covers
  a dual-stack [::] socket.

As a result, an unprivileged user can bind and listen on a port that
another user is listening on, for example a dual-stack [::] socket on
top of a 0.0.0.0 listener:

  victim:   bind(0.0.0.0:P), listen()
  attacker: SO_REUSEPORT + IPV6_V6ONLY, bind([::]:P)   /* fills cache */
  attacker: SO_REUSEPORT, bind([::]:P), listen()       /* should fail */

Only take the fast path when the new socket matches the cached one in
family, IPV6_V6ONLY, bound device and SO_REUSEADDR. Identical
SO_REUSEPORT listeners, the workload this cache was added for, still
skip the scan (binding 16384 of them: 9.80 ms before, 9.86 ms after,
5874 ms with the full check every time).

Fixes: 637bc8bbe6c0 ("inet: reset tb->fastreuseport when adding a reuseport sk")
Cc: stable@vger.kernel.org
Signed-off-by: Juan Agustin Niccolini <juanagusnicco@gmail.com>
---
 include/net/inet_hashtables.h   |  2 ++
 net/ipv4/inet_connection_sock.c | 15 ++++++++++++---
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/include/net/inet_hashtables.h b/include/net/inet_hashtables.h
index 6e2fe186d0dc..44b7e39a1245 100644
--- a/include/net/inet_hashtables.h
+++ b/include/net/inet_hashtables.h
@@ -87,6 +87,8 @@ struct inet_bind_bucket {
 	__be32			fast_rcv_saddr;
 	unsigned short		fast_sk_family;
 	bool			fast_ipv6_only;
+	unsigned char		fast_sk_reuse;
+	int			fast_bound_dev_if;
 	struct hlist_node	node;
 	struct hlist_head	bhash2;
 	struct rcu_head		rcu;
diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
index e56beec43877..70b8662b3004 100644
--- a/net/ipv4/inet_connection_sock.c
+++ b/net/ipv4/inet_connection_sock.c
@@ -420,13 +420,18 @@ static inline int sk_reuseport_match(struct inet_bind_bucket *tb,
 		return 0;
 	if (!uid_eq(tb->fastuid, sk_uid(sk)))
 		return 0;
-	/* We only need to check the rcv_saddr if this tb was once marked
+	/* We only need to check the fast_* fields if this tb was once marked
 	 * without fastreuseport and then was reset, as we can only know that
-	 * the fast_*rcv_saddr doesn't have any conflicts with the socks on the
-	 * owners list.
+	 * the socket they describe doesn't have any conflicts with the socks
+	 * on the owners list.
 	 */
 	if (tb->fastreuseport == FASTREUSEPORT_ANY)
 		return 1;
+	if (tb->fast_sk_family != sk->sk_family ||
+	    tb->fast_ipv6_only != ipv6_only_sock(sk) ||
+	    tb->fast_bound_dev_if != sk->sk_bound_dev_if ||
+	    tb->fast_sk_reuse != sk->sk_reuse)
+		return 0;
 #if IS_ENABLED(CONFIG_IPV6)
 	if (tb->fast_sk_family == AF_INET6)
 		return ipv6_rcv_saddr_equal(&tb->fast_v6_rcv_saddr,
@@ -455,6 +460,8 @@ void inet_csk_update_fastreuse(const struct sock *sk,
 			tb->fast_rcv_saddr = sk->sk_rcv_saddr;
 			tb->fast_ipv6_only = ipv6_only_sock(sk);
 			tb->fast_sk_family = sk->sk_family;
+			tb->fast_sk_reuse = sk->sk_reuse;
+			tb->fast_bound_dev_if = sk->sk_bound_dev_if;
 #if IS_ENABLED(CONFIG_IPV6)
 			tb->fast_v6_rcv_saddr = sk->sk_v6_rcv_saddr;
 #endif
@@ -482,6 +489,8 @@ void inet_csk_update_fastreuse(const struct sock *sk,
 				tb->fast_rcv_saddr = sk->sk_rcv_saddr;
 				tb->fast_ipv6_only = ipv6_only_sock(sk);
 				tb->fast_sk_family = sk->sk_family;
+				tb->fast_sk_reuse = sk->sk_reuse;
+				tb->fast_bound_dev_if = sk->sk_bound_dev_if;
 #if IS_ENABLED(CONFIG_IPV6)
 				tb->fast_v6_rcv_saddr = sk->sk_v6_rcv_saddr;
 #endif
-- 
2.43.0

[PATCH net v2 2/2] selftests: net: Add SO_REUSEPORT bind cache test to bind_wildcard.c

[Juan Agustin Niccolini]

Add a test for the bug fixed by "tcp: Fix the SO_REUSEPORT bind cache
admitting a conflicting socket". With a 0.0.0.0 listener on the port, a
v6only [::] SO_REUSEPORT socket fills the bind cache, and a dual-stack
[::] SO_REUSEPORT socket must then fail to bind with EADDRINUSE.

Without the fix:

  #  RUN           global.reuseport_v6only_cache ...
  # bind_wildcard.c:860:reuseport_v6only_cache:Expected ret (0) == -1 (-1)
  # reuseport_v6only_cache: Test terminated by assertion
  #          FAIL  global.reuseport_v6only_cache
  not ok 1 global.reuseport_v6only_cache

With it:

  #  RUN           global.reuseport_v6only_cache ...
  #            OK  global.reuseport_v6only_cache
  ok 1 global.reuseport_v6only_cache

Signed-off-by: Juan Agustin Niccolini <juanagusnicco@gmail.com>
---
 tools/testing/selftests/net/bind_wildcard.c | 54 +++++++++++++++++++++
 1 file changed, 54 insertions(+)

diff --git a/tools/testing/selftests/net/bind_wildcard.c b/tools/testing/selftests/net/bind_wildcard.c
index be3298c791b2..334a0cf87edf 100644
--- a/tools/testing/selftests/net/bind_wildcard.c
+++ b/tools/testing/selftests/net/bind_wildcard.c
@@ -811,4 +811,58 @@ TEST_F(bind_wildcard, reuseport)
 		bind_socket(_metadata, self, variant, i, SO_REUSEPORT);
 }
 
+/* The SO_REUSEPORT bind cache (tb->fast_*) filled by a v6only [::] socket
+ * must not let a dual-stack [::] socket skip the conflict check against a
+ * 0.0.0.0 listener.
+ */
+TEST(reuseport_v6only_cache)
+{
+	struct sockaddr_in6 addr6 = {
+		.sin6_family = AF_INET6,
+		.sin6_addr = in6addr_any,
+	};
+	struct sockaddr_in addr4 = {
+		.sin_family = AF_INET,
+		.sin_addr.s_addr = htonl(INADDR_ANY),
+	};
+	socklen_t addrlen;
+	int fd[3], ret;
+	int on = 1;
+
+	ASSERT_EQ(unshare(CLONE_NEWNET), 0);
+
+	/* bind(0.0.0.0:P), listen() */
+	fd[0] = socket(AF_INET, SOCK_STREAM, 0);
+	ASSERT_GT(fd[0], 0);
+	ASSERT_EQ(bind(fd[0], (struct sockaddr *)&addr4, sizeof(addr4)), 0);
+	ASSERT_EQ(listen(fd[0], 1), 0);
+
+	addrlen = sizeof(addr4);
+	ASSERT_EQ(getsockname(fd[0], (struct sockaddr *)&addr4, &addrlen), 0);
+	addr6.sin6_port = addr4.sin_port;
+
+	/* SO_REUSEPORT + IPV6_V6ONLY, bind([::]:P) fills the cache. */
+	fd[1] = socket(AF_INET6, SOCK_STREAM, 0);
+	ASSERT_GT(fd[1], 0);
+	ret = setsockopt(fd[1], SOL_IPV6, IPV6_V6ONLY, &on, sizeof(on));
+	ASSERT_EQ(ret, 0);
+	ret = setsockopt(fd[1], SOL_SOCKET, SO_REUSEPORT, &on, sizeof(on));
+	ASSERT_EQ(ret, 0);
+	ASSERT_EQ(bind(fd[1], (struct sockaddr *)&addr6, sizeof(addr6)), 0);
+
+	/* SO_REUSEPORT, bind([::]:P) conflicts with 0.0.0.0:P. */
+	fd[2] = socket(AF_INET6, SOCK_STREAM, 0);
+	ASSERT_GT(fd[2], 0);
+	ret = setsockopt(fd[2], SOL_SOCKET, SO_REUSEPORT, &on, sizeof(on));
+	ASSERT_EQ(ret, 0);
+
+	ret = bind(fd[2], (struct sockaddr *)&addr6, sizeof(addr6));
+	ASSERT_EQ(ret, -1);
+	ASSERT_EQ(errno, EADDRINUSE);
+
+	close(fd[2]);
+	close(fd[1]);
+	close(fd[0]);
+}
+
 TEST_HARNESS_MAIN
-- 
2.43.0