* Re: [PATCH net] tcp: ipv6: do not read ireq->pktopts from tcp_v6_send_synack()
2026-11-02 7:54 [PATCH net] tcp: ipv6: do not read ireq->pktopts from tcp_v6_send_synack() Eric Dumazet
@ 2026-11-02 8:30 ` Eric Dumazet
2026-11-02 13:00 ` Jiayuan Chen
2026-11-02 13:47 ` Neal Cardwell
2 siblings, 0 replies; 5+ messages in thread
From: Eric Dumazet @ 2026-11-02 8:30 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski, Paolo Abeni, Neal Cardwell
Cc: Kuniyuki Iwashima, Simon Horman, netdev, Anvik Loitongbam,
Yingjie Wang
Le lun. 2 nov. 2026 à 08:54, Eric Dumazet <edumazet@kernel.org> a écrit :
>
> When IPV6_FL_F_REFLECT is set on a listener, or the
> net.ipv6.flowlabel_reflect sysctl has FLOWLABEL_REFLECT_ESTABLISHED,
> tcp_v6_send_synack() reflects the flowlabel of the SYN packet,
> reading it from the skb stored in ireq->pktopts.
>
> Since commit 079096f103fa ("tcp/dccp: install syn_recv requests
> into ehash table"), request sockets are processed without holding
> the listener lock. A cpu can then process a retransmitted SYN for a
> request socket, and send a SYNACK from tcp_check_req(), while another
> cpu receives the ACK completing the 3WHS. tcp_v6_syn_recv_sock()
> consumes ireq->pktopts when it wins the hashdance, and the first cpu
> reads a freed skb:
>
> BUG: KASAN: slab-use-after-free in tcp_v6_send_synack (include/linux/skbuff.h:3152 include/linux/ipv6.h:110 net/ipv6/tcp_ipv6.c:582)
> Read of size 8 at addr ffff88802b08b608 by task poc-ipv6-pktopt/5076
>
> Call Trace:
> <IRQ>
> tcp_v6_send_synack (include/linux/skbuff.h:3152 include/linux/ipv6.h:110 net/ipv6/tcp_ipv6.c:582)
> tcp_rtx_synack (net/ipv4/tcp_output.c:4719)
> tcp_check_req (net/ipv4/tcp_minisocks.c:762)
> tcp_v6_rcv (net/ipv6/tcp_ipv6.c:1826)
> ip6_protocol_deliver_rcu (net/ipv6/ip6_input.c:479)
> ip6_input_finish (net/ipv6/ip6_input.c:534)
> ip6_input (include/linux/netfilter.h:325 include/linux/netfilter.h:319 net/ipv6/ip6_input.c:545)
> ipv6_rcv (include/net/dst.h:481 net/ipv6/ip6_input.c:119 include/linux/netfilter.h:325 include/linux/netfilter.h:319 net/ipv6/ip6_input.c:351)
>
> Freed by task 5074 on cpu 0 at 15.412249s:
> kmem_cache_free (include/linux/kasan.h:235 mm/slub.c:2780 mm/slub.c:6630 mm/slub.c:6757)
> tcp_v6_syn_recv_sock (net/ipv6/tcp_ipv6.c:1528)
> tcp_check_req (net/ipv4/tcp_minisocks.c:934)
> tcp_v6_rcv (net/ipv6/tcp_ipv6.c:1826)
> ip6_protocol_deliver_rcu (net/ipv6/ip6_input.c:479)
> ip6_input_finish (net/ipv6/ip6_input.c:534)
>
> Request socket migration has the same issue: reqsk_migrate_reset()
> transfers ireq->pktopts to the clone, which can be freed while
> another cpu holding a reference on the old request socket is still
> sending a SYNACK.
>
> The SYNACK timer itself is not an issue, because
> inet_csk_reqsk_queue_drop() calls timer_delete_sync().
>
> Save the flowlabel of the SYN in the request socket instead, so that
> tcp_v6_send_synack() no longer needs ireq->pktopts.
>
> As a side effect, the flowlabel is also reflected for SYN packets
> reusing a TIME_WAIT socket, as ireq->pktopts is not set in this case.
>
> Fixes: e994b2f0fb92 ("tcp: do not lock listener to process SYN packets")
> Fixes: 079096f103fa ("tcp/dccp: install syn_recv requests into ehash table")
> Fixes: c905dee62232 ("tcp: Migrate TCP_NEW_SYN_RECV requests at retransmitting SYN+ACKs.")
> Reported-by: Anvik Loitongbam <permissiontogallop@gmail.com>
> Reported-by: Yingjie Wang <x-professor@outlook.com>
> Closes: https://lore.kernel.org/netdev/82c31e4d5043f3dbd11b44177cfba779d335aae3.1793599929.git.x-professor@outlook.com/
> Signed-off-by: Eric Dumazet <edumazet@kernel.org>
> ---
> Initially reported to security@ mailing list by Anvik Loitongbam, with a patch.
> Anvik Loitongbam provided a reproducer.
> I was able to trigger the ASAN warning before the patch, and checked
> the warning can no longer trigger.
>
> Yingjie Wang also sent a public patch later, very similar to Anvik's one.
>
> include/linux/tcp.h | 3 +++
> net/ipv6/tcp_ipv6.c | 5 +++--
> 2 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/tcp.h b/include/linux/tcp.h
> index d18f7bc1e5d0f73c1c7452fbc573fa7cb38b206f..5a4c61fc240ec2d4554795cea7bbd65bbea0ef02 100644
> --- a/include/linux/tcp.h
> +++ b/include/linux/tcp.h
> @@ -179,6 +179,9 @@ struct tcp_request_sock {
> u8 ao_rcv_next;
> bool used_tcp_ao;
> #endif
> +#if IS_ENABLED(CONFIG_IPV6)
> + __be32 syn_flowlabel;
> +#endif
> };
>
> #define tcp_rsk(ptr) container_of_const(ptr, struct tcp_request_sock, req.req)
> diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
> index e006a30360eae1b43a4d53173b46b931ae668825..d406cd3c8ceabfdbb66f3e1b4f9e9a74754e8d0b 100644
> --- a/net/ipv6/tcp_ipv6.c
> +++ b/net/ipv6/tcp_ipv6.c
> @@ -578,8 +578,8 @@ static int tcp_v6_send_synack(const struct sock *sk, struct dst_entry *dst,
> &ireq->ir_v6_rmt_addr);
>
> fl6->daddr = ireq->ir_v6_rmt_addr;
> - if (inet6_test_bit(REPFLOW, sk) && ireq->pktopts)
> - fl6->flowlabel = ip6_flowlabel(ipv6_hdr(ireq->pktopts));
> + if (inet6_test_bit(REPFLOW, sk))
> + fl6->flowlabel = tcp_rsk(req)->syn_flowlabel;
>
> tclass = READ_ONCE(sock_net(sk)->ipv4.sysctl_tcp_reflect_tos) ?
> (tcp_rsk(req)->syn_tos & ~INET_ECN_MASK) |
> @@ -789,6 +789,7 @@ static void tcp_v6_init_req(struct request_sock *req,
> ireq->ir_v6_loc_addr = ipv6_hdr(skb)->daddr;
> ireq->ir_rmt_addr = LOOPBACK4_IPV6;
> ireq->ir_loc_addr = LOOPBACK4_IPV6;
> + tcp_rsk(req)->syn_flowlabel = ip6_flowlabel(ipv6_hdr(skb));
>
> /* So that link locals have meaning */
> if ((!sk_listener->sk_bound_dev_if || l3_slave) &&
> --
> 2.53.0
>
It seems we also can drop REPFLOW condition from tcp_v6_init_req()
This means each pending request on a REPFLOW-only listener would no
longer pin a full SYN skb.
I will send for net-next:
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index d406cd3c8ceabfdbb66f3e1b4f9e9a74754e8d0b..8ad5f13d4a74f025bc47a4336f3c518cca773315
100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -800,7 +800,7 @@ static void tcp_v6_init_req(struct request_sock *req,
(ipv6_opt_accepted(sk_listener, skb, &TCP_SKB_CB(skb)->header.h6) ||
np->rxopt.bits.rxinfo ||
np->rxopt.bits.rxoinfo || np->rxopt.bits.rxhlim ||
- np->rxopt.bits.rxohlim || inet6_test_bit(REPFLOW, sk_listener))) {
+ np->rxopt.bits.rxohlim)) {
refcount_inc(&skb->users);
ireq->pktopts = skb;
}
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH net] tcp: ipv6: do not read ireq->pktopts from tcp_v6_send_synack()
2026-11-02 7:54 [PATCH net] tcp: ipv6: do not read ireq->pktopts from tcp_v6_send_synack() Eric Dumazet
2026-11-02 8:30 ` Eric Dumazet
@ 2026-11-02 13:00 ` Jiayuan Chen
2026-11-02 13:47 ` Neal Cardwell
2 siblings, 0 replies; 5+ messages in thread
From: Jiayuan Chen @ 2026-11-02 13:00 UTC (permalink / raw)
To: Eric Dumazet, David S . Miller, Jakub Kicinski, Paolo Abeni,
Neal Cardwell
Cc: Kuniyuki Iwashima, Simon Horman, netdev, Anvik Loitongbam,
Yingjie Wang
On 11/2/26 3:54 PM, Eric Dumazet wrote:
> When IPV6_FL_F_REFLECT is set on a listener, or the
> net.ipv6.flowlabel_reflect sysctl has FLOWLABEL_REFLECT_ESTABLISHED,
> tcp_v6_send_synack() reflects the flowlabel of the SYN packet,
> reading it from the skb stored in ireq->pktopts.
>
> Since commit 079096f103fa ("tcp/dccp: install syn_recv requests
> into ehash table"), request sockets are processed without holding
> the listener lock. A cpu can then process a retransmitted SYN for a
> request socket, and send a SYNACK from tcp_check_req(), while another
> cpu receives the ACK completing the 3WHS. tcp_v6_syn_recv_sock()
> consumes ireq->pktopts when it wins the hashdance, and the first cpu
> reads a freed skb:
>
> BUG: KASAN: slab-use-after-free in tcp_v6_send_synack (include/linux/skbuff.h:3152 include/linux/ipv6.h:110 net/ipv6/tcp_ipv6.c:582)
> Read of size 8 at addr ffff88802b08b608 by task poc-ipv6-pktopt/5076
>
> Call Trace:
> <IRQ>
> tcp_v6_send_synack (include/linux/skbuff.h:3152 include/linux/ipv6.h:110 net/ipv6/tcp_ipv6.c:582)
> tcp_rtx_synack (net/ipv4/tcp_output.c:4719)
> tcp_check_req (net/ipv4/tcp_minisocks.c:762)
> tcp_v6_rcv (net/ipv6/tcp_ipv6.c:1826)
> ip6_protocol_deliver_rcu (net/ipv6/ip6_input.c:479)
> ip6_input_finish (net/ipv6/ip6_input.c:534)
> ip6_input (include/linux/netfilter.h:325 include/linux/netfilter.h:319 net/ipv6/ip6_input.c:545)
> ipv6_rcv (include/net/dst.h:481 net/ipv6/ip6_input.c:119 include/linux/netfilter.h:325 include/linux/netfilter.h:319 net/ipv6/ip6_input.c:351)
>
> Freed by task 5074 on cpu 0 at 15.412249s:
> kmem_cache_free (include/linux/kasan.h:235 mm/slub.c:2780 mm/slub.c:6630 mm/slub.c:6757)
> tcp_v6_syn_recv_sock (net/ipv6/tcp_ipv6.c:1528)
> tcp_check_req (net/ipv4/tcp_minisocks.c:934)
> tcp_v6_rcv (net/ipv6/tcp_ipv6.c:1826)
> ip6_protocol_deliver_rcu (net/ipv6/ip6_input.c:479)
> ip6_input_finish (net/ipv6/ip6_input.c:534)
>
> Request socket migration has the same issue: reqsk_migrate_reset()
> transfers ireq->pktopts to the clone, which can be freed while
> another cpu holding a reference on the old request socket is still
> sending a SYNACK.
>
> The SYNACK timer itself is not an issue, because
> inet_csk_reqsk_queue_drop() calls timer_delete_sync().
Unlike v4, I don't think this part hold here:
tcp_v6_syn_recv_sock() consumes pktopts before inet_csk_complete_hashdance() calls timer_delete_sync()
> Save the flowlabel of the SYN in the request socket instead, so that
> tcp_v6_send_synack() no longer needs ireq->pktopts.
>
> As a side effect, the flowlabel is also reflected for SYN packets
> reusing a TIME_WAIT socket, as ireq->pktopts is not set in this case.
>
> Fixes: e994b2f0fb92 ("tcp: do not lock listener to process SYN packets")
> Fixes: 079096f103fa ("tcp/dccp: install syn_recv requests into ehash table")
> Fixes: c905dee62232 ("tcp: Migrate TCP_NEW_SYN_RECV requests at retransmitting SYN+ACKs.")
> Reported-by: Anvik Loitongbam <permissiontogallop@gmail.com>
> Reported-by: Yingjie Wang <x-professor@outlook.com>
> Closes: https://lore.kernel.org/netdev/82c31e4d5043f3dbd11b44177cfba779d335aae3.1793599929.git.x-professor@outlook.com/
> Signed-off-by: Eric Dumazet <edumazet@kernel.org>
Reviewed-by: Jiayuan Chen <jiayuan.chen@linux.dev>
> ---
> Initially reported to security@ mailing list by Anvik Loitongbam, with a patch.
> Anvik Loitongbam provided a reproducer.
> I was able to trigger the ASAN warning before the patch, and checked
> the warning can no longer trigger.
>
> Yingjie Wang also sent a public patch later, very similar to Anvik's one.
I was just looking at that patch...
I think that misses the syncookie path:
tcp_v6_do_rcv
// skb is final ack
cookie_v6_check
// inc ref
refcount_inc(&skb->users);
ireq->pktopts = skb;
tcp_get_cookie_sock
tcp_v6_syn_recv_sock <== consume_skb() removed, and ireq->pktopts still point to the ACK
Then the same ACK is delivered to the child
tcp_v6_do_rcv
tcp_v6_cookie_check
tcp_child_process
tcp_data_queue
__kfree_skb(skb) <== frees it regardless of skb->users
req->pktopts is now dangling, and tcp_v6_reqsk_destructor() will
consume_skb() it later at accept() time.
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH net] tcp: ipv6: do not read ireq->pktopts from tcp_v6_send_synack()
2026-11-02 7:54 [PATCH net] tcp: ipv6: do not read ireq->pktopts from tcp_v6_send_synack() Eric Dumazet
2026-11-02 8:30 ` Eric Dumazet
2026-11-02 13:00 ` Jiayuan Chen
@ 2026-11-02 13:47 ` Neal Cardwell
2026-11-02 15:04 ` Neal Cardwell
2 siblings, 1 reply; 5+ messages in thread
From: Neal Cardwell @ 2026-11-02 13:47 UTC (permalink / raw)
To: Eric Dumazet
Cc: David S . Miller, Jakub Kicinski, Paolo Abeni, Kuniyuki Iwashima,
Simon Horman, netdev, Anvik Loitongbam, Yingjie Wang
On Mon, Nov 2, 2026 at 2:54 AM Eric Dumazet <edumazet@kernel.org> wrote:
>
> When IPV6_FL_F_REFLECT is set on a listener, or the
> net.ipv6.flowlabel_reflect sysctl has FLOWLABEL_REFLECT_ESTABLISHED,
> tcp_v6_send_synack() reflects the flowlabel of the SYN packet,
> reading it from the skb stored in ireq->pktopts.
>
> Since commit 079096f103fa ("tcp/dccp: install syn_recv requests
> into ehash table"), request sockets are processed without holding
> the listener lock. A cpu can then process a retransmitted SYN for a
> request socket, and send a SYNACK from tcp_check_req(), while another
> cpu receives the ACK completing the 3WHS. tcp_v6_syn_recv_sock()
> consumes ireq->pktopts when it wins the hashdance, and the first cpu
> reads a freed skb:
>
> BUG: KASAN: slab-use-after-free in tcp_v6_send_synack (include/linux/skbuff.h:3152 include/linux/ipv6.h:110 net/ipv6/tcp_ipv6.c:582)
> Read of size 8 at addr ffff88802b08b608 by task poc-ipv6-pktopt/5076
>
> Call Trace:
> <IRQ>
> tcp_v6_send_synack (include/linux/skbuff.h:3152 include/linux/ipv6.h:110 net/ipv6/tcp_ipv6.c:582)
> tcp_rtx_synack (net/ipv4/tcp_output.c:4719)
> tcp_check_req (net/ipv4/tcp_minisocks.c:762)
> tcp_v6_rcv (net/ipv6/tcp_ipv6.c:1826)
> ip6_protocol_deliver_rcu (net/ipv6/ip6_input.c:479)
> ip6_input_finish (net/ipv6/ip6_input.c:534)
> ip6_input (include/linux/netfilter.h:325 include/linux/netfilter.h:319 net/ipv6/ip6_input.c:545)
> ipv6_rcv (include/net/dst.h:481 net/ipv6/ip6_input.c:119 include/linux/netfilter.h:325 include/linux/netfilter.h:319 net/ipv6/ip6_input.c:351)
>
> Freed by task 5074 on cpu 0 at 15.412249s:
> kmem_cache_free (include/linux/kasan.h:235 mm/slub.c:2780 mm/slub.c:6630 mm/slub.c:6757)
> tcp_v6_syn_recv_sock (net/ipv6/tcp_ipv6.c:1528)
> tcp_check_req (net/ipv4/tcp_minisocks.c:934)
> tcp_v6_rcv (net/ipv6/tcp_ipv6.c:1826)
> ip6_protocol_deliver_rcu (net/ipv6/ip6_input.c:479)
> ip6_input_finish (net/ipv6/ip6_input.c:534)
>
> Request socket migration has the same issue: reqsk_migrate_reset()
> transfers ireq->pktopts to the clone, which can be freed while
> another cpu holding a reference on the old request socket is still
> sending a SYNACK.
>
> The SYNACK timer itself is not an issue, because
> inet_csk_reqsk_queue_drop() calls timer_delete_sync().
>
> Save the flowlabel of the SYN in the request socket instead, so that
> tcp_v6_send_synack() no longer needs ireq->pktopts.
>
> As a side effect, the flowlabel is also reflected for SYN packets
> reusing a TIME_WAIT socket, as ireq->pktopts is not set in this case.
>
> Fixes: e994b2f0fb92 ("tcp: do not lock listener to process SYN packets")
> Fixes: 079096f103fa ("tcp/dccp: install syn_recv requests into ehash table")
> Fixes: c905dee62232 ("tcp: Migrate TCP_NEW_SYN_RECV requests at retransmitting SYN+ACKs.")
> Reported-by: Anvik Loitongbam <permissiontogallop@gmail.com>
> Reported-by: Yingjie Wang <x-professor@outlook.com>
> Closes: https://lore.kernel.org/netdev/82c31e4d5043f3dbd11b44177cfba779d335aae3.1793599929.git.x-professor@outlook.com/
> Signed-off-by: Eric Dumazet <edumazet@kernel.org>
> ---
> Initially reported to security@ mailing list by Anvik Loitongbam, with a patch.
> Anvik Loitongbam provided a reproducer.
> I was able to trigger the ASAN warning before the patch, and checked
> the warning can no longer trigger.
>
> Yingjie Wang also sent a public patch later, very similar to Anvik's one.
>
> include/linux/tcp.h | 3 +++
> net/ipv6/tcp_ipv6.c | 5 +++--
> 2 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/tcp.h b/include/linux/tcp.h
> index d18f7bc1e5d0f73c1c7452fbc573fa7cb38b206f..5a4c61fc240ec2d4554795cea7bbd65bbea0ef02 100644
> --- a/include/linux/tcp.h
> +++ b/include/linux/tcp.h
> @@ -179,6 +179,9 @@ struct tcp_request_sock {
> u8 ao_rcv_next;
> bool used_tcp_ao;
> #endif
> +#if IS_ENABLED(CONFIG_IPV6)
> + __be32 syn_flowlabel;
> +#endif
> };
>
> #define tcp_rsk(ptr) container_of_const(ptr, struct tcp_request_sock, req.req)
> diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
> index e006a30360eae1b43a4d53173b46b931ae668825..d406cd3c8ceabfdbb66f3e1b4f9e9a74754e8d0b 100644
> --- a/net/ipv6/tcp_ipv6.c
> +++ b/net/ipv6/tcp_ipv6.c
> @@ -578,8 +578,8 @@ static int tcp_v6_send_synack(const struct sock *sk, struct dst_entry *dst,
> &ireq->ir_v6_rmt_addr);
>
> fl6->daddr = ireq->ir_v6_rmt_addr;
> - if (inet6_test_bit(REPFLOW, sk) && ireq->pktopts)
> - fl6->flowlabel = ip6_flowlabel(ipv6_hdr(ireq->pktopts));
> + if (inet6_test_bit(REPFLOW, sk))
> + fl6->flowlabel = tcp_rsk(req)->syn_flowlabel;
>
> tclass = READ_ONCE(sock_net(sk)->ipv4.sysctl_tcp_reflect_tos) ?
> (tcp_rsk(req)->syn_tos & ~INET_ECN_MASK) |
> @@ -789,6 +789,7 @@ static void tcp_v6_init_req(struct request_sock *req,
> ireq->ir_v6_loc_addr = ipv6_hdr(skb)->daddr;
> ireq->ir_rmt_addr = LOOPBACK4_IPV6;
> ireq->ir_loc_addr = LOOPBACK4_IPV6;
> + tcp_rsk(req)->syn_flowlabel = ip6_flowlabel(ipv6_hdr(skb));
>
> /* So that link locals have meaning */
> if ((!sk_listener->sk_bound_dev_if || l3_slave) &&
> --
Reviewed-by: Neal Cardwell <ncardwell@google.com>
Thanks, Eric! The fix looks correct to me, and I ran it through two
frontier LLMs, and they agreed that the fix was correct. They also
suggested your follow-up change of:
- np->rxopt.bits.rxohlim || inet6_test_bit(REPFLOW, sk_listener))) {
+ np->rxopt.bits.rxohlim)) {
So that follow-up looks good to me and the models.
The models suggested two tweaks to the changelog. AFAICT their
suggested changelog tweaks are correct, so I'm including them below
for discussion and completeness, in case it is worth respinning the
patches to update the changelog:
(a) The changelog says: "The SYNACK timer itself is not an issue,
because inet_csk_reqsk_queue_drop() calls timer_delete_sync()."
This looks carried over from e3f33b0a1d89 ("ipv4: free inet_opt and
ireq_opt after an RCU grace period"). It is true there, because
ireq_opt is handed to the child, and the child cannot be freed
before timer_delete_sync().
It is not true for pktopts:
- tcp_v6_syn_recv_sock() frees pktopts immediately with
consume_skb() (tcp_ipv6.c:1528).
- tcp_check_req() calls ->syn_recv_sock() at tcp_minisocks.c:934.
- Only afterward does it reach inet_csk_complete_hashdance()
(tcp_minisocks.c:952), then inet_csk_reqsk_queue_drop(), then
timer_delete_sync() (inet_connection_sock.c:1016).
- The MPTCP rsk_drop_req() path has the same ordering.
So the timer path (reqsk_timer_handler() -> tcp_rtx_synack() ->
tcp_v6_send_synack()) running on another CPU races with that
consume_skb() just like the dup-SYN path. The patch fixes this too,
since pktopts is no longer read, but the sentence should
ideally be corrected. Suggested wording:
"The SYNACK timer has the same issue: tcp_v6_syn_recv_sock()
frees pktopts before inet_csk_complete_hashdance() reaches
timer_delete_sync()."
(b) Missing side effect: after this commit, TFO SYN-ACKs will now
correctly carry the reflected flow label.
- With TFO, the child is created and pktopts consumed
(tcp_try_fastopen() -> tcp_fastopen_create_child() ->
tcp_v6_syn_recv_sock()) *before* tcp_conn_request() sends the TFO
SYN-ACK.
- So today, TFO SYN-ACKs, including retransmits from
tcp_fastopen_synack_timer(), never reflect the label.
The new behavior is the correct one. But it changes what
goes on the wire, so it would be good to add
a sentence next to the TIME_WAIT note.
Thanks!
neal
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH net] tcp: ipv6: do not read ireq->pktopts from tcp_v6_send_synack()
2026-11-02 13:47 ` Neal Cardwell
@ 2026-11-02 15:04 ` Neal Cardwell
0 siblings, 0 replies; 5+ messages in thread
From: Neal Cardwell @ 2026-11-02 15:04 UTC (permalink / raw)
To: Eric Dumazet
Cc: David S . Miller, Jakub Kicinski, Paolo Abeni, Kuniyuki Iwashima,
Simon Horman, netdev, Anvik Loitongbam, Yingjie Wang
[-- Attachment #1: Type: text/plain, Size: 8889 bytes --]
On Mon, Nov 2, 2026 at 8:47 AM Neal Cardwell <ncardwell@google.com> wrote:
>
> On Mon, Nov 2, 2026 at 2:54 AM Eric Dumazet <edumazet@kernel.org> wrote:
> >
> > When IPV6_FL_F_REFLECT is set on a listener, or the
> > net.ipv6.flowlabel_reflect sysctl has FLOWLABEL_REFLECT_ESTABLISHED,
> > tcp_v6_send_synack() reflects the flowlabel of the SYN packet,
> > reading it from the skb stored in ireq->pktopts.
> >
> > Since commit 079096f103fa ("tcp/dccp: install syn_recv requests
> > into ehash table"), request sockets are processed without holding
> > the listener lock. A cpu can then process a retransmitted SYN for a
> > request socket, and send a SYNACK from tcp_check_req(), while another
> > cpu receives the ACK completing the 3WHS. tcp_v6_syn_recv_sock()
> > consumes ireq->pktopts when it wins the hashdance, and the first cpu
> > reads a freed skb:
> >
> > BUG: KASAN: slab-use-after-free in tcp_v6_send_synack (include/linux/skbuff.h:3152 include/linux/ipv6.h:110 net/ipv6/tcp_ipv6.c:582)
> > Read of size 8 at addr ffff88802b08b608 by task poc-ipv6-pktopt/5076
> >
> > Call Trace:
> > <IRQ>
> > tcp_v6_send_synack (include/linux/skbuff.h:3152 include/linux/ipv6.h:110 net/ipv6/tcp_ipv6.c:582)
> > tcp_rtx_synack (net/ipv4/tcp_output.c:4719)
> > tcp_check_req (net/ipv4/tcp_minisocks.c:762)
> > tcp_v6_rcv (net/ipv6/tcp_ipv6.c:1826)
> > ip6_protocol_deliver_rcu (net/ipv6/ip6_input.c:479)
> > ip6_input_finish (net/ipv6/ip6_input.c:534)
> > ip6_input (include/linux/netfilter.h:325 include/linux/netfilter.h:319 net/ipv6/ip6_input.c:545)
> > ipv6_rcv (include/net/dst.h:481 net/ipv6/ip6_input.c:119 include/linux/netfilter.h:325 include/linux/netfilter.h:319 net/ipv6/ip6_input.c:351)
> >
> > Freed by task 5074 on cpu 0 at 15.412249s:
> > kmem_cache_free (include/linux/kasan.h:235 mm/slub.c:2780 mm/slub.c:6630 mm/slub.c:6757)
> > tcp_v6_syn_recv_sock (net/ipv6/tcp_ipv6.c:1528)
> > tcp_check_req (net/ipv4/tcp_minisocks.c:934)
> > tcp_v6_rcv (net/ipv6/tcp_ipv6.c:1826)
> > ip6_protocol_deliver_rcu (net/ipv6/ip6_input.c:479)
> > ip6_input_finish (net/ipv6/ip6_input.c:534)
> >
> > Request socket migration has the same issue: reqsk_migrate_reset()
> > transfers ireq->pktopts to the clone, which can be freed while
> > another cpu holding a reference on the old request socket is still
> > sending a SYNACK.
> >
> > The SYNACK timer itself is not an issue, because
> > inet_csk_reqsk_queue_drop() calls timer_delete_sync().
> >
> > Save the flowlabel of the SYN in the request socket instead, so that
> > tcp_v6_send_synack() no longer needs ireq->pktopts.
> >
> > As a side effect, the flowlabel is also reflected for SYN packets
> > reusing a TIME_WAIT socket, as ireq->pktopts is not set in this case.
> >
> > Fixes: e994b2f0fb92 ("tcp: do not lock listener to process SYN packets")
> > Fixes: 079096f103fa ("tcp/dccp: install syn_recv requests into ehash table")
> > Fixes: c905dee62232 ("tcp: Migrate TCP_NEW_SYN_RECV requests at retransmitting SYN+ACKs.")
> > Reported-by: Anvik Loitongbam <permissiontogallop@gmail.com>
> > Reported-by: Yingjie Wang <x-professor@outlook.com>
> > Closes: https://lore.kernel.org/netdev/82c31e4d5043f3dbd11b44177cfba779d335aae3.1793599929.git.x-professor@outlook.com/
> > Signed-off-by: Eric Dumazet <edumazet@kernel.org>
> > ---
> > Initially reported to security@ mailing list by Anvik Loitongbam, with a patch.
> > Anvik Loitongbam provided a reproducer.
> > I was able to trigger the ASAN warning before the patch, and checked
> > the warning can no longer trigger.
> >
> > Yingjie Wang also sent a public patch later, very similar to Anvik's one.
> >
> > include/linux/tcp.h | 3 +++
> > net/ipv6/tcp_ipv6.c | 5 +++--
> > 2 files changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/linux/tcp.h b/include/linux/tcp.h
> > index d18f7bc1e5d0f73c1c7452fbc573fa7cb38b206f..5a4c61fc240ec2d4554795cea7bbd65bbea0ef02 100644
> > --- a/include/linux/tcp.h
> > +++ b/include/linux/tcp.h
> > @@ -179,6 +179,9 @@ struct tcp_request_sock {
> > u8 ao_rcv_next;
> > bool used_tcp_ao;
> > #endif
> > +#if IS_ENABLED(CONFIG_IPV6)
> > + __be32 syn_flowlabel;
> > +#endif
> > };
> >
> > #define tcp_rsk(ptr) container_of_const(ptr, struct tcp_request_sock, req.req)
> > diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
> > index e006a30360eae1b43a4d53173b46b931ae668825..d406cd3c8ceabfdbb66f3e1b4f9e9a74754e8d0b 100644
> > --- a/net/ipv6/tcp_ipv6.c
> > +++ b/net/ipv6/tcp_ipv6.c
> > @@ -578,8 +578,8 @@ static int tcp_v6_send_synack(const struct sock *sk, struct dst_entry *dst,
> > &ireq->ir_v6_rmt_addr);
> >
> > fl6->daddr = ireq->ir_v6_rmt_addr;
> > - if (inet6_test_bit(REPFLOW, sk) && ireq->pktopts)
> > - fl6->flowlabel = ip6_flowlabel(ipv6_hdr(ireq->pktopts));
> > + if (inet6_test_bit(REPFLOW, sk))
> > + fl6->flowlabel = tcp_rsk(req)->syn_flowlabel;
> >
> > tclass = READ_ONCE(sock_net(sk)->ipv4.sysctl_tcp_reflect_tos) ?
> > (tcp_rsk(req)->syn_tos & ~INET_ECN_MASK) |
> > @@ -789,6 +789,7 @@ static void tcp_v6_init_req(struct request_sock *req,
> > ireq->ir_v6_loc_addr = ipv6_hdr(skb)->daddr;
> > ireq->ir_rmt_addr = LOOPBACK4_IPV6;
> > ireq->ir_loc_addr = LOOPBACK4_IPV6;
> > + tcp_rsk(req)->syn_flowlabel = ip6_flowlabel(ipv6_hdr(skb));
> >
> > /* So that link locals have meaning */
> > if ((!sk_listener->sk_bound_dev_if || l3_slave) &&
> > --
>
> Reviewed-by: Neal Cardwell <ncardwell@google.com>
>
> Thanks, Eric! The fix looks correct to me, and I ran it through two
> frontier LLMs, and they agreed that the fix was correct. They also
> suggested your follow-up change of:
>
> - np->rxopt.bits.rxohlim || inet6_test_bit(REPFLOW, sk_listener))) {
> + np->rxopt.bits.rxohlim)) {
>
> So that follow-up looks good to me and the models.
>
> The models suggested two tweaks to the changelog. AFAICT their
> suggested changelog tweaks are correct, so I'm including them below
> for discussion and completeness, in case it is worth respinning the
> patches to update the changelog:
>
> (a) The changelog says: "The SYNACK timer itself is not an issue,
> because inet_csk_reqsk_queue_drop() calls timer_delete_sync()."
>
> This looks carried over from e3f33b0a1d89 ("ipv4: free inet_opt and
> ireq_opt after an RCU grace period"). It is true there, because
> ireq_opt is handed to the child, and the child cannot be freed
> before timer_delete_sync().
>
> It is not true for pktopts:
> - tcp_v6_syn_recv_sock() frees pktopts immediately with
> consume_skb() (tcp_ipv6.c:1528).
> - tcp_check_req() calls ->syn_recv_sock() at tcp_minisocks.c:934.
> - Only afterward does it reach inet_csk_complete_hashdance()
> (tcp_minisocks.c:952), then inet_csk_reqsk_queue_drop(), then
> timer_delete_sync() (inet_connection_sock.c:1016).
> - The MPTCP rsk_drop_req() path has the same ordering.
>
> So the timer path (reqsk_timer_handler() -> tcp_rtx_synack() ->
> tcp_v6_send_synack()) running on another CPU races with that
> consume_skb() just like the dup-SYN path. The patch fixes this too,
> since pktopts is no longer read, but the sentence should
> ideally be corrected. Suggested wording:
>
> "The SYNACK timer has the same issue: tcp_v6_syn_recv_sock()
> frees pktopts before inet_csk_complete_hashdance() reaches
> timer_delete_sync()."
>
> (b) Missing side effect: after this commit, TFO SYN-ACKs will now
> correctly carry the reflected flow label.
> - With TFO, the child is created and pktopts consumed
> (tcp_try_fastopen() -> tcp_fastopen_create_child() ->
> tcp_v6_syn_recv_sock()) *before* tcp_conn_request() sends the TFO
> SYN-ACK.
> - So today, TFO SYN-ACKs, including retransmits from
> tcp_fastopen_synack_timer(), never reflect the label.
> The new behavior is the correct one. But it changes what
> goes on the wire, so it would be good to add
> a sentence next to the TIME_WAIT note.
I had an LLM write packetdrill tests to verify that TFO SYNACK flow
label reflection is broken before this patch and fixed after this
patch. I've attached the packetdrill scripts and packetdrill
enhancements to support them, in case it's useful. I can merge the
packetdrill enhancement patches and send the packetdrill self tests to
the list if there's consensus that this is useful.
neal
[-- Attachment #2: linux-0001-selftests-net-packetdrill-add-TFO-IPv6-flow-label-reflection-tests.patch --]
[-- Type: application/x-patch, Size: 5985 bytes --]
[-- Attachment #3: packetdrill-0001-support-exact-IPv6-flow-label-checks.patch --]
[-- Type: application/x-patch, Size: 8637 bytes --]
[-- Attachment #4: packetdrill-0002-support-IPV6_FLOWLABEL_MGR-setsockopt.patch --]
[-- Type: application/x-patch, Size: 10893 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread