Re: [PATCH net v2] tcp: do not coalesce segments after FIN

[Kuniyuki Iwashima <kuniyu@google.com>]

On Fri, 06 Nov 2026 21:25:21 +0900, Hyunwoo Kim wrote:
> A FIN consumes a sequence number. If another FIN segment is coalesced
> into an skb that already carries FIN, end_seq advances to account for
> the additional FIN, but tcp_flags still contains only one FIN bit.
> 
> When the preceding gap is filled and tcp_ofo_queue() processes the
> coalesced skb, rcv_nxt advances to the skb's end_seq. When the receive
> path consumes all data in the skb, copied_seq advances by the data
> length and one sequence number for FIN. If three or more FIN segments
> have been coalesced into one skb, tcp_inq() can still return a positive
> value after the receive queue becomes empty and the SOCK_DONE adjustment
> is applied.
> 
> TCP_ZEROCOPY_RECEIVE is another consumer affected by this state.
> tcp_zerocopy_receive() takes its length from tcp_inq() and follows
> skb->next without checking for the end of the receive queue.
> 
> Software kTLS receive is one consumer affected by this state. When
> strp->copy_mode is false, tls_strp_msg_consume() calls the TCP helper
> tcp_read_done() to consume the current TLS record. In the reproducer,
> tcp_read_done() consumed the record data and one FIN before freeing the
> receive skb.
> 
> When kTLS looked for the next record, tcp_inq() reported at least
> TLS_HEADER_SIZE bytes even though the receive queue was empty.
> tcp_recv_skb() returned NULL, so tls_strp_load_anchor_with_queue() hit
> WARN_ON_ONCE() and returned without updating the anchor.
> 
> Since tcp_inq() had reported at least TLS_HEADER_SIZE,
> tls_strp_read_sock() proceeded to tls_strp_check_queue_ok().
> skb_shinfo(strp->anchor)->frag_list still pointed to the freed receive
> skb. tls_strp_check_queue_ok() then read TCP_SKB_CB(first)->seq through
> the stale pointer, causing a use-after-free.
> 
> The reproducer followed this call path:
> 
>   tls_rx_rec_done()
>     tls_strp_msg_consume()
>       tcp_read_done()                       // frees the receive skb
>     tls_strp_check_rcv()
>       tls_strp_read_sock()
>         tls_strp_load_anchor_with_queue()   // returns after WARN_ON_ONCE()
>           tcp_recv_skb() == NULL
>         tls_strp_check_queue_ok()
>           TCP_SKB_CB(first)->seq            // reads the freed receive skb
> 
> KASAN reported:
> 
>   BUG: KASAN: slab-use-after-free in tls_strp_check_queue_ok.isra.0+0x5f/0x100
>   Read of size 4 at addr ffff888103169ca8 by task ktls-server/71
>   ...
>   Call Trace:
>    <TASK>
>    ...
>    tls_strp_check_queue_ok.isra.0+0x5f/0x100
>    tls_strp_check_rcv+0x2a9/0x400
>    ...
>    tls_sw_recvmsg+0x3ce/0xa10
>    ...
>    sock_recvmsg+0xea/0x100
>    __sys_recvfrom+0x142/0x1f0
>    ...
>    __x64_sys_recvfrom+0x76/0x90
>    do_syscall_64+0xdd/0x4a0
>    entry_SYSCALL_64_after_hwframe+0x77/0x7f
>    ...
>    </TASK>
>   ...
>   Freed by task 71:
>    ...
>    napi_skb_cache_put+0x24/0xf0
>    skb_attempt_defer_free+0x183/0x3b0
>    tcp_read_done+0x2d4/0x340
>    tls_strp_msg_consume+0x45/0x80
>    tls_sw_recvmsg+0x3c4/0xa10
>    ...
> 
> In tcp_try_coalesce(), skip coalescing when the destination skb already
> carries FIN. tcp_add_backlog() calls skb_try_coalesce() directly instead
> of using this helper, so add the same check there. Since both paths check
> only the destination skb, a final segment carrying data and FIN can still
> be coalesced into an skb without FIN when the other coalescing checks pass.
> 
> Fixes: e93a0435f809 ("tcp: allow segment with FIN in tcp_try_coalesce()")
> Fixes: 4f693b55c3d2 ("tcp: implement coalescing on backlog queue")
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Hyunwoo Kim <imv4bel@gmail.com>

Reviewed-by: Kuniyuki Iwashima <kuniyu@google.com>