[PATCH net 0/2] tcp: initialize missing request_sock fields in syncookie path

[PATCH net 0/2] tcp: initialize missing request_sock fields in syncookie path

[Thaison Phan]

Initialize missing request_sock fields for TCP connections accepted via
SYN cookies, where cookie_tcp_reqsk_alloc() and
bpf_sk_assign_tcp_reqsk() allocate without __GFP_ZERO:

- Patch 1 initializes ireq->ir_rmt_addr and ireq->ir_loc_addr to
  LOOPBACK4_IPV6 in cookie_v6_check(), matching tcp_v6_init_req().
- Patch 2 initializes the AccECN request_sock fields in
  cookie_tcp_reqsk_init() and sets syn_ect_snt in cookie_{v4,v6}_check(),
  matching tcp_openreq_init() and tcp_{v4,v6}_send_synack().

An AI coding assistant helped audit the request_sock initialization
paths, write the QEMU reproducer, and draft the changelogs.

Thaison Phan (2):
  tcp: initialize ir_rmt_addr and ir_loc_addr in cookie_v6_check()
  tcp: accecn: initialize AccECN fields in syncookie path

 net/ipv4/syncookies.c | 5 +++++
 net/ipv6/syncookies.c | 4 ++++
 2 files changed, 9 insertions(+)


base-commit: 19ec8416a76c08326fe5f814410a1b43046e63be
-- 
2.56.0.611.g1030f0b940-goog

[PATCH net 1/2] tcp: initialize ir_rmt_addr and ir_loc_addr in cookie_v6_check()

[Thaison Phan]

Commit a3a128f611a9 ("inet: consolidate inet_csk_clone_lock()") moved
the LOOPBACK4_IPV6 initialization for passive IPv6 TCP sockets from
tcp_v6_syn_recv_sock() to tcp_v6_init_req().

IPv6 SYN cookie connections allocate a fresh request_sock without
__GFP_ZERO and call tcp_v6_syn_recv_sock() without tcp_v6_init_req(),
leaving ireq->ir_rmt_addr and ireq->ir_loc_addr uninitialized in
cookie_v6_check(). inet_csk_clone_lock() then copies uninitialized slab
bytes into sk_daddr, sk_rcv_saddr, and inet_saddr, exposing them to
tracepoints and bpf_sock and causing inet_daddr != LOOPBACK4_IPV6 checks
(such as IP_OPTIONS in do_ip_setsockopt()) to misclassify native IPv6
child sockets as IPv4-mapped.

Initialize ireq->ir_rmt_addr and ireq->ir_loc_addr to LOOPBACK4_IPV6 in
cookie_v6_check() to match tcp_v6_init_req().

Found in CI testing on a KASAN kernel; reproduced without kernel
modifications in a KASAN VM (tcp_syncookies=2) and tested against the
reproducer and packetdrill syncookie selftests.

Fixes: a3a128f611a9 ("inet: consolidate inet_csk_clone_lock()")
Signed-off-by: Thaison Phan <thaisonphan@google.com>
---
 net/ipv6/syncookies.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/ipv6/syncookies.c b/net/ipv6/syncookies.c
index b581cb1ee2e8..b33011b71fff 100644
--- a/net/ipv6/syncookies.c
+++ b/net/ipv6/syncookies.c
@@ -17,6 +17,7 @@
 #include <net/ipv6.h>
 #include <net/tcp.h>
 #include <net/tcp_ecn.h>
+#include <net/transp_v6.h>
 
 #define COOKIEBITS 24	/* Upper bits store count */
 #define COOKIEMASK (((__u32)1 << COOKIEBITS) - 1)
@@ -205,6 +206,8 @@ struct sock *cookie_v6_check(struct sock *sk, struct sk_buff *skb)
 
 	ireq->ir_v6_rmt_addr = ipv6_hdr(skb)->saddr;
 	ireq->ir_v6_loc_addr = ipv6_hdr(skb)->daddr;
+	ireq->ir_rmt_addr = LOOPBACK4_IPV6;
+	ireq->ir_loc_addr = LOOPBACK4_IPV6;
 
 	if (security_inet_conn_request(sk, skb, req)) {
 		SKB_DR_SET(reason, SECURITY_HOOK);
-- 
2.56.0.611.g1030f0b940-goog

Re: [PATCH net 1/2] tcp: initialize ir_rmt_addr and ir_loc_addr in cookie_v6_check()

[Eric Dumazet]

Le sam. 14 nov. 2026 à 17:13, Thaison Phan <thaisonphan@google.com> a écrit :
>
> Commit a3a128f611a9 ("inet: consolidate inet_csk_clone_lock()") moved
> the LOOPBACK4_IPV6 initialization for passive IPv6 TCP sockets from
> tcp_v6_syn_recv_sock() to tcp_v6_init_req().
>
> IPv6 SYN cookie connections allocate a fresh request_sock without
> __GFP_ZERO and call tcp_v6_syn_recv_sock() without tcp_v6_init_req(),
> leaving ireq->ir_rmt_addr and ireq->ir_loc_addr uninitialized in
> cookie_v6_check(). inet_csk_clone_lock() then copies uninitialized slab
> bytes into sk_daddr, sk_rcv_saddr, and inet_saddr, exposing them to
> tracepoints and bpf_sock and causing inet_daddr != LOOPBACK4_IPV6 checks
> (such as IP_OPTIONS in do_ip_setsockopt()) to misclassify native IPv6
> child sockets as IPv4-mapped.
>
> Initialize ireq->ir_rmt_addr and ireq->ir_loc_addr to LOOPBACK4_IPV6 in
> cookie_v6_check() to match tcp_v6_init_req().
>
> Found in CI testing on a KASAN kernel; reproduced without kernel
> modifications in a KASAN VM (tcp_syncookies=2) and tested against the
> reproducer and packetdrill syncookie selftests.
>

Nice catch, but KASAN does not report uninitialized reads.

Was this KMSAN, or did you notice garbage in the inet_sock_set_state
tracepoint output?

Please say which in the changelog, and include the report or the
trace line if you have one.

> Fixes: a3a128f611a9 ("inet: consolidate inet_csk_clone_lock()")
> Signed-off-by: Thaison Phan <thaisonphan@google.com>
> ---
>  net/ipv6/syncookies.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/net/ipv6/syncookies.c b/net/ipv6/syncookies.c
> index b581cb1ee2e8..b33011b71fff 100644
> --- a/net/ipv6/syncookies.c
> +++ b/net/ipv6/syncookies.c
> @@ -17,6 +17,7 @@
>  #include <net/ipv6.h>
>  #include <net/tcp.h>
>  #include <net/tcp_ecn.h>
> +#include <net/transp_v6.h>
>
>  #define COOKIEBITS 24  /* Upper bits store count */
>  #define COOKIEMASK (((__u32)1 << COOKIEBITS) - 1)
> @@ -205,6 +206,8 @@ struct sock *cookie_v6_check(struct sock *sk, struct sk_buff *skb)
>
>         ireq->ir_v6_rmt_addr = ipv6_hdr(skb)->saddr;
>         ireq->ir_v6_loc_addr = ipv6_hdr(skb)->daddr;
> +       ireq->ir_rmt_addr = LOOPBACK4_IPV6;
> +       ireq->ir_loc_addr = LOOPBACK4_IPV6;

With the changelog clarified:

Reviewed-by: Eric Dumazet <edumazet@kernel.org>

pw-bot: cr

[PATCH net 2/2] tcp: accecn: initialize AccECN fields in syncookie path

[Thaison Phan]

cookie_tcp_reqsk_alloc() and bpf_sk_assign_tcp_reqsk() allocate struct
request_sock without __GFP_ZERO, leaving saw_accecn_opt,
accecn_fail_mode, syn_ect_rcv, and syn_ect_snt uninitialized. When
accecn_ok is negotiated in cookie_{v4,v6}_check(),
tcp_ecn_openreq_child() reads all four fields to initialize the child
socket's AccECN state.

In addition, because the temporary request_sock used to send a cookie
SYN-ACK is immediately freed, treq->syn_ect_snt is lost. If the listener
marks ECT (e.g. DCTCP), leaving syn_ect_snt at 0 causes
tcp_accecn_third_ack() to flag TCP_ACCECN_ACE_FAIL_RECV when the peer
reflects ECT, disabling ECT for the rest of the connection.

Initialize the AccECN fields in cookie_tcp_reqsk_init() to match
tcp_openreq_init(), and set syn_ect_snt from the listener's TOS/tclass
in cookie_{v4,v6}_check() to match tcp_{v4,v6}_send_synack().

Found by code inspection while auditing request_sock initialization in
the syncookie path; reproduced without kernel modifications in a KASAN
VM (tcp_syncookies=2, tcp_ecn=3) and tested against the reproducer and
packetdrill AccECN/syncookie selftests.

Fixes: 3cae34274c79 ("tcp: accecn: AccECN negotiation")
Fixes: b40671b5ee58 ("tcp: accecn: AccECN option failure handling")
Signed-off-by: Thaison Phan <thaisonphan@google.com>
---
 net/ipv4/syncookies.c | 5 +++++
 net/ipv6/syncookies.c | 1 +
 2 files changed, 6 insertions(+)

diff --git a/net/ipv4/syncookies.c b/net/ipv4/syncookies.c
index 4e0ff3b82062..5747f9a5a6d1 100644
--- a/net/ipv4/syncookies.c
+++ b/net/ipv4/syncookies.c
@@ -290,6 +290,10 @@ static int cookie_tcp_reqsk_init(struct sock *sk, struct sk_buff *skb,
 	 */
 	treq->txhash = treq->snt_isn;
 	treq->syn_tos = TCP_SKB_CB(skb)->ip_dsfield;
+	treq->saw_accecn_opt = TCP_ACCECN_OPT_NOT_SEEN;
+	treq->accecn_fail_mode = 0;
+	treq->syn_ect_rcv = 0;
+	treq->syn_ect_snt = 0;
 
 #if IS_ENABLED(CONFIG_MPTCP)
 	treq->is_mptcp = sk_is_mptcp(sk);
@@ -502,6 +506,7 @@ struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb)
 		ireq->rcv_wscale = rcv_wscale;
 	ireq->ecn_ok &= cookie_ecn_ok(net, &rt->dst);
 	treq->accecn_ok = ireq->ecn_ok && cookie_accecn_ok(th);
+	treq->syn_ect_snt = READ_ONCE(inet_sk(sk)->tos) & INET_ECN_MASK;
 
 	ret = tcp_get_cookie_sock(sk, skb, req, &rt->dst);
 	/* ip_queue_xmit() depends on our flow being setup
diff --git a/net/ipv6/syncookies.c b/net/ipv6/syncookies.c
index b33011b71fff..7435ebcc366c 100644
--- a/net/ipv6/syncookies.c
+++ b/net/ipv6/syncookies.c
@@ -276,6 +276,7 @@ struct sock *cookie_v6_check(struct sock *sk, struct sk_buff *skb)
 		ireq->rcv_wscale = rcv_wscale;
 	ireq->ecn_ok &= cookie_ecn_ok(net, dst);
 	tcp_rsk(req)->accecn_ok = ireq->ecn_ok && cookie_accecn_ok(th);
+	tcp_rsk(req)->syn_ect_snt = READ_ONCE(np->tclass) & INET_ECN_MASK;
 
 	ret = tcp_get_cookie_sock(sk, skb, req, dst);
 	if (!ret) {
-- 
2.56.0.611.g1030f0b940-goog

Re: [PATCH net 2/2] tcp: accecn: initialize AccECN fields in syncookie path

[Eric Dumazet]

Le sam. 14 nov. 2026 à 17:13, Thaison Phan <thaisonphan@google.com> a écrit :
>
> cookie_tcp_reqsk_alloc() and bpf_sk_assign_tcp_reqsk() allocate struct
> request_sock without __GFP_ZERO, leaving saw_accecn_opt,
> accecn_fail_mode, syn_ect_rcv, and syn_ect_snt uninitialized. When
> accecn_ok is negotiated in cookie_{v4,v6}_check(),
> tcp_ecn_openreq_child() reads all four fields to initialize the child
> socket's AccECN state.
>

Right.

> In addition, because the temporary request_sock used to send a cookie
> SYN-ACK is immediately freed, treq->syn_ect_snt is lost. If the listener
> marks ECT (e.g. DCTCP), leaving syn_ect_snt at 0 causes
> tcp_accecn_third_ack() to flag TCP_ACCECN_ACE_FAIL_RECV when the peer
> reflects ECT, disabling ECT for the rest of the connection.

This is a different bug than the uninitialized fields above, with
a different impact. I would prefer two patches, one per bug, but
since both Fixes: commits landed in the same release I will not
insist.

>
> Initialize the AccECN fields in cookie_tcp_reqsk_init() to match
> tcp_openreq_init(), and set syn_ect_snt from the listener's TOS/tclass
> in cookie_{v4,v6}_check() to match tcp_{v4,v6}_send_synack().
>
> Found by code inspection while auditing request_sock initialization in
> the syncookie path; reproduced without kernel modifications in a KASAN
> VM (tcp_syncookies=2, tcp_ecn=3) and tested against the reproducer and
> packetdrill AccECN/syncookie selftests.

Same remark as for the LOOPBACK4_IPV6 patch: KASAN does not catch
uninitialized bitfields. Please say how the failure was observed
(KMSAN, tcpi_accecn_fail_mode in TCP_INFO on the child, ...).

>
> Fixes: 3cae34274c79 ("tcp: accecn: AccECN negotiation")
> Fixes: b40671b5ee58 ("tcp: accecn: AccECN option failure handling")
> Signed-off-by: Thaison Phan <thaisonphan@google.com>
> ---
>  net/ipv4/syncookies.c | 5 +++++
>  net/ipv6/syncookies.c | 1 +
>  2 files changed, 6 insertions(+)
>
> diff --git a/net/ipv4/syncookies.c b/net/ipv4/syncookies.c
> index 4e0ff3b82062..5747f9a5a6d1 100644
> --- a/net/ipv4/syncookies.c
> +++ b/net/ipv4/syncookies.c
> @@ -290,6 +290,10 @@ static int cookie_tcp_reqsk_init(struct sock *sk, struct sk_buff *skb,
>          */
>         treq->txhash = treq->snt_isn;
>         treq->syn_tos = TCP_SKB_CB(skb)->ip_dsfield;
> +       treq->saw_accecn_opt = TCP_ACCECN_OPT_NOT_SEEN;
> +       treq->accecn_fail_mode = 0;
> +       treq->syn_ect_rcv = 0;
> +       treq->syn_ect_snt = 0;

I think syn_ect_snt is always overwritten in cookie_{v4,v6}_check()
before being read, this line is not needed?

>
>  #if IS_ENABLED(CONFIG_MPTCP)
>         treq->is_mptcp = sk_is_mptcp(sk);
> @@ -502,6 +506,7 @@ struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb)
>                 ireq->rcv_wscale = rcv_wscale;
>         ireq->ecn_ok &= cookie_ecn_ok(net, &rt->dst);
>         treq->accecn_ok = ireq->ecn_ok && cookie_accecn_ok(th);
> +       treq->syn_ect_snt = READ_ONCE(inet_sk(sk)->tos) & INET_ECN_MASK;
>
>         ret = tcp_get_cookie_sock(sk, skb, req, &rt->dst);
>         /* ip_queue_xmit() depends on our flow being setup
> diff --git a/net/ipv6/syncookies.c b/net/ipv6/syncookies.c
> index b33011b71fff..7435ebcc366c 100644
> --- a/net/ipv6/syncookies.c
> +++ b/net/ipv6/syncookies.c
> @@ -276,6 +276,7 @@ struct sock *cookie_v6_check(struct sock *sk, struct sk_buff *skb)
>                 ireq->rcv_wscale = rcv_wscale;
>         ireq->ecn_ok &= cookie_ecn_ok(net, dst);
>         tcp_rsk(req)->accecn_ok = ireq->ecn_ok && cookie_accecn_ok(th);
> +       tcp_rsk(req)->syn_ect_snt = READ_ONCE(np->tclass) & INET_ECN_MASK;
>
>         ret = tcp_get_cookie_sock(sk, skb, req, dst);
>         if (!ret) {
> --
> 2.56.0.611.g1030f0b940-goog
>

Reviewed-by: Eric Dumazet <edumazet@kernel.org>

Thanks.