mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH net v3] tcp: prevent cross-family skb processing after disconnect
@ 2026-11-03 10:36 Zhiyu Zhang
  2026-11-03 11:03 ` Eric Dumazet
  2026-11-05 22:39 ` netdev-bot+sashiko
  0 siblings, 2 replies; 4+ messages in thread
From: Zhiyu Zhang @ 2026-11-03 10:36 UTC (permalink / raw)
  To: Eric Dumazet, Kuniyuki Iwashima
  Cc: Neal Cardwell, David S. Miller, Jakub Kicinski, Paolo Abeni,
	Simon Horman, Matthieu Baerts, Mat Martineau, Geliang Tang,
	netdev, mptcp, linux-kernel, Tong Liu, Qi Tang

From: Eric Dumazet <edumazet@kernel.org>

A successful connect() of an IPv6 socket to a v4-mapped address switches
icsk_af_ops, sk_backlog_rcv and af_specific to their IPv4-mapped
variants. A later connect(AF_UNSPEC) calls tcp_disconnect(), which does
not restore them.

The socket can then be reused for a native IPv6 connection while
tcp_v4_do_rcv() remains installed. IPv6 packets drained from its backlog
are consequently passed to the IPv4 receive path.

There is also a race in the opposite direction. tcp_v6_rcv() can look up
the socket and initialize TCP_SKB_CB(skb)->header.h6 before taking the
socket lock. If the socket is disconnected and reconnected to a
v4-mapped peer in that window, release_sock() can drain the queued IPv6
skb through the newly installed tcp_v4_do_rcv().

tcp_v4_send_reset() and tcp_v4_send_ack() then interpret header.h6 as
IPv4 options. __ip_options_echo() can consequently overflow its 40-byte
destination buffer.

Commit 8cc3aef0cb19 ("tcp: Do not allow buggy transitions between ehash
and lhash2.") prevents listen() after connect(AF_UNSPEC), but a native
IPv6 connect is still allowed.

Add tcp_v6_disconnect() to restore the IPv6 handlers after disconnecting
a v4-mapped socket, and use the same helper in the connect error path.
Reject non-IPv4 skbs in tcp_v4_do_rcv() before accessing IPv4
control-block data. In the reverse race, the skb belongs to the
disconnected connection, so drop it instead of redirecting it to the
IPv6 receive path.

MPTCP subflows override sk_prot, while two teardown paths call
tcp_disconnect() directly. Route them through the protocol-specific
disconnect operation so IPv6 MPTCP subflows receive the same
restoration.

Tested with TCP and MPTCP by reconnecting an AF_INET6 socket in both
directions after connect(AF_UNSPEC). A socket filter was used to widen
the native-to-mapped race window and verify that the stale IPv6 skb did
not reach tcp_v4_send_reset().

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Reported-by: Tong Liu <lyutoon@gmail.com>
Reported-by: Qi Tang <tpluszz77@gmail.com>
Closes: https://lore.kernel.org/netdev/CALf2hKvp0NPVLwr=a3+jUHtPohxbra8gox+aYAfWathJnn-J+Q@mail.gmail.com/
Link: https://lore.kernel.org/netdev/CAL4WiiqFJpmcV4=TWRpukCq6q=-729658FwGvieNcPXe97kLvQ@mail.gmail.com/
Signed-off-by: Eric Dumazet <edumazet@kernel.org>
Co-developed-by: Zhiyu Zhang <zhiyuzhang999@gmail.com>
Signed-off-by: Zhiyu Zhang <zhiyuzhang999@gmail.com>
---
Changes in v3:
- Reject non-IPv4 skbs in tcp_v4_do_rcv() to cover packets queued
  across a native IPv6 to v4-mapped reconnect.
- Drop Kuniyuki's Reviewed-by tag due to the code change.

v2: https://lore.kernel.org/netdev/20261101182736.430733-1-zhiyuzhang999@gmail.com/
v1: https://lore.kernel.org/netdev/20261030032314.1978575-1-zhiyuzhang999@gmail.com/

 net/ipv4/tcp_ipv4.c  |  5 +++++
 net/ipv6/tcp_ipv6.c  | 37 ++++++++++++++++++++++++++++---------
 net/mptcp/protocol.c |  4 ++--
 3 files changed, 35 insertions(+), 11 deletions(-)

diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index a7f9242f1e42..8fa6564d2b60 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -1836,6 +1836,11 @@ int tcp_v4_do_rcv(struct sock *sk, struct sk_buff *skb)
 {
 	enum skb_drop_reason reason;
 
+	if (unlikely(skb->protocol != htons(ETH_P_IP))) {
+		reason = SKB_DROP_REASON_INVALID_PROTO;
+		goto err_discard;
+	}
+
 	reason = psp_sk_rx_policy_check(sk, skb);
 	if (reason)
 		goto err_discard;
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index e006a30360ea..a93410aab66e 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -130,6 +130,19 @@ static int tcp_v6_pre_connect(struct sock *sk, struct sockaddr_unsized *uaddr,
 	return BPF_CGROUP_RUN_PROG_INET6_CONNECT(sk, uaddr, &addr_len);
 }
 
+/* Undo the v4-mapped setup done by tcp_v6_connect(). */
+static void tcp_v6_restore_af_ops(struct sock *sk)
+{
+	/* Paired with READ_ONCE() in tcp_(get|set)sockopt() */
+	WRITE_ONCE(inet_csk(sk)->icsk_af_ops, &ipv6_specific);
+	if (sk_is_mptcp(sk))
+		mptcpv6_handle_mapped(sk, false);
+	sk->sk_backlog_rcv = tcp_v6_do_rcv;
+#if defined(CONFIG_TCP_MD5SIG) || defined(CONFIG_TCP_AO)
+	tcp_sk(sk)->af_specific = &tcp_sock_ipv6_specific;
+#endif
+}
+
 static int tcp_v6_connect(struct sock *sk, struct sockaddr_unsized *uaddr,
 			  int addr_len)
 {
@@ -243,14 +256,7 @@ static int tcp_v6_connect(struct sock *sk, struct sockaddr_unsized *uaddr,
 
 		if (err) {
 			icsk->icsk_ext_hdr_len = exthdrlen;
-			/* Paired with READ_ONCE() in tcp_(get|set)sockopt() */
-			WRITE_ONCE(icsk->icsk_af_ops, &ipv6_specific);
-			if (sk_is_mptcp(sk))
-				mptcpv6_handle_mapped(sk, false);
-			sk->sk_backlog_rcv = tcp_v6_do_rcv;
-#if defined(CONFIG_TCP_MD5SIG) || defined(CONFIG_TCP_AO)
-			tp->af_specific = &tcp_sock_ipv6_specific;
-#endif
+			tcp_v6_restore_af_ops(sk);
 			goto failure;
 		}
 		np->saddr = sk->sk_v6_rcv_saddr;
@@ -2093,6 +2099,19 @@ static int tcp_v6_init_sock(struct sock *sk)
 	return 0;
 }
 
+static int tcp_v6_disconnect(struct sock *sk, int flags)
+{
+	int ret = tcp_disconnect(sk, flags);
+
+	/* A socket used for a v4-mapped connection must not keep IPv4
+	 * handlers when it is reused for listen() or a native IPv6 connect().
+	 */
+	if (sk->sk_backlog_rcv == tcp_v4_do_rcv)
+		tcp_v6_restore_af_ops(sk);
+
+	return ret;
+}
+
 #ifdef CONFIG_PROC_FS
 /* Proc filesystem TCPv6 sock list dumping. */
 static void get_openreq6(struct seq_file *seq,
@@ -2283,7 +2302,7 @@ struct proto tcpv6_prot = {
 	.close			= tcp_close,
 	.pre_connect		= tcp_v6_pre_connect,
 	.connect		= tcp_v6_connect,
-	.disconnect		= tcp_disconnect,
+	.disconnect		= tcp_v6_disconnect,
 	.accept			= inet_csk_accept,
 	.ioctl			= tcp_ioctl,
 	.init			= tcp_v6_init_sock,
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 7a22e9388e0e..fdfbc7de2ec0 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -2608,7 +2608,7 @@ static void __mptcp_subflow_disconnect(struct sock *ssk,
 		/* The MPTCP code never wait on the subflow sockets, TCP-level
 		 * disconnect should never fail
 		 */
-		WARN_ON_ONCE(tcp_disconnect(ssk, 0));
+		WARN_ON_ONCE(ssk->sk_prot->disconnect(ssk, 0));
 		mptcp_subflow_ctx_reset(subflow);
 	} else {
 		tcp_shutdown(ssk, SEND_SHUTDOWN);
@@ -3287,7 +3287,7 @@ void mptcp_subflow_shutdown(struct sock *sk, struct sock *ssk, int how)
 			break;
 		fallthrough;
 	case TCP_SYN_SENT:
-		WARN_ON_ONCE(tcp_disconnect(ssk, O_NONBLOCK));
+		WARN_ON_ONCE(ssk->sk_prot->disconnect(ssk, O_NONBLOCK));
 		break;
 	default:
 		if (__mptcp_check_fallback(mptcp_sk(sk))) {

base-commit: 5838f10563d58afca6711edb5e4eda968ca181d9
-- 
2.34.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH net v3] tcp: prevent cross-family skb processing after disconnect
  2026-11-03 10:36 [PATCH net v3] tcp: prevent cross-family skb processing after disconnect Zhiyu Zhang
@ 2026-11-03 11:03 ` Eric Dumazet
  2026-11-04 19:19   ` Zhiyu Zhang
  2026-11-05 22:39 ` netdev-bot+sashiko
  1 sibling, 1 reply; 4+ messages in thread
From: Eric Dumazet @ 2026-11-03 11:03 UTC (permalink / raw)
  To: Zhiyu Zhang
  Cc: Kuniyuki Iwashima, Neal Cardwell, David S. Miller,
	Jakub Kicinski, Paolo Abeni, Simon Horman, Matthieu Baerts,
	Mat Martineau, Geliang Tang, netdev, mptcp, linux-kernel,
	Tong Liu, Qi Tang

Le mar. 3 nov. 2026 à 11:40, Zhiyu Zhang <zhiyuzhang999@gmail.com> a écrit :
>
> From: Eric Dumazet <edumazet@kernel.org>
>
> A successful connect() of an IPv6 socket to a v4-mapped address switches
> icsk_af_ops, sk_backlog_rcv and af_specific to their IPv4-mapped
> variants. A later connect(AF_UNSPEC) calls tcp_disconnect(), which does
> not restore them.
>
> The socket can then be reused for a native IPv6 connection while
> tcp_v4_do_rcv() remains installed. IPv6 packets drained from its backlog
> are consequently passed to the IPv4 receive path.
>
> There is also a race in the opposite direction. tcp_v6_rcv() can look up
> the socket and initialize TCP_SKB_CB(skb)->header.h6 before taking the
> socket lock. If the socket is disconnected and reconnected to a
> v4-mapped peer in that window, release_sock() can drain the queued IPv6
> skb through the newly installed tcp_v4_do_rcv().
>
> tcp_v4_send_reset() and tcp_v4_send_ack() then interpret header.h6 as
> IPv4 options. __ip_options_echo() can consequently overflow its 40-byte
> destination buffer.
>
> Commit 8cc3aef0cb19 ("tcp: Do not allow buggy transitions between ehash
> and lhash2.") prevents listen() after connect(AF_UNSPEC), but a native
> IPv6 connect is still allowed.
>
> Add tcp_v6_disconnect() to restore the IPv6 handlers after disconnecting
> a v4-mapped socket, and use the same helper in the connect error path.
> Reject non-IPv4 skbs in tcp_v4_do_rcv() before accessing IPv4
> control-block data. In the reverse race, the skb belongs to the
> disconnected connection, so drop it instead of redirecting it to the
> IPv6 receive path.
>
> MPTCP subflows override sk_prot, while two teardown paths call
> tcp_disconnect() directly. Route them through the protocol-specific
> disconnect operation so IPv6 MPTCP subflows receive the same
> restoration.
>
> Tested with TCP and MPTCP by reconnecting an AF_INET6 socket in both
> directions after connect(AF_UNSPEC). A socket filter was used to widen
> the native-to-mapped race window and verify that the stale IPv6 skb did
> not reach tcp_v4_send_reset().
>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Reported-by: Tong Liu <lyutoon@gmail.com>
> Reported-by: Qi Tang <tpluszz77@gmail.com>
> Closes: https://lore.kernel.org/netdev/CALf2hKvp0NPVLwr=a3+jUHtPohxbra8gox+aYAfWathJnn-J+Q@mail.gmail.com/
> Link: https://lore.kernel.org/netdev/CAL4WiiqFJpmcV4=TWRpukCq6q=-729658FwGvieNcPXe97kLvQ@mail.gmail.com/
> Signed-off-by: Eric Dumazet <edumazet@kernel.org>
> Co-developed-by: Zhiyu Zhang <zhiyuzhang999@gmail.com>
> Signed-off-by: Zhiyu Zhang <zhiyuzhang999@gmail.com>
> ---
> Changes in v3:
> - Reject non-IPv4 skbs in tcp_v4_do_rcv() to cover packets queued
>   across a native IPv6 to v4-mapped reconnect.
> - Drop Kuniyuki's Reviewed-by tag due to the code change.
>
> v2: https://lore.kernel.org/netdev/20261101182736.430733-1-zhiyuzhang999@gmail.com/
> v1: https://lore.kernel.org/netdev/20261030032314.1978575-1-zhiyuzhang999@gmail.com/
>
>  net/ipv4/tcp_ipv4.c  |  5 +++++
>  net/ipv6/tcp_ipv6.c  | 37 ++++++++++++++++++++++++++++---------
>  net/mptcp/protocol.c |  4 ++--
>  3 files changed, 35 insertions(+), 11 deletions(-)
>
> diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
> index a7f9242f1e42..8fa6564d2b60 100644
> --- a/net/ipv4/tcp_ipv4.c
> +++ b/net/ipv4/tcp_ipv4.c
> @@ -1836,6 +1836,11 @@ int tcp_v4_do_rcv(struct sock *sk, struct sk_buff *skb)
>  {
>         enum skb_drop_reason reason;
>
> +       if (unlikely(skb->protocol != htons(ETH_P_IP))) {
> +               reason = SKB_DROP_REASON_INVALID_PROTO;
> +               goto err_discard;
> +       }
> +

We do not want to add yet another check in TCP IPv4 fast path,
for a problem that only exists for IPv6 sockets.

tcp_v6_do_rcv() already dispatches IPv4 skbs to tcp_v4_do_rcv().

Simply stop changing sk->sk_backlog_rcv for v4-mapped sockets
(tcp_v6_connect() and tcp_v6_mapped_child_init()),
so that skbs found in the backlog are always handled by the correct path.

tcp_v6_disconnect() can then test
inet_csk(sk)->icsk_af_ops->net_header_len == sizeof(struct iphdr)
(this also covers subflow_v6m_specific).

Thanks.

Untested patch:

diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index e006a30360eae1b43a4d53173b46b931ae668825..74358bed6ba210074f0534dff61247de34a56727
100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -130,6 +130,18 @@ static int tcp_v6_pre_connect(struct sock *sk,
struct sockaddr_unsized *uaddr,
        return BPF_CGROUP_RUN_PROG_INET6_CONNECT(sk, uaddr, &addr_len);
 }

+/* Undo the v4-mapped setup done by tcp_v6_connect(). */
+static void tcp_v6_restore_af_ops(struct sock *sk)
+{
+       /* Paired with READ_ONCE() in tcp_(get|set)sockopt() */
+       WRITE_ONCE(inet_csk(sk)->icsk_af_ops, &ipv6_specific);
+       if (sk_is_mptcp(sk))
+               mptcpv6_handle_mapped(sk, false);
+#if defined(CONFIG_TCP_MD5SIG) || defined(CONFIG_TCP_AO)
+       tcp_sk(sk)->af_specific = &tcp_sock_ipv6_specific;
+#endif
+}
+
 static int tcp_v6_connect(struct sock *sk, struct sockaddr_unsized *uaddr,
                          int addr_len)
 {
@@ -234,7 +246,6 @@ static int tcp_v6_connect(struct sock *sk, struct
sockaddr_unsized *uaddr,
                WRITE_ONCE(icsk->icsk_af_ops, &ipv6_mapped);
                if (sk_is_mptcp(sk))
                        mptcpv6_handle_mapped(sk, true);
-               sk->sk_backlog_rcv = tcp_v4_do_rcv;
 #if defined(CONFIG_TCP_MD5SIG) || defined(CONFIG_TCP_AO)
                tp->af_specific = &tcp_sock_ipv6_mapped_specific;
 #endif
@@ -243,14 +254,7 @@ static int tcp_v6_connect(struct sock *sk, struct
sockaddr_unsized *uaddr,

                if (err) {
                        icsk->icsk_ext_hdr_len = exthdrlen;
-                       /* Paired with READ_ONCE() in tcp_(get|set)sockopt() */
-                       WRITE_ONCE(icsk->icsk_af_ops, &ipv6_specific);
-                       if (sk_is_mptcp(sk))
-                               mptcpv6_handle_mapped(sk, false);
-                       sk->sk_backlog_rcv = tcp_v6_do_rcv;
-#if defined(CONFIG_TCP_MD5SIG) || defined(CONFIG_TCP_AO)
-                       tp->af_specific = &tcp_sock_ipv6_specific;
-#endif
+                       tcp_v6_restore_af_ops(sk);
                        goto failure;
                }
                np->saddr = sk->sk_v6_rcv_saddr;
@@ -1363,7 +1367,6 @@ static void tcp_v6_mapped_child_init(struct sock
*newsk, const struct sock *sk)
        inet_csk(newsk)->icsk_af_ops = &ipv6_mapped;
        if (sk_is_mptcp(newsk))
                mptcpv6_handle_mapped(newsk, true);
-       newsk->sk_backlog_rcv = tcp_v4_do_rcv;
 #if defined(CONFIG_TCP_MD5SIG) || defined(CONFIG_TCP_AO)
        tcp_sk(newsk)->af_specific = &tcp_sock_ipv6_mapped_specific;
 #endif
@@ -2093,6 +2096,21 @@ static int tcp_v6_init_sock(struct sock *sk)
        return 0;
 }

+static int tcp_v6_disconnect(struct sock *sk, int flags)
+{
+       int ret = tcp_disconnect(sk, flags);
+
+       /* A socket used for a v4-mapped connection must not keep IPv4
+        * handlers when it is reused for listen() or a native IPv6 connect().
+        * Note that ipv6_mapped and subflow_v6m_specific both use an IPv4
+        * network header.
+        */
+       if (inet_csk(sk)->icsk_af_ops->net_header_len == sizeof(struct iphdr))
+               tcp_v6_restore_af_ops(sk);
+
+       return ret;
+}
+
 #ifdef CONFIG_PROC_FS
 /* Proc filesystem TCPv6 sock list dumping. */
 static void get_openreq6(struct seq_file *seq,
@@ -2283,7 +2301,7 @@ struct proto tcpv6_prot = {
        .close                  = tcp_close,
        .pre_connect            = tcp_v6_pre_connect,
        .connect                = tcp_v6_connect,
-       .disconnect             = tcp_disconnect,
+       .disconnect             = tcp_v6_disconnect,
        .accept                 = inet_csk_accept,
        .ioctl                  = tcp_ioctl,
        .init                   = tcp_v6_init_sock,
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 7a22e9388e0e9a8b1bd6fe27d04245a1db6950b1..fdfbc7de2ec03612bc49d0a337a80fe1089a7d81
100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -2608,7 +2608,7 @@ static void __mptcp_subflow_disconnect(struct sock *ssk,
                /* The MPTCP code never wait on the subflow sockets, TCP-level
                 * disconnect should never fail
                 */
-               WARN_ON_ONCE(tcp_disconnect(ssk, 0));
+               WARN_ON_ONCE(ssk->sk_prot->disconnect(ssk, 0));
                mptcp_subflow_ctx_reset(subflow);
        } else {
                tcp_shutdown(ssk, SEND_SHUTDOWN);
@@ -3287,7 +3287,7 @@ void mptcp_subflow_shutdown(struct sock *sk,
struct sock *ssk, int how)
                        break;
                fallthrough;
        case TCP_SYN_SENT:
-               WARN_ON_ONCE(tcp_disconnect(ssk, O_NONBLOCK));
+               WARN_ON_ONCE(ssk->sk_prot->disconnect(ssk, O_NONBLOCK));
                break;
        default:
                if (__mptcp_check_fallback(mptcp_sk(sk))) {

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH net v3] tcp: prevent cross-family skb processing after disconnect
  2026-11-03 11:03 ` Eric Dumazet
@ 2026-11-04 19:19   ` Zhiyu Zhang
  0 siblings, 0 replies; 4+ messages in thread
From: Zhiyu Zhang @ 2026-11-04 19:19 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Kuniyuki Iwashima, Neal Cardwell, David S. Miller,
	Jakub Kicinski, Paolo Abeni, Simon Horman, Matthieu Baerts,
	Mat Martineau, Geliang Tang, netdev, mptcp, linux-kernel,
	Tong Liu, Qi Tang

Eric Dumazet <edumazet@kernel.org> 于2026年11月3日周二 19:03写道:
>
> We do not want to add yet another check in TCP IPv4 fast path,
> for a problem that only exists for IPv6 sockets.
>
> tcp_v6_do_rcv() already dispatches IPv4 skbs to tcp_v4_do_rcv().
>
> Simply stop changing sk->sk_backlog_rcv for v4-mapped sockets
> (tcp_v6_connect() and tcp_v6_mapped_child_init()),
> so that skbs found in the backlog are always handled by the correct path.
>
> tcp_v6_disconnect() can then test
> inet_csk(sk)->icsk_af_ops->net_header_len == sizeof(struct iphdr)
> (this also covers subflow_v6m_specific).
>
> Thanks.
>
> Untested patch:

Hi Eric,

Thanks. I applied your untested patch as-is on top of 5838f10563d5.

I agree that keeping tcp_v6_do_rcv() as the backlog handler for
AF_INET6 sockets is cleaner than adding another check to the IPv4 fast
path.

The patch fixes the reported bug in my tests. In the native IPv6 to
v4-mapped race, the stale IPv6 skb no longer reaches
tcp_v4_send_reset(). The disconnect/reconnect tests also pass for both
TCP and MPTCP.

I also checked the two races reported by Sashiko. On one v3 run with
400 reconnects in each direction, both races were hit four times.

For a native IPv6 to v4-mapped reconnect, I observed:

  tcp_v6_rcv()
    tcp_v6_do_rcv()
      tcp_rcv_established()
        inet_sk_rx_dst_set()          /* IPv6 skb */

For the reverse reconnect, I observed:

  tcp_v4_rcv()
    tcp_v4_do_rcv()
      tcp_rcv_established()
        inet6_sk_rx_dst_set()         /* IPv4 skb */

I added probes after the rcu_assign_pointer(sk->sk_rx_dst, dst)
instructions and confirmed that the mismatched dst was actually stored.
Both cases can also be reproduced with your patch.

So the receive paths and the cross-family dst assignments are real.
However, I have not reproduced the Critical outcome suggested by the
review. I saw no warning, oops or panic, and did not observe an IPv6
skb reaching ip_local_deliver() or an IPv4 skb reaching ip6_input().
For the local IPv4 input route in this test, the bytes overlapping
rt6_info::sernum are non-zero due to rtable::rt_type/rt_is_input, so
the invalid rt->from dereference did not occur.

Given that, I think your patch is still the right fix for the reported
header.h6/header.h4 bug. I would post it as v4 with that scope, and
handle the stale skb/sk_rx_dst race separately instead of adding the
IPv4 fast-path check back.

Does that sound like the right split, or do you think the sk_rx_dst
race should be addressed before v4 or should not even take care?

Thanks,
Zhiyu

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH net v3] tcp: prevent cross-family skb processing after disconnect
  2026-11-03 10:36 [PATCH net v3] tcp: prevent cross-family skb processing after disconnect Zhiyu Zhang
  2026-11-03 11:03 ` Eric Dumazet
@ 2026-11-05 22:39 ` netdev-bot+sashiko
  1 sibling, 0 replies; 4+ messages in thread
From: netdev-bot+sashiko @ 2026-11-05 22:39 UTC (permalink / raw)
  To: zhiyuzhang999
  Cc: edumazet, kuniyu, ncardwell, davem, kuba, pabeni, horms, matttbe,
	martineau, geliang, netdev, mptcp, linux-kernel, lyutoon,
	tpluszz77

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

Critical: 0 · High: 1 · Medium: 1 · 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 patch blocks only one direction of the stale-skb race (an
  IPv6 skb reaching tcp_v4_do_rcv()).
- [Medium] The commit message says that in the native-to-mapped race the
  stale IPv6 skb 'belongs to the disconnected connection, so drop it'.

--

tcp: prevent cross-family skb processing after disconnect

This commit adds tcp_v6_disconnect(), which puts back the IPv6 af_ops,
backlog_rcv and af_specific handlers after a v4-mapped socket is
disconnected. It also makes tcp_v4_do_rcv() drop skbs that are not
ETH_P_IP, and sends the MPTCP subflow teardown paths through
sk_prot->disconnect.

Link: https://lore.kernel.org/netdev/CAL4WiiqFJpmcV4=TWRpukCq6q=-729658FwGvieNcPXe97kLvQ@mail.gmail.com/

> diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
> index a7f9242f1e429..8fa6564d2b602 100644
> --- a/net/ipv4/tcp_ipv4.c
> +++ b/net/ipv4/tcp_ipv4.c
> @@ -1836,6 +1836,11 @@ int tcp_v4_do_rcv(struct sock *sk, struct sk_buff *skb)
>  {
>  	enum skb_drop_reason reason;
>  
> +	if (unlikely(skb->protocol != htons(ETH_P_IP))) {
> +		reason = SKB_DROP_REASON_INVALID_PROTO;
> +		goto err_discard;
> +	}
> +

[Severity: Medium]
The commit message says:

  In the reverse race, the skb belongs to the disconnected connection, so
  drop it instead of redirecting it to the IPv6 receive path.

Is the skb dropped when the socket is not owned by the user? This check
only runs when the skb goes through sk->sk_backlog_rcv. The unowned branch
of tcp_v6_rcv() calls tcp_v6_do_rcv() directly:

net/ipv6/tcp_ipv6.c:tcp_v6_rcv() {
    ...
	bh_lock_sock_nested(sk);
	tcp_segs_in(tcp_sk(sk), skb);
	ret = 0;
	if (!sock_owned_by_user(sk)) {
		ret = tcp_v6_do_rcv(sk, skb);
    ...
}

tcp_v6_do_rcv() only diverts ETH_P_IP skbs:

	if (skb->protocol == htons(ETH_P_IP))
		return tcp_v4_do_rcv(sk, skb);

Take an IPv6 skb that was looked up before connect(AF_UNSPEC), where the
socket is reconnected to a v4-mapped peer before bh_lock_sock_nested().
That skb goes through tcp_v6_do_rcv() and tcp_rcv_state_process() on the
new connection, whose icsk_af_ops is now &ipv6_mapped.

In SYN_SENT, can an IPv6 SYN-ACK carrying the new ISN (old write_seq +
max_window + 2) complete the mapped connection? If so, tcp_finish_connect()
calls:

	icsk->icsk_af_ops->sk_rx_dst_set(sk, skb);

For ipv6_mapped that callback is inet_sk_rx_dst_set(), so an IPv6 rt6_info
gets cached as sk_rx_dst on the v4-mapped socket.

The __ip_options_echo() overflow is not reached on this path, because
resets go out through tcp_v6_send_reset(). I could not confirm whether the
cached IPv6 dst is later reused for IPv4 skbs. That depends on
ip6_dst_check() accepting a zero cookie.

Even so, the stale skb can still change the state of the new connection.
That contradicts the drop described in the commit message.

> diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
> index e006a30360eae..a93410aab66e6 100644
> --- a/net/ipv6/tcp_ipv6.c
> +++ b/net/ipv6/tcp_ipv6.c

[ ... ]

> @@ -2093,6 +2099,19 @@ static int tcp_v6_init_sock(struct sock *sk)
>  	return 0;
>  }
>  
> +static int tcp_v6_disconnect(struct sock *sk, int flags)
> +{
> +	int ret = tcp_disconnect(sk, flags);
> +
> +	/* A socket used for a v4-mapped connection must not keep IPv4
> +	 * handlers when it is reused for listen() or a native IPv6 connect().
> +	 */
> +	if (sk->sk_backlog_rcv == tcp_v4_do_rcv)
> +		tcp_v6_restore_af_ops(sk);

[Severity: High]
After this restore, can a stale IPv4 skb be processed with IPv6 af_ops?
The new check in tcp_v4_do_rcv() only looks at skb->protocol. It does not
check whether the socket is still in v4-mapped mode.

A possible sequence:

1. An AF_INET6 socket is connected to a v4-mapped peer. tcp_v4_rcv() looks
   it up for an IPv4 segment.

2. tcp_v4_rcv() runs tcp_inbound_hash() and tcp_filter() before
   bh_lock_sock_nested(). A socket filter can widen this window, as the
   commit message describes.

3. Inside that window, connect(AF_UNSPEC) calls tcp_v6_disconnect(). That
   restores ipv6_specific, tcp_v6_do_rcv and tcp_sock_ipv6_specific. The
   socket is then connected to a native IPv6 peer.

4. The stale IPv4 skb reaches tcp_v4_do_rcv() in one of two ways. If the
   socket is not owned, tcp_v4_rcv() calls it directly. Otherwise the
   backlog drain goes through tcp_v6_do_rcv(), which forwards any ETH_P_IP
   skb without checking the socket's mode:

	if (skb->protocol == htons(ETH_P_IP))
		return tcp_v4_do_rcv(sk, skb);

5. The skb really is IPv4, so the new ETH_P_IP check lets it through.

On a SYN-ACK in SYN_SENT, tcp_finish_connect() calls
icsk_af_ops->sk_rx_dst_set(sk, skb). tcp_rcv_established() makes the same
call before any sequence check when sk_rx_dst is NULL:

	if (unlikely(!rcu_access_pointer(sk->sk_rx_dst)))
		inet_csk(sk)->icsk_af_ops->sk_rx_dst_set(sk, skb);

After the restore, that callback is inet6_sk_rx_dst_set():

	if (dst && dst_hold_safe(dst)) {
		rcu_assign_pointer(sk->sk_rx_dst, dst);
		sk->sk_rx_dst_ifindex = skb->skb_iif;
		sk->sk_rx_dst_cookie = rt6_get_cookie(dst_rt6_info(dst));
	}

This stores an IPv4 rtable as sk_rx_dst and reads it as an rt6_info in
rt6_get_cookie().

Could later IPv6 segments then get this IPv4 dst attached through early
demux? ipv4_dst_check() ignores the cookie, so dst_input() could end up
calling ip_local_deliver() on IPv6 packets. I have not traced this last
step completely.

Before this patch, the socket kept ipv6_mapped ops, so an IPv4 skb still
matched them. With the restore on the normal disconnect path, an IPv4 skb
can now meet IPv6 af_ops, and nothing guards against that.

Would it be better to compare the skb family against the socket's
mapped/native mode? For example, ETH_P_IP skbs could be rejected on a
non-listening AF_INET6 socket whose icsk_af_ops != &ipv6_mapped.
Alternatively, the socket could be revalidated after bh_lock_sock_nested().

> +
> +	return ret;
> +}

[ ... ]

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-11-05 22:39 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-11-03 10:36 [PATCH net v3] tcp: prevent cross-family skb processing after disconnect Zhiyu Zhang
2026-11-03 11:03 ` Eric Dumazet
2026-11-04 19:19   ` Zhiyu Zhang
2026-11-05 22:39 ` netdev-bot+sashiko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®