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

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

[Hyunwoo Kim]

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.

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_ooo_try_coalesce() and tcp_add_backlog(), skip coalescing when
the destination skb already carries FIN. This still allows a FIN
segment to 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>
---
 net/ipv4/tcp_input.c | 3 ++-
 net/ipv4/tcp_ipv4.c  | 1 +
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index e1f2d9765f810..1f9f86028da73 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -5314,7 +5314,8 @@ static bool tcp_ooo_try_coalesce(struct sock *sk,
 			     struct sk_buff *from,
 			     bool *fragstolen)
 {
-	bool res = tcp_try_coalesce(sk, to, from, fragstolen);
+	bool res = !(TCP_SKB_CB(to)->tcp_flags & TCPHDR_FIN) &&
+		   tcp_try_coalesce(sk, to, from, fragstolen);
 
 	/* In case tcp_drop_reason() is called later, update to->gso_segs */
 	if (res) {
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index a7f9242f1e429..ef01768292fbc 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -1949,6 +1949,7 @@ enum skb_drop_reason tcp_add_backlog(struct sock *sk, struct sk_buff *skb)
 	thtail = (struct tcphdr *)tail->data;
 
 	if (TCP_SKB_CB(tail)->end_seq != TCP_SKB_CB(skb)->seq ||
+	    (TCP_SKB_CB(tail)->tcp_flags & TCPHDR_FIN) ||
 	    TCP_SKB_CB(tail)->ip_dsfield != TCP_SKB_CB(skb)->ip_dsfield ||
 	    ((TCP_SKB_CB(tail)->tcp_flags |
 	      TCP_SKB_CB(skb)->tcp_flags) & (TCPHDR_SYN | TCPHDR_RST | TCPHDR_URG)) ||
-- 
2.43.0

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

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

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

[Eric Dumazet]

Le jeu. 5 nov. 2026 à 13:09, Hyunwoo Kim <imv4bel@gmail.com> a écrit :
>
> 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.
>

Yeah, we have been working lately on this issue (initial report from
Chris Mason)

For some reason Kuniyuki has not sent his patch yet.

> 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.
>
> 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.
>

TCP_ZEROCOPY_RECEIVE is another one: tcp_zerocopy_receive() takes its
length from tcp_inq() and follows skb->next without checking for the
end of the receive queue. Please mention it in the changelog.


> 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_ooo_try_coalesce() and tcp_add_backlog(), skip coalescing when
> the destination skb already carries FIN. This still allows a FIN
> segment to 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>
> ---
>  net/ipv4/tcp_input.c | 3 ++-
>  net/ipv4/tcp_ipv4.c  | 1 +
>  2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> index e1f2d9765f810..1f9f86028da73 100644
> --- a/net/ipv4/tcp_input.c
> +++ b/net/ipv4/tcp_input.c
> @@ -5314,7 +5314,8 @@ static bool tcp_ooo_try_coalesce(struct sock *sk,
>                              struct sk_buff *from,
>                              bool *fragstolen)
>  {
> -       bool res = tcp_try_coalesce(sk, to, from, fragstolen);
> +       bool res = !(TCP_SKB_CB(to)->tcp_flags & TCPHDR_FIN) &&
> +                  tcp_try_coalesce(sk, to, from, fragstolen);

Please put the check in tcp_try_coalesce() itself.

The receive queue callers can not hit this today (tcp_fin() purges
the ofo queue, and tcp_rcv_state_process() drops anything at or after
rcv_nxt in CLOSE_WAIT/CLOSING/LAST_ACK), but the check costs nothing
there, and it keeps the rule "nothing coalesces after a FIN" in one
place.

Also, since TCP_SKB_CB(to)->tcp_flags & TCPHDR_FIN should almost never
be true, it should come after the more relevant tests, just before
skb_try_coalesce():

        if (TCP_SKB_CB(from)->seq != TCP_SKB_CB(to)->end_seq)
                return false;

        if (!tcp_skb_can_collapse_rx(to, from))
                return false;

        /* Nothing follows a FIN. */
        if (unlikely(TCP_SKB_CB(to)->tcp_flags & TCPHDR_FIN))
                return false;

        if (!skb_try_coalesce(to, from, fragstolen, &delta))
                return false;


>
>         /* In case tcp_drop_reason() is called later, update to->gso_segs */
>         if (res) {
> diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
> index a7f9242f1e429..ef01768292fbc 100644
> --- a/net/ipv4/tcp_ipv4.c
> +++ b/net/ipv4/tcp_ipv4.c
> @@ -1949,6 +1949,7 @@ enum skb_drop_reason tcp_add_backlog(struct sock *sk, struct sk_buff *skb)
>         thtail = (struct tcphdr *)tail->data;
>
>         if (TCP_SKB_CB(tail)->end_seq != TCP_SKB_CB(skb)->seq ||
> +           (TCP_SKB_CB(tail)->tcp_flags & TCPHDR_FIN) ||


Same remark: please move this after the other tcp_flags tests, with a
short comment:

            /* Nothing follows a FIN, see tcp_try_coalesce() */
            (TCP_SKB_CB(tail)->tcp_flags & TCPHDR_FIN) ||

Checking only the tail is the right thing to do. It keeps the common
case where the last data+FIN segment is merged onto a data tail,
which happens on every connection close while the socket is owned by
the user.

Separately, tls_strp_load_anchor_with_queue() hits WARN_ON_ONCE() and
returns without telling its caller, which then uses a stale frag_list.
kTLS should probably not trust tcp_inq() that much, but this can be
handled in a follow-up patch.

Thanks.

>             TCP_SKB_CB(tail)->ip_dsfield != TCP_SKB_CB(skb)->ip_dsfield ||
>             ((TCP_SKB_CB(tail)->tcp_flags |
>               TCP_SKB_CB(skb)->tcp_flags) & (TCPHDR_SYN | TCPHDR_RST | TCPHDR_URG)) ||
> --
> 2.43.0

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

[Hyunwoo Kim]

On Thu, Nov 05, 2026 at 01:22:12PM +0100, Eric Dumazet wrote:
> Le jeu. 5 nov. 2026 à 13:09, Hyunwoo Kim <imv4bel@gmail.com> a écrit :
> >
> > 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.
> >
> 
> Yeah, we have been working lately on this issue (initial report from
> Chris Mason)
> 
> For some reason Kuniyuki has not sent his patch yet.
> 
> > 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.
> >
> > 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.
> >
> 
> TCP_ZEROCOPY_RECEIVE is another one: tcp_zerocopy_receive() takes its
> length from tcp_inq() and follows skb->next without checking for the
> end of the receive queue. Please mention it in the changelog.
> 
> 
> > 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_ooo_try_coalesce() and tcp_add_backlog(), skip coalescing when
> > the destination skb already carries FIN. This still allows a FIN
> > segment to 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>
> > ---
> >  net/ipv4/tcp_input.c | 3 ++-
> >  net/ipv4/tcp_ipv4.c  | 1 +
> >  2 files changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> > index e1f2d9765f810..1f9f86028da73 100644
> > --- a/net/ipv4/tcp_input.c
> > +++ b/net/ipv4/tcp_input.c
> > @@ -5314,7 +5314,8 @@ static bool tcp_ooo_try_coalesce(struct sock *sk,
> >                              struct sk_buff *from,
> >                              bool *fragstolen)
> >  {
> > -       bool res = tcp_try_coalesce(sk, to, from, fragstolen);
> > +       bool res = !(TCP_SKB_CB(to)->tcp_flags & TCPHDR_FIN) &&
> > +                  tcp_try_coalesce(sk, to, from, fragstolen);
> 
> Please put the check in tcp_try_coalesce() itself.
> 
> The receive queue callers can not hit this today (tcp_fin() purges
> the ofo queue, and tcp_rcv_state_process() drops anything at or after
> rcv_nxt in CLOSE_WAIT/CLOSING/LAST_ACK), but the check costs nothing
> there, and it keeps the rule "nothing coalesces after a FIN" in one
> place.
> 
> Also, since TCP_SKB_CB(to)->tcp_flags & TCPHDR_FIN should almost never
> be true, it should come after the more relevant tests, just before
> skb_try_coalesce():
> 
>         if (TCP_SKB_CB(from)->seq != TCP_SKB_CB(to)->end_seq)
>                 return false;
> 
>         if (!tcp_skb_can_collapse_rx(to, from))
>                 return false;
> 
>         /* Nothing follows a FIN. */
>         if (unlikely(TCP_SKB_CB(to)->tcp_flags & TCPHDR_FIN))
>                 return false;
> 
>         if (!skb_try_coalesce(to, from, fragstolen, &delta))
>                 return false;
> 
> 
> >
> >         /* In case tcp_drop_reason() is called later, update to->gso_segs */
> >         if (res) {
> > diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
> > index a7f9242f1e429..ef01768292fbc 100644
> > --- a/net/ipv4/tcp_ipv4.c
> > +++ b/net/ipv4/tcp_ipv4.c
> > @@ -1949,6 +1949,7 @@ enum skb_drop_reason tcp_add_backlog(struct sock *sk, struct sk_buff *skb)
> >         thtail = (struct tcphdr *)tail->data;
> >
> >         if (TCP_SKB_CB(tail)->end_seq != TCP_SKB_CB(skb)->seq ||
> > +           (TCP_SKB_CB(tail)->tcp_flags & TCPHDR_FIN) ||
> 
> 
> Same remark: please move this after the other tcp_flags tests, with a
> short comment:
> 
>             /* Nothing follows a FIN, see tcp_try_coalesce() */
>             (TCP_SKB_CB(tail)->tcp_flags & TCPHDR_FIN) ||
> 
> Checking only the tail is the right thing to do. It keeps the common
> case where the last data+FIN segment is merged onto a data tail,
> which happens on every connection close while the socket is owned by
> the user.

Understood. I'll send a v2 addressing your comments.


Best regards,
Hyunwoo Kim