Skip to contentSkip to conversation

tcp: fix device binding after failed IPv6 connect

PATCH New Xia Yan 1 participant 1 message Nov 11

Subsystems (4)
lore
Share & tools
lore

Xia Yan

author
#0 lore raw

PATCH v3 1/1 tcp: fix device binding after failed IPv6 connect

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.

---
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/[email protected]/
v1: https://lore.kernel.org/netdev/[email protected]/

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(-)
net/ipv6/tcp_ipv6.c +8 -2
index b1dec5d86d633db67a754c70ee1c63290a9acb5a..641b74aaf1963e71bb59e5542899e30e33b82a1e 100644
@@ -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