Netdev List
 help / color / mirror / Atom feed
* [PATCH net v2] tcp: do not coalesce segments after FIN
@ 2026-11-06 12:25 Hyunwoo Kim
  2026-11-06 13:28 ` Eric Dumazet
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Hyunwoo Kim @ 2026-11-06 12:25 UTC (permalink / raw)
  To: Eric Dumazet, Neal Cardwell, Kuniyuki Iwashima, David S. Miller,
	Jakub Kicinski, Paolo Abeni, Simon Horman, john.fastabend, sd
  Cc: netdev, imv4bel

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>
---
Changes in v2:
- Mention TCP_ZEROCOPY_RECEIVE as another affected path in the changelog.
  (Eric)
- Move the FIN check from tcp_ooo_try_coalesce() to tcp_try_coalesce(),
  after the existing checks and before skb_try_coalesce(), using unlikely().
  (Eric)
- Move the tcp_add_backlog() FIN check after the other tcp_flags checks
  and add a short comment. (Eric)
v1: https://lore.kernel.org/all/auxx1bK0QJfUvcB7@v4bel/
---
 net/ipv4/tcp_input.c | 4 ++++
 net/ipv4/tcp_ipv4.c  | 2 ++
 2 files changed, 6 insertions(+)

diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index e1f2d9765f810..1e25db7e4514d 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -5290,6 +5290,10 @@ static bool tcp_try_coalesce(struct sock *sk,
 	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;
 
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index a7f9242f1e429..661c9f94f9c07 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -1957,6 +1957,8 @@ enum skb_drop_reason tcp_add_backlog(struct sock *sk, struct sk_buff *skb)
 	    ((TCP_SKB_CB(tail)->tcp_flags ^
 	      TCP_SKB_CB(skb)->tcp_flags) &
 	     (TCPHDR_ECE | TCPHDR_CWR | TCPHDR_AE)) ||
+	    /* Nothing follows a FIN, see tcp_try_coalesce() */
+	    (TCP_SKB_CB(tail)->tcp_flags & TCPHDR_FIN) ||
 	    !tcp_skb_can_collapse_rx(tail, skb) ||
 	    thtail->doff != th->doff ||
 	    memcmp(thtail + 1, th + 1, hdrlen - sizeof(*th)) ||
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH net v2] tcp: do not coalesce segments after FIN
  2026-11-06 12:25 [PATCH net v2] tcp: do not coalesce segments after FIN Hyunwoo Kim
@ 2026-11-06 13:28 ` Eric Dumazet
  2026-11-07 19:38 ` Kuniyuki Iwashima
  2026-11-10 23:20 ` patchwork-bot+netdevbpf
  2 siblings, 0 replies; 4+ messages in thread
From: Eric Dumazet @ 2026-11-06 13:28 UTC (permalink / raw)
  To: Hyunwoo Kim
  Cc: Neal Cardwell, Kuniyuki Iwashima, David S. Miller, Jakub Kicinski,
	Paolo Abeni, Simon Horman, john.fastabend, sd, netdev

Le ven. 6 nov. 2026 à 13:25, 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.
>
> 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.

...

>
> 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: Eric Dumazet <edumazet@kernel.org>

Thanks.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH net v2] tcp: do not coalesce segments after FIN
  2026-11-06 12:25 [PATCH net v2] tcp: do not coalesce segments after FIN Hyunwoo Kim
  2026-11-06 13:28 ` Eric Dumazet
@ 2026-11-07 19:38 ` Kuniyuki Iwashima
  2026-11-10 23:20 ` patchwork-bot+netdevbpf
  2 siblings, 0 replies; 4+ messages in thread
From: Kuniyuki Iwashima @ 2026-11-07 19:38 UTC (permalink / raw)
  To: Hyunwoo Kim
  Cc: Eric Dumazet, Neal Cardwell, David S. Miller, Jakub Kicinski,
	Paolo Abeni, Simon Horman, john.fastabend, sd, netdev

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>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH net v2] tcp: do not coalesce segments after FIN
  2026-11-06 12:25 [PATCH net v2] tcp: do not coalesce segments after FIN Hyunwoo Kim
  2026-11-06 13:28 ` Eric Dumazet
  2026-11-07 19:38 ` Kuniyuki Iwashima
@ 2026-11-10 23:20 ` patchwork-bot+netdevbpf
  2 siblings, 0 replies; 4+ messages in thread
From: patchwork-bot+netdevbpf @ 2026-11-10 23:20 UTC (permalink / raw)
  To: Hyunwoo Kim
  Cc: edumazet, ncardwell, kuniyu, davem, kuba, pabeni, horms,
	john.fastabend, sd, netdev

Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Fri, 6 Nov 2026 21:25:21 +0900 you 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.
> 
> [...]

Here is the summary with links:
  - [net,v2] tcp: do not coalesce segments after FIN
    https://git.kernel.org/netdev/net/c/1c46780dec48

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-11-10 23:20 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-06 12:25 [PATCH net v2] tcp: do not coalesce segments after FIN Hyunwoo Kim
2026-11-06 13:28 ` Eric Dumazet
2026-11-07 19:38 ` Kuniyuki Iwashima
2026-11-10 23:20 ` patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox