[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

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

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

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

[Juan Agustin Niccolini]

> This is an automated message. This series looks like a fix, but its
> commit messages seem to be missing some information:
>
>  - How the issue was discovered, e.g. hit in production, hit during
>    development, syzbot report, manual code inspection, LLM or static
>    analysis tool scan.

The bug was found by my LLM-based bug discovery pipeline.

Best,
Agustin

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

[Kuniyuki Iwashima]

On Wed, 11 Nov 2026 12:47:03 -0500, Juan Agustin Niccolini wrote:
> 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>

Reviewed-by: Kuniyuki Iwashima <kuniyu@google.com>

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

[Kuniyuki Iwashima]

On Wed, 11 Nov 2026 12:47:04 -0500, Juan Agustin Niccolini wrote:
> 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>

Reviewed-by: Kuniyuki Iwashima <kuniyu@google.com>

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

[Eric Dumazet]

Le sam. 14 nov. 2026 à 08:28, Kuniyuki Iwashima <kuniyu@google.com> a écrit :
>
> On Wed, 11 Nov 2026 12:47:03 -0500, Juan Agustin Niccolini wrote:
> > 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>
>
> Reviewed-by: Kuniyuki Iwashima <kuniyu@google.com>

Reviewed-by: Eric Dumazet <edumazet@kernel.org>

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

[Eric Dumazet]

Le sam. 14 nov. 2026 à 08:29, Kuniyuki Iwashima <kuniyu@google.com> a écrit :
>
> On Wed, 11 Nov 2026 12:47:04 -0500, Juan Agustin Niccolini wrote:
> > 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>
>
> Reviewed-by: Kuniyuki Iwashima <kuniyu@google.com>

Reviewed-by: Eric Dumazet <edumazet@kernel.org>

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

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 1 · Medium: 0 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] The fix still lets the SO_REUSEPORT bind cache admit a
  conflicting socket.

--

tcp: Fix the SO_REUSEPORT bind cache admitting a conflicting socket

This commit adds fast_sk_reuse and fast_bound_dev_if to struct
inet_bind_bucket. In the FASTREUSEPORT_STRICT state, sk_reuseport_match()
now skips the bind conflict check only when the new socket matches the
cached socket's family, IPV6_V6ONLY, bound device and SO_REUSEADDR.

> diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
> index e56beec438773..70b8662b30048 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;

[ ... ]

> @@ -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

[Severity: High]
Can a socket that never went through a bind conflict check still fill the
STRICT cache?

The rewritten comment in sk_reuseport_match() assumes the cached socket
has no conflicts with the sockets on the owners list.

__inet_inherit_port() breaks that assumption. When the child's port
differs from the listener's bucket port (the TPROXY redirect case
described in the NOTE there), it calls inet_csk_update_fastreuse() for
the child. It never checks the child against the owners of the target
bucket:

net/ipv4/inet_hashtables.c:__inet_inherit_port() {
    ...
	if (tb->port != port) {
		...
		update_fastreuse = true;

		goto bhash2_find;
    ...
	if (update_fastreuse)
		inet_csk_update_fastreuse(child, tb, tb2);
    ...
}

The child copies sk_reuseport and sk_uid from the listener, and
sk_clone_lock() clears sk_reuseport_cb.

Suppose a listener without SO_REUSEPORT owns the target port. Then
tb->fastreuseport is 0 and sk_reuseport_match() returns 0. The else
branch above therefore resets the bucket to FASTREUSEPORT_STRICT with the
child's uid, address, family, v6only, sk_reuse and bound_dev_if.

One possible sequence:

1. user B, no SO_REUSEPORT: bind(0.0.0.0:P), listen()
   tb(P)->fastreuseport is 0

2. TPROXY steers a connection for X:P to listener L (user A,
   SO_REUSEPORT) on port Q
   __inet_inherit_port()->inet_csk_update_fastreuse() puts the child
   in tb(P), which becomes STRICT with fast_rcv_saddr = X and
   fastuid = A

3. user A sets SO_REUSEPORT (plus IP_FREEBIND if X is not local), uses
   the same family, v6only, device and SO_REUSEADDR as the child, and
   calls bind(X:P)
   inet_csk_get_port()->sk_reuseport_match() returns 1, so
   check_bind_conflict = false

4. user A calls listen()
   get_port() matches again and skips inet_bhash2_addr_any_conflict()
   and inet_csk_bind_conflict() again

A full check would return EADDRINUSE here, because inet_bind_conflict()
sees !sk2->sk_reuseport for user B's listener.

The new field comparisons cannot catch this, since the cached values
really do belong to the child. The setup needs a TPROXY rule configured
with CAP_NET_ADMIN, and the attacker has to run as the proxy listener's
uid. The final bind() and listen() need no privilege.

The selftest patch that follows in this series does not touch kernel
code, so this path is still open after the whole series.

Should an inherited child be kept out of the STRICT cache unless it was
conflict-checked? For example, this path could set tb->fastreuseport = 0
instead of resetting the cache to the child's attributes.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261111174704.24312-1-juanagusnicco%40gmail.com

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

[Juan Agustin Niccolini]

On Sun, 15 Nov 2026 18:15:58 +0000, netdev-bot+sashiko@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider.
>
> Critical: 0 · High: 1 · Medium: 0 · Low: 0
>
> If you believe none of the issues are legitimate, please reply to each
> issue of Medium severity or higher and explain why the AI is wrong.
> If any of them are legitimate and you plan to respin, please let patchwork
> know by including "pw-bot: cr" as a separate line at the end of your reply
> (one such reply per series is enough).
>
> - [High] The fix still lets the SO_REUSEPORT bind cache admit a
>   conflicting socket.
>
> --
>
> tcp: Fix the SO_REUSEPORT bind cache admitting a conflicting socket
>
> This commit adds fast_sk_reuse and fast_bound_dev_if to struct
> inet_bind_bucket. In the FASTREUSEPORT_STRICT state, sk_reuseport_match()
> now skips the bind conflict check only when the new socket matches the
> cached socket's family, IPV6_V6ONLY, bound device and SO_REUSEADDR.
>
> > diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
> > index e56beec438773..70b8662b30048 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;
>
> [ ... ]
>
> > @@ -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
>
> [Severity: High]
> Can a socket that never went through a bind conflict check still fill the
> STRICT cache?
>
> The rewritten comment in sk_reuseport_match() assumes the cached socket
> has no conflicts with the sockets on the owners list.
>
> __inet_inherit_port() breaks that assumption. When the child's port
> differs from the listener's bucket port (the TPROXY redirect case
> described in the NOTE there), it calls inet_csk_update_fastreuse() for
> the child. It never checks the child against the owners of the target
> bucket:
>
> net/ipv4/inet_hashtables.c:__inet_inherit_port() {
>     ...
> 	if (tb->port != port) {
> 		...
> 		update_fastreuse = true;
>
> 		goto bhash2_find;
>     ...
> 	if (update_fastreuse)
> 		inet_csk_update_fastreuse(child, tb, tb2);
>     ...
> }
>
> The child copies sk_reuseport and sk_uid from the listener, and
> sk_clone_lock() clears sk_reuseport_cb.
>
> Suppose a listener without SO_REUSEPORT owns the target port. Then
> tb->fastreuseport is 0 and sk_reuseport_match() returns 0. The else
> branch above therefore resets the bucket to FASTREUSEPORT_STRICT with the
> child's uid, address, family, v6only, sk_reuse and bound_dev_if.
>
> One possible sequence:
>
> 1. user B, no SO_REUSEPORT: bind(0.0.0.0:P), listen()
>    tb(P)->fastreuseport is 0
>
> 2. TPROXY steers a connection for X:P to listener L (user A,
>    SO_REUSEPORT) on port Q
>    __inet_inherit_port()->inet_csk_update_fastreuse() puts the child
>    in tb(P), which becomes STRICT with fast_rcv_saddr = X and
>    fastuid = A
>
> 3. user A sets SO_REUSEPORT (plus IP_FREEBIND if X is not local), uses
>    the same family, v6only, device and SO_REUSEADDR as the child, and
>    calls bind(X:P)
>    inet_csk_get_port()->sk_reuseport_match() returns 1, so
>    check_bind_conflict = false
>
> 4. user A calls listen()
>    get_port() matches again and skips inet_bhash2_addr_any_conflict()
>    and inet_csk_bind_conflict() again
>
> A full check would return EADDRINUSE here, because inet_bind_conflict()
> sees !sk2->sk_reuseport for user B's listener.
>
> The new field comparisons cannot catch this, since the cached values
> really do belong to the child. The setup needs a TPROXY rule configured
> with CAP_NET_ADMIN, and the attacker has to run as the proxy listener's
> uid. The final bind() and listen() need no privilege.
>
> The selftest patch that follows in this series does not touch kernel
> code, so this path is still open after the whole series.
>
> Should an inherited child be kept out of the STRICT cache unless it was
> conflict-checked? For example, this path could set tb->fastreuseport = 0
> instead of resetting the cache to the child's attributes.
>
> -- 
> Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261111174704.24312-1-juanagusnicco%40gmail.com

This is a separate, pre-existing issue that needs a rare configuration.
I was already aware of it. My Sashiko instance flagged it too, and
correctly classified it as pre-existing, unlike this one.

It only applies where an administrator has already set up a transparent
proxy (which takes CAP_NET_ADMIN in the netns that owns the port), and
the attacker must run as the proxy's uid. A user namespace does not get
around that requirement. For the attacker to hijack traffic, which is
what matters for severity, even further requirements are needed.

Severity is medium at best. Blast radius is low, if anything at all.
Regardless, my patch does not cause it: the bug reproduces without it.

The bug this patch fixes needs no special configuration or privilege.
I confirmed it can be used to hijack traffic, even by a sandboxed app
on Android. It can be exploited on any listener that binds only to
0.0.0.0, which is very common, among other cases.

The TPROXY bug that Sashiko found can't be addressed by the check my
patch changes, even if more fields are recorded and compared, since
the cached socket there never goes through a conflict check. It is a
different kind of bug with a similar consequence, and it needs changes
in __inet_inherit_port(), which my patch does not touch.

Best,
Agustin