[PATCH net v2] 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 failure path
leaves that device binding in place. A retry to ::1 on the same socket
can then fail with ENETUNREACH, although a fresh socket can connect.

Save the original binding and restore it on synchronous errors returned
by tcp_v6_connect(). Restore it before tcp_set_state(TCP_CLOSE) and
inet_bhash2_reset_saddr(), since the latter uses sk_bound_dev_if to
select the l3mdev for the bind2 bucket.

Only undo a binding installed from sin6_scope_id by this call, and leave
a different binding installed by a later callback intact. Once the
late_failure path has handled the binding, do not restore it again
after the state callback and source-address cleanup.

This patch does not change disconnect handling after an asynchronous
connection failure.

Found during LLM-assisted review of the tcp_v6_connect() error paths;
the same assistance was used for the fix, tests and commit message.
Reproduced in UML by connecting to fe80::1 through a down dummy
interface, then retrying the same socket against a listener on ::1.

Tested in UML with 39 targeted userspace checks covering reconnects,
existing bindings, VRF bind-bucket cleanup and BPF sock_ops binding
changes. All 39 checks pass with v2, including the retry reproducer.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@xxxxxxxxxxxxxxx
Suggested-by: Jiayuan Chen <jiayuan.chen@xxxxxxxxx>
Link: https://lore.kernel.org/netdev/062d02ca-e9e1-4f4e-ac57-0de27272a205@xxxxxxxxx/
Signed-off-by: Xia Yan <yanxia@xxxxxxxxxxxxxxxxxxxx>
---
Changes in v2:
- Restore the scope binding before the TCP_CLOSE state change and
  inet_bhash2_reset_saddr(), as Jiayuan pointed out.
- Only restore a scope binding installed by this call, and preserve a
  different binding installed by a callback.
- Avoid a second restore after the late_failure cleanup.
- Follow reverse xmas tree declaration order.
- Clarify that this change handles synchronous connect failures.

v1: https://lore.kernel.org/netdev/20261031074654.73952-1-yanxia@xxxxxxxxxxxxxxxxxxxx/

Testing on net cb284914e749:
- UML: baseline 32/39, v1 35/39, v2 39/39. The tests reproduce the stale
  binding on baseline, the bind2 warning on v1, and the BPF binding
  regression on v1. v2 passes all checks without kernel warnings.
  These are custom userspace tests; the kernel networking selftest
  suite was not run.
- x86_64, GCC 12.3.1, W=1: baseline and v2 completed vmlinux, bzImage
  and module builds using configurations derived from allmodconfig and
  allyesconfig. WERROR, KASAN and KCSAN were disabled; allyes also had
  KUNIT_ALL_TESTS disabled, with the resulting dependency changes.
  The changed object compiled without diagnostics in both configs.
- Unmodified allmodconfig and allyesconfig failed on both baseline and
  v2 with the same existing WERROR diagnostics in lockdep_proc.c,
  callthunks.c, coda/upcall.c and stackinit_kunit.c. The derived allyes
  builds have identical diagnostics. The v2 allmod build was incremental,
  so its log does not establish full-tree warning equality.

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

diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index e006a30360eae..5bd4bd49d2824 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);
@@ -142,6 +143,7 @@ static int tcp_v6_connect(struct sock *sk, struct sockaddr_unsized *uaddr,
 	struct tcp_sock *tp = tcp_sk(sk);
 	struct net *net = sock_net(sk);
 	struct ipv6_txoptions *opt;
+	bool scope_id_set = false;
 	struct dst_entry *dst;
 	struct flowi6 *fl6;
 	int addr_type;
@@ -198,6 +200,7 @@ static int tcp_v6_connect(struct sock *sk, struct sockaddr_unsized *uaddr,
 				return -EINVAL;
 
 			sk->sk_bound_dev_if = usin->sin6_scope_id;
+			scope_id_set = true;
 		}
 
 		/* Connect to link-local address requires an interface */
@@ -353,9 +356,16 @@ static int tcp_v6_connect(struct sock *sk, struct sockaddr_unsized *uaddr,
 	return 0;
 
 late_failure:
+	if (scope_id_set &&
+	    READ_ONCE(sk->sk_bound_dev_if) == usin->sin6_scope_id)
+		WRITE_ONCE(sk->sk_bound_dev_if, old_bound_dev_if);
+	scope_id_set = false;
 	tcp_set_state(sk, TCP_CLOSE);
 	inet_bhash2_reset_saddr(sk);
 failure:
+	if (scope_id_set &&
+	    READ_ONCE(sk->sk_bound_dev_if) == usin->sin6_scope_id)
+		WRITE_ONCE(sk->sk_bound_dev_if, old_bound_dev_if);
 	inet->inet_dport = 0;
 	sk->sk_route_caps = 0;
 	return err;

base-commit: cb284914e749388eef402938bdb3115fe1f6d337