Re: [PATCH net] tcp: ipv6: do not read ireq->pktopts from tcp_v6_send_synack()

[Jiayuan Chen <jiayuan.chen@linux.dev>]

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.