include/net/inet_connection_sock.h | 4 +- include/net/tcp.h | 4 +- net/ipv4/syncookies.c | 2 +- net/ipv4/tcp_fastopen.c | 2 +- net/ipv4/tcp_ipv4.c | 8 ++- net/ipv4/tcp_minisocks.c | 2 +- net/ipv6/tcp_ipv6.c | 97 +++++++++++++----------------- net/mptcp/subflow.c | 6 +- net/smc/af_smc.c | 6 +- 9 files changed, 66 insertions(+), 65 deletions(-)
From: Eric Dumazet <edumazet@google.com>
Code in tcp_v6_syn_recv_sock() after the call to tcp_v4_syn_recv_sock()
is done too late.
After tcp_v4_syn_recv_sock(), the child socket is already visible
from TCP ehash table and other cpus might use it.
Since newinet->pinet6 is still pointing to the listener ipv6_pinfo
bad things can happen as syzbot found.
Move the problematic code in tcp_v6_mapped_child_init()
and call this new helper from tcp_v4_syn_recv_sock() before
the ehash insertion.
This allows the removal of one tcp_sync_mss(), since
tcp_v4_syn_recv_sock() will call it with the correct
context.
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Reported-by: syzbot+937b5bbb6a815b3e5d0b@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/netdev/69949275.050a0220.2eeac1.0145.GAE@google.com/
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reviewed-by: Kuniyuki Iwashima <kuniyu@google.com>
Link: https://patch.msgid.link/20260217161205.2079883-1-edumazet@google.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
(cherry picked from commit 858d2a4f67ff69e645a43487ef7ea7f28f06deae)
[florian:
- net/ipv6/tcp_ipv6.c:
- Set `newnp->ipv6_fl_list = NULL` instead of `newinet->ipv6_fl_list = NULL`,
as `ipv6_fl_list` is in `struct ipv6_pinfo`.
- Guarded `af_specific` assignment with `#ifdef CONFIG_TCP_MD5SIG` instead
of checking `CONFIG_TCP_AO`.
- Used `if (tcp_inet6_sk(sk)->repflow)` instead of `inet6_test_bit(REPFLOW, sk)`.]
Assisted-by: Cursor:gemini-3.7-flash
Signed-off-by: Florian Fainelli <florian.fainelli@broadcom.com>
---
include/net/inet_connection_sock.h | 4 +-
include/net/tcp.h | 4 +-
net/ipv4/syncookies.c | 2 +-
net/ipv4/tcp_fastopen.c | 2 +-
net/ipv4/tcp_ipv4.c | 8 ++-
net/ipv4/tcp_minisocks.c | 2 +-
net/ipv6/tcp_ipv6.c | 97 +++++++++++++-----------------
net/mptcp/subflow.c | 6 +-
net/smc/af_smc.c | 6 +-
9 files changed, 66 insertions(+), 65 deletions(-)
diff --git a/include/net/inet_connection_sock.h b/include/net/inet_connection_sock.h
index 3eb715f66cbf..b7935e293757 100644
--- a/include/net/inet_connection_sock.h
+++ b/include/net/inet_connection_sock.h
@@ -42,7 +42,9 @@ struct inet_connection_sock_af_ops {
struct request_sock *req,
struct dst_entry *dst,
struct request_sock *req_unhash,
- bool *own_req);
+ bool *own_req,
+ void (*opt_child_init)(struct sock *newsk,
+ const struct sock *sk));
u16 net_header_len;
u16 net_frag_header_len;
u16 sockaddr_len;
diff --git a/include/net/tcp.h b/include/net/tcp.h
index 23d830a7a6c8..7392f51a3479 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -461,7 +461,9 @@ struct sock *tcp_v4_syn_recv_sock(const struct sock *sk, struct sk_buff *skb,
struct request_sock *req,
struct dst_entry *dst,
struct request_sock *req_unhash,
- bool *own_req);
+ bool *own_req,
+ void (*opt_child_init)(struct sock *newsk,
+ const struct sock *sk));
int tcp_v4_do_rcv(struct sock *sk, struct sk_buff *skb);
int tcp_v4_connect(struct sock *sk, struct sockaddr *uaddr, int addr_len);
int tcp_connect(struct sock *sk);
diff --git a/net/ipv4/syncookies.c b/net/ipv4/syncookies.c
index e14356207795..9deeb3f8215e 100644
--- a/net/ipv4/syncookies.c
+++ b/net/ipv4/syncookies.c
@@ -199,7 +199,7 @@ struct sock *tcp_get_cookie_sock(struct sock *sk, struct sk_buff *skb,
bool own_req;
child = icsk->icsk_af_ops->syn_recv_sock(sk, skb, req, dst,
- NULL, &own_req);
+ NULL, &own_req, NULL);
if (child) {
refcount_set(&req->rsk_refcnt, 1);
tcp_sk(child)->tsoffset = tsoff;
diff --git a/net/ipv4/tcp_fastopen.c b/net/ipv4/tcp_fastopen.c
index 408985eb74ee..51bd11588d6d 100644
--- a/net/ipv4/tcp_fastopen.c
+++ b/net/ipv4/tcp_fastopen.c
@@ -247,7 +247,7 @@ static struct sock *tcp_fastopen_create_child(struct sock *sk,
bool own_req;
child = inet_csk(sk)->icsk_af_ops->syn_recv_sock(sk, skb, req, NULL,
- NULL, &own_req);
+ NULL, &own_req, NULL);
if (!child)
return NULL;
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 3f9e1cfde008..163c8726a17d 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -1566,7 +1566,9 @@ struct sock *tcp_v4_syn_recv_sock(const struct sock *sk, struct sk_buff *skb,
struct request_sock *req,
struct dst_entry *dst,
struct request_sock *req_unhash,
- bool *own_req)
+ bool *own_req,
+ void (*opt_child_init)(struct sock *newsk,
+ const struct sock *sk))
{
struct inet_request_sock *ireq;
bool found_dup_sk = false;
@@ -1622,6 +1624,10 @@ struct sock *tcp_v4_syn_recv_sock(const struct sock *sk, struct sk_buff *skb,
}
sk_setup_caps(newsk, dst);
+#if IS_ENABLED(CONFIG_IPV6)
+ if (opt_child_init)
+ opt_child_init(newsk, sk);
+#endif
tcp_ca_openreq_child(newsk, dst);
tcp_sync_mss(newsk, dst_mtu(dst));
diff --git a/net/ipv4/tcp_minisocks.c b/net/ipv4/tcp_minisocks.c
index 86f0feb2497f..d6d64697ce89 100644
--- a/net/ipv4/tcp_minisocks.c
+++ b/net/ipv4/tcp_minisocks.c
@@ -817,7 +817,7 @@ struct sock *tcp_check_req(struct sock *sk, struct sk_buff *skb,
* socket is created, wait for troubles.
*/
child = inet_csk(sk)->icsk_af_ops->syn_recv_sock(sk, skb, req, NULL,
- req, &own_req);
+ req, &own_req, NULL);
if (!child)
goto listen_overflow;
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index 689c0b383ebf..f13fdd1214aa 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -1180,11 +1180,48 @@ static void tcp_v6_restore_cb(struct sk_buff *skb)
sizeof(struct inet6_skb_parm));
}
+/* Called from tcp_v4_syn_recv_sock() for v6_mapped children. */
+static void tcp_v6_mapped_child_init(struct sock *newsk, const struct sock *sk)
+{
+ struct inet_sock *newinet = inet_sk(newsk);
+ struct ipv6_pinfo *newnp;
+
+ newinet->pinet6 = newnp = tcp_inet6_sk(newsk);
+
+ memcpy(newnp, tcp_inet6_sk(sk), sizeof(struct ipv6_pinfo));
+
+ newnp->saddr = newsk->sk_v6_rcv_saddr;
+
+ 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;
+#ifdef CONFIG_TCP_MD5SIG
+ tcp_sk(newsk)->af_specific = &tcp_sock_ipv6_mapped_specific;
+#endif
+
+ newnp->ipv6_mc_list = NULL;
+ newnp->ipv6_ac_list = NULL;
+ newnp->ipv6_fl_list = NULL;
+ newnp->pktoptions = NULL;
+ newnp->opt = NULL;
+
+ /* tcp_v4_syn_recv_sock() has initialized newinet->mc_{index,ttl} */
+ newnp->mcast_oif = newinet->mc_index;
+ newnp->mcast_hops = newinet->mc_ttl;
+
+ newnp->rcv_flowinfo = 0;
+ if (tcp_inet6_sk(sk)->repflow)
+ newnp->flow_label = 0;
+}
+
static struct sock *tcp_v6_syn_recv_sock(const struct sock *sk, struct sk_buff *skb,
struct request_sock *req,
struct dst_entry *dst,
struct request_sock *req_unhash,
- bool *own_req)
+ bool *own_req,
+ void (*opt_child_init)(struct sock *newsk,
+ const struct sock *sk))
{
struct inet_request_sock *ireq;
struct ipv6_pinfo *newnp;
@@ -1200,60 +1237,10 @@ static struct sock *tcp_v6_syn_recv_sock(const struct sock *sk, struct sk_buff *
#endif
struct flowi6 fl6;
- if (skb->protocol == htons(ETH_P_IP)) {
- /*
- * v6 mapped
- */
-
- newsk = tcp_v4_syn_recv_sock(sk, skb, req, dst,
- req_unhash, own_req);
-
- if (!newsk)
- return NULL;
-
- inet_sk(newsk)->pinet6 = tcp_inet6_sk(newsk);
-
- newnp = tcp_inet6_sk(newsk);
- newtp = tcp_sk(newsk);
-
- memcpy(newnp, np, sizeof(struct ipv6_pinfo));
-
- newnp->saddr = newsk->sk_v6_rcv_saddr;
-
- 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;
-#ifdef CONFIG_TCP_MD5SIG
- newtp->af_specific = &tcp_sock_ipv6_mapped_specific;
-#endif
-
- newnp->ipv6_mc_list = NULL;
- newnp->ipv6_ac_list = NULL;
- newnp->ipv6_fl_list = NULL;
- newnp->pktoptions = NULL;
- newnp->opt = NULL;
- newnp->mcast_oif = inet_iif(skb);
- newnp->mcast_hops = ip_hdr(skb)->ttl;
- newnp->rcv_flowinfo = 0;
- if (np->repflow)
- newnp->flow_label = 0;
-
- /*
- * No need to charge this sock to the relevant IPv6 refcnt debug socks count
- * here, tcp_create_openreq_child now does this for us, see the comment in
- * that function for the gory details. -acme
- */
-
- /* It is tricky place. Until this moment IPv4 tcp
- worked with IPv6 icsk.icsk_af_ops.
- Sync it now.
- */
- tcp_sync_mss(newsk, inet_csk(newsk)->icsk_pmtu_cookie);
-
- return newsk;
- }
-
+ if (skb->protocol == htons(ETH_P_IP))
+ return tcp_v4_syn_recv_sock(sk, skb, req, dst,
+ req_unhash, own_req,
+ tcp_v6_mapped_child_init);
ireq = inet_rsk(req);
if (sk_acceptq_is_full(sk))
diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index d3b5c2d48b53..db2e3a1ebb6e 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -788,7 +788,9 @@ static struct sock *subflow_syn_recv_sock(const struct sock *sk,
struct request_sock *req,
struct dst_entry *dst,
struct request_sock *req_unhash,
- bool *own_req)
+ bool *own_req,
+ void (*opt_child_init)(struct sock *newsk,
+ const struct sock *sk))
{
struct mptcp_subflow_context *listener = mptcp_subflow_ctx(sk);
struct mptcp_subflow_request_sock *subflow_req;
@@ -834,7 +836,7 @@ static struct sock *subflow_syn_recv_sock(const struct sock *sk,
create_child:
child = listener->icsk_af_ops->syn_recv_sock(sk, skb, req, dst,
- req_unhash, own_req);
+ req_unhash, own_req, opt_child_init);
if (child && *own_req) {
struct mptcp_subflow_context *ctx = mptcp_subflow_ctx(child);
diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
index 087102ff3c64..dba856528890 100644
--- a/net/smc/af_smc.c
+++ b/net/smc/af_smc.c
@@ -119,7 +119,9 @@ static struct sock *smc_tcp_syn_recv_sock(const struct sock *sk,
struct request_sock *req,
struct dst_entry *dst,
struct request_sock *req_unhash,
- bool *own_req)
+ bool *own_req,
+ void (*opt_child_init)(struct sock *newsk,
+ const struct sock *sk))
{
struct smc_sock *smc;
struct sock *child;
@@ -144,7 +146,7 @@ static struct sock *smc_tcp_syn_recv_sock(const struct sock *sk,
/* passthrough to original syn recv sock fct */
child = smc->ori_af_ops->syn_recv_sock(sk, skb, req, dst, req_unhash,
- own_req);
+ own_req, opt_child_init);
/* child must not inherit smc or its ops */
if (child) {
rcu_assign_sk_user_data(child, NULL);
--
2.34.1Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
New issues:
- [Low] The DCCP subsystem was not updated to handle the new `syn_recv_sock` function pointer signature, resulting in a build failure and Control Flow Integrity (CFI) violations.
Pre-existing issues:
- [Critical] Use-After-Free and Double-Free of listener's `ipv6_pinfo` on the `tcp_v4_syn_recv_sock` error path.
- [High] The patch leaves the identical ehash race condition completely unfixed in the DCCP subsystem.
- [High] The `opt_child_init` callback is non-composable and ignored by IPv6, leaving wrapper ULPs (SMC, MPTCP) vulnerable to the same ehash race condition.
--
commit 1232c8e7afdc63a365473845ad392043814df4fb
Author: Eric Dumazet <edumazet@google.com>
tcp: fix potential race in tcp_v6_syn_recv_sock()
This commit fixes a race condition where a child socket is inserted into the
TCP ehash table while its newinet->pinet6 still points to the listener's
IPv6 info. It introduces a new opt_child_init callback parameter to
syn_recv_sock to perform the initialization before ehash insertion.
Link: https://patch.msgid.link/20260217161205.2079883-1-edumazet@google.com
> diff --git a/include/net/inet_connection_sock.h b/include/net/inet_connection_sock.h
> --- a/include/net/inet_connection_sock.h
> +++ b/include/net/inet_connection_sock.h
> @@ -42,7 +42,9 @@ struct inet_connection_sock_af_ops {
> struct request_sock *req,
> struct dst_entry *dst,
> struct request_sock *req_unhash,
> - bool *own_req);
> + bool *own_req,
> + void (*opt_child_init)(struct sock *newsk,
> + const struct sock *sk));
[Severity: Low]
Does this break the DCCP subsystem?
The DCCP implementations of syn_recv_sock (dccp_v4_request_recv_sock and
dccp_v6_request_recv_sock) were not updated to accept this new 7th parameter.
Since dccp_check_req() still passes 6 arguments, this appears to cause a
direct compilation error (too few arguments). If compiled with loose
warnings and kCFI enabled, it could also cause pointer mismatch panics at
runtime.
[Severity: High]
This is a pre-existing issue, but does this leave the identical ehash race
condition unfixed in DCCP?
dccp_v6_request_recv_sock() calls dccp_v4_request_recv_sock(), which inserts
the socket into the ehash via inet_ehash_nolisten(). Only after it returns
does dccp_v6_request_recv_sock() initialize newinet->pinet6. A lockless
reader (like sock_diag or BPF) can access the socket from the ehash in this
window and observe the listener's pinet6 pointer, which is exactly the race
condition this patch fixes for TCP.
[ ... ]
> diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
> --- a/net/ipv4/tcp_ipv4.c
> +++ b/net/ipv4/tcp_ipv4.c
> @@ -1622,6 +1624,10 @@ struct sock *tcp_v4_syn_recv_sock(const struct sock *sk, struct sk_buff *skb,
> }
> sk_setup_caps(newsk, dst);
>
> +#if IS_ENABLED(CONFIG_IPV6)
> + if (opt_child_init)
> + opt_child_init(newsk, sk);
> +#endif
[Severity: Critical]
This is a pre-existing issue, but is there a use-after-free of the listener's
ipv6_pinfo if routing fails earlier in this function?
If inet_csk_route_child_sock() fails before sk_setup_caps() and this new
opt_child_init() call:
if (!dst) {
dst = inet_csk_route_child_sock(sk, newsk, req);
if (!dst)
goto put_and_exit;
}
The code jumps to put_and_exit, which destroys the child socket:
put_and_exit:
newinet->inet_opt = NULL;
inet_csk_prepare_forced_close(newsk);
tcp_done(newsk);
goto exit;
Since put_and_exit calls tcp_done() before opt_child_init() has repointed
pinet6 to the child's own memory, pinet6 still points to the listener. The
destruction path will free the listener's pktoptions, rxpmtu, and opt while
the listener is still active, causing a double-free memory corruption.
[ ... ]
> diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
> --- a/net/ipv6/tcp_ipv6.c
> +++ b/net/ipv6/tcp_ipv6.c
> @@ -1200,60 +1237,10 @@ static struct sock *tcp_v6_syn_recv_sock(const struct sock *sk, struct sk_buff *
> #endif
> struct flowi6 fl6;
>
> - if (skb->protocol == htons(ETH_P_IP)) {
[ ... ]
> + if (skb->protocol == htons(ETH_P_IP))
> + return tcp_v4_syn_recv_sock(sk, skb, req, dst,
> + req_unhash, own_req,
> + tcp_v6_mapped_child_init);
[Severity: High]
This is a pre-existing issue, but by overriding opt_child_init with a
hardcoded tcp_v6_mapped_child_init callback here, does this leave wrapper
ULPs like SMC and MPTCP vulnerable to the same ehash race condition?
Because tcp_v6_syn_recv_sock() ignores the passed opt_child_init callback for
IPv6 (and hardcodes it for v4-mapped), wrapper ULPs cannot use it to
initialize their state before ehash insertion.
[ ... ]
> diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
> --- a/net/mptcp/subflow.c
> +++ b/net/mptcp/subflow.c
> @@ -834,7 +836,7 @@ static struct sock *subflow_syn_recv_sock(const struct sock *sk,
>
> create_child:
> child = listener->icsk_af_ops->syn_recv_sock(sk, skb, req, dst,
> - req_unhash, own_req);
> + req_unhash, own_req, opt_child_init);
[Severity: High]
This is a pre-existing issue, but since MPTCP mutates ctx->conn via
mptcp_sk_clone_init() after syn_recv_sock() returns, does this create a race
window where lockless readers can access the listener's state on the newly
hashed child?
This seems especially problematic for IPv6 because the passed opt_child_init
callback is completely ignored by tcp_v6_syn_recv_sock().
[ ... ]
> diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
> --- a/net/smc/af_smc.c
> +++ b/net/smc/af_smc.c
> @@ -144,7 +146,7 @@ static struct sock *smc_tcp_syn_recv_sock(const struct sock *sk,
>
> /* passthrough to original syn recv sock fct */
> child = smc->ori_af_ops->syn_recv_sock(sk, skb, req, dst, req_unhash,
> - own_req);
> + own_req, opt_child_init);
> /* child must not inherit smc or its ops */
> if (child) {
> rcu_assign_sk_user_data(child, NULL);
[Severity: High]
This is a pre-existing issue, but does clearing the inherited user data via
rcu_assign_sk_user_data(child, NULL) here occur too late?
Because this happens after syn_recv_sock() has already hashed the child
socket globally, there is a window where lockless readers can access the
listener's SMC state on the child socket. Again, this is exacerbated for IPv6
where the opt_child_init callback is ignored.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260903000029.3508006-4-florian.fainelli@broadcom.com?part=1
© 2016 - 2026 Red Hat, Inc.