[PATCH net v3] tcp: fix device binding after failed IPv6 connect

[PATCH net v3] tcp: fix device binding after failed IPv6 connect

[Xia Yan]

tcp_v6_connect() sets sk_bound_dev_if from sin6_scope_id when connecting
to a link-local address. If the route lookup fails, the error path
leaves that binding in place. Retrying against ::1 on the same socket
can then fail with ENETUNREACH, although a fresh socket can connect.

Save the original binding and restore it unconditionally on native
IPv6 errors returned by tcp_v6_connect(). Restore it before
tcp_set_state(TCP_CLOSE) and inet_bhash2_reset_saddr(), which uses the
binding to select the l3mdev for the bind2 bucket. Do not restore it
again after the TCP_CLOSE callback. Keep IPv4-mapped errors on their
existing cleanup path.

Found during LLM-assisted review of the tcp_v6_connect() error paths.
Reproduced in UML by connecting to fe80::1 through a down dummy
interface, then retrying against a listener on ::1 on the same socket.
With the patch, the retry succeeds. The VRF cleanup tests also pass
without the inet_csk_get_port() warning seen on the baseline. Tests
also covered existing bindings and BPF sock_ops callbacks. These are
custom userspace tests; the networking selftest suite was not run.

Deferred Fast Open failures after connect() returns success are
unchanged.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Suggested-by: Jiayuan Chen <jiayuan.chen@linux.dev>
Suggested-by: Jakub Kicinski <kuba@kernel.org>
Link: https://lore.kernel.org/netdev/062d02ca-e9e1-4f4e-ac57-0de27272a205@linux.dev/
Link: https://lore.kernel.org/netdev/20261110100224.57cbf548@kernel.org/
Signed-off-by: Xia Yan <yanxia@cmss.chinamobile.com>
---
Changes in v3:
- Restore old_bound_dev_if without the scope_id_set and current-value
  checks, as Jakub requested.
- Keep the early restore before TCP_CLOSE and bind2 cleanup, and
  avoid restoring again after the close callback.
- Leave IPv4-mapped failures on the existing cleanup path.
- Use WRITE_ONCE() when installing the scope binding.

v2: https://lore.kernel.org/netdev/20261102143352.135769-1-yanxia@cmss.chinamobile.com/
v1: https://lore.kernel.org/netdev/20261031074654.73952-1-yanxia@cmss.chinamobile.com/

Testing on net d2809ccf3cfc:
- UML: reconnect and existing-binding checks, two VRF cleanup cases,
  and five BPF sock_ops cases. The baseline reproduces the stale
  binding and VRF warning; v3 completes without kernel warnings.
  Native CONNECT callback bindings are rolled back on failure;
  CLOSE callback bindings and IPv4-mapped behavior are preserved.
- x86_64, GCC 12.3.1, W=1: completed kernel and module builds with
  configurations derived from allmodconfig and allyesconfig.
  WERROR, KASAN and KCSAN were disabled; allyes also had
  KUNIT_ALL_TESTS disabled, with the resulting dependency changes.
  tcp_ipv6.o compiled without diagnostics in both configurations.

 net/ipv6/tcp_ipv6.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index b1dec5d86d633db67a754c70ee1c63290a9acb5a..641b74aaf1963e71bb59e5542899e30e33b82a1e 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -134,6 +134,7 @@ static int tcp_v6_connect(struct sock *sk, struct sockaddr_unsized *uaddr,
 			  int addr_len)
 {
 	struct sockaddr_in6 *usin = (struct sockaddr_in6 *) uaddr;
+	int old_bound_dev_if = READ_ONCE(sk->sk_bound_dev_if);
 	struct inet_connection_sock *icsk = inet_csk(sk);
 	struct inet_timewait_death_row *tcp_death_row;
 	struct ipv6_pinfo *np = tcp_inet6_sk(sk);
@@ -197,7 +198,7 @@ static int tcp_v6_connect(struct sock *sk, struct sockaddr_unsized *uaddr,
 			if (!sk_dev_equal_l3scope(sk, usin->sin6_scope_id))
 				return -EINVAL;
 
-			sk->sk_bound_dev_if = usin->sin6_scope_id;
+			WRITE_ONCE(sk->sk_bound_dev_if, usin->sin6_scope_id);
 		}
 
 		/* Connect to link-local address requires an interface */
@@ -251,7 +252,7 @@ static int tcp_v6_connect(struct sock *sk, struct sockaddr_unsized *uaddr,
 #if defined(CONFIG_TCP_MD5SIG) || defined(CONFIG_TCP_AO)
 			tp->af_specific = &tcp_sock_ipv6_specific;
 #endif
-			goto failure;
+			goto out;
 		}
 		np->saddr = sk->sk_v6_rcv_saddr;
 
@@ -353,9 +354,14 @@ static int tcp_v6_connect(struct sock *sk, struct sockaddr_unsized *uaddr,
 	return 0;
 
 late_failure:
+	WRITE_ONCE(sk->sk_bound_dev_if, old_bound_dev_if);
 	tcp_set_state(sk, TCP_CLOSE);
 	inet_bhash2_reset_saddr(sk);
+	goto out;
+
 failure:
+	WRITE_ONCE(sk->sk_bound_dev_if, old_bound_dev_if);
+out:
 	inet->inet_dport = 0;
 	sk->sk_route_caps = 0;
 	return err;

base-commit: d2809ccf3cfc3c30581dc64ebec5f49f7ee1e228

Re: [PATCH net v3] tcp: fix device binding after failed IPv6 connect

[netdev-bot+sashiko]

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

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

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 only covers errors that tcp_v6_connect() returns
  synchronously.
- [Low] In the late_failure path of tcp_v6_connect()
  (net/ipv6/tcp_ipv6.c:356-359), the patch now restores sk_bound_dev_if…

--

tcp: fix device binding after failed IPv6 connect

This patch saves sk_bound_dev_if when tcp_v6_connect() starts and restores
it on native IPv6 error returns. The goal is that a scope-id binding left
by a failed link-local connect does not affect a later connect on the same
socket. In late_failure the restore happens before tcp_set_state(TCP_CLOSE)
and inet_bhash2_reset_saddr(), and IPv4-mapped errors keep their existing
cleanup path.

Link: https://lore.kernel.org/netdev/062d02ca-e9e1-4f4e-ac57-0de27272a205@linux.dev/
Link: https://lore.kernel.org/netdev/20261110100224.57cbf548@kernel.org/

> diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
> index b1dec5d86d633..641b74aaf1963 100644
> --- a/net/ipv6/tcp_ipv6.c
> +++ b/net/ipv6/tcp_ipv6.c
> @@ -134,6 +134,7 @@ static int tcp_v6_connect(struct sock *sk, struct sockaddr_unsized *uaddr,
>  			  int addr_len)
>  {
>  	struct sockaddr_in6 *usin = (struct sockaddr_in6 *) uaddr;
> +	int old_bound_dev_if = READ_ONCE(sk->sk_bound_dev_if);
>  	struct inet_connection_sock *icsk = inet_csk(sk);

[ ... ]

> @@ -197,7 +198,7 @@ static int tcp_v6_connect(struct sock *sk, struct sockaddr_unsized *uaddr,
>  			if (!sk_dev_equal_l3scope(sk, usin->sin6_scope_id))
>  				return -EINVAL;
>  
> -			sk->sk_bound_dev_if = usin->sin6_scope_id;
> +			WRITE_ONCE(sk->sk_bound_dev_if, usin->sin6_scope_id);
>  		}

[Severity: High]
The binding is now restored when tcp_v6_connect() fails synchronously.
What happens when tcp_v6_connect() returns 0 and the handshake fails
afterwards?

If tcp_connect() sends the SYN, the scope id written here stays in
sk_bound_dev_if. Some ways the handshake can still fail:

  - a blocking connect() gets an RST, an ICMP unreachable or a SYN
    timeout
  - a non-blocking connect returns EINPROGRESS and fails later

Either way the socket ends up in __inet_stream_connect():

net/ipv4/af_inet.c:__inet_stream_connect() {
    ...
sock_error:
	err = sock_error(sk) ? : -ECONNABORTED;
	sock->state = SS_UNCONNECTED;
	sk->sk_disconnects++;
	if (sk->sk_prot->disconnect(sk, flags))
    ...
}

tcp_disconnect() never touches sk_bound_dev_if, and the socket can then
be connected again.

Would a retry to ::1 on the same socket then skip the link-local branch?
It would use fl6->flowi6_oif = sk->sk_bound_dev_if, which is the old
interface, and could fail with ENETUNREACH. That is the symptom the
commit message describes.

The VRF case looks reachable too. Take a socket bound to [::]:P with no
device. It connects to fe80::x%vrf_slave, the connect fails
asynchronously, and tcp_disconnect() then runs with the stale binding
still set:

net/ipv4/tcp.c:tcp_disconnect() {
    ...
	inet->inet_dport = 0;

	inet_bhash2_reset_saddr(sk);
    ...
}

__inet_bhash2_update_saddr() would look up (any, P, VRF), find nothing,
and create a new tb2 under the existing tb, whose l3mdev is 0.

A later listen() reaches inet_csk_get_port(). There, l3mdev is computed
from the stale binding, the existing tb does not match, and this fires:

	WARN_ON(inet_csk(sk)->icsk_bind_hash != tb);

The commit message says the VRF cleanup tests no longer show this
warning.

For comparison, UDP covers both paths:

  - ip6_datagram_connect() restores old_bound_dev_if on error
  - __udp_disconnect() does WRITE_ONCE(sk->sk_bound_dev_if, 0)

The commit message only excludes "Deferred Fast Open failures after
connect() returns success". Should ordinary handshake failures
(ECONNREFUSED, ETIMEDOUT, EHOSTUNREACH) be covered as well? One option is
a restore at disconnect time that records whether the binding was
implicit.

[ ... ]

> @@ -353,9 +354,14 @@ static int tcp_v6_connect(struct sock *sk, struct sockaddr_unsized *uaddr,
>  	return 0;
>  
>  late_failure:
> +	WRITE_ONCE(sk->sk_bound_dev_if, old_bound_dev_if);
>  	tcp_set_state(sk, TCP_CLOSE);
>  	inet_bhash2_reset_saddr(sk);

[Severity: Low]
When the binding is restored here, the socket is still in ehash.

inet6_hash_connect() has already hashed it, and its 4-tuple uniqueness
check was done against the scope device. The socket is only unhashed
later, inside tcp_set_state(TCP_CLOSE), after the BPF STATE_CB and
bpf_tcp_ops_call(set_state) have run.

Lockless RX lookup reads the binding through inet6_match():

	return inet_sk_bound_dev_eq(net, READ_ONCE(sk->sk_bound_dev_if), dif,
				    sdif);

and inet_bound_dev_eq() does:

	if (!bound_dev_if)
		return !sdif || l3mdev_accept;
	return bound_dev_if == dif || bound_dev_if == sdif;

In this window, can the hashed socket match the same link-local 4-tuple
arriving on a different interface? With an old binding of 0 that would be
any non-VRF device. With an old binding of the VRF master it would be any
slave of that VRF.

Errors that reach late_failure after hashing include:

  - -EKEYREJECTED from tcp_connect() with TCP-AO
  - -EHOSTUNREACH
  - -ENOBUFS
  - -ECONNREFUSED

Such packets would go to the backlog of the closing socket and then be
discarded by tcp_rcv_state_process() in TCP_CLOSE. A different connection
using the same link-local 4-tuple on another link could therefore lose
segments, though retransmission should recover them. Before this patch
the binding stayed on the scope device until after unhash.

Could the binding be restored after the socket is unhashed but before
inet_bhash2_reset_saddr()? That would need to fit with the intent that a
binding set by the CLOSE callback takes priority.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261111113455.284747-1-yanxia%40cmss.chinamobile.com