Re: [PATCH] tcp: drain out_of_order_queue and update SACKs in tcp_send_rcvq()

[Eric Dumazet]

On Sat, Sep 19, 2026 at 11:52 PM Hui Peng <benquike@xxxxxxxxx> wrote:
>
> When TCP_REPAIR injects data into the receive queue via tcp_send_rcvq() and
> advances tp->copied_seq and tp->rcv_nxt, any segments already sitting in

tcp_send_rcvq() does not advance tp->copied_seq. The only sequence
update comes from tcp_queue_rcv() -> tcp_rcv_nxt_update(), which touches
rcv_nxt. copied_seq of the receive queue is only written from
TCP_QUEUE_SEQ, which requires TCP_CLOSE state.


> tp->out_of_order_queue that are now covered or contiguous with rcv_nxt are
> not coalesced or removed from tp->rx_opt.num_sacks. Drain


num_sacks is a counter, nothing is "removed from" it.

> out_of_order_queue via tcp_ofo_queue(sk) and update SACK blocks via
> tcp_sack_remove(tp) after advancing rcv_nxt in tcp_send_rcvq().
>
> Fixes: 292e8d8c8538 ("tcp: Move rcvq sending to tcp_input.c")

This commit is a pure code move, it changed no behavior.

> Assisted-by: LLM
> Signed-off-by: Hui Peng <benquike@xxxxxxxxx>

What is the bug exactly ? What is the user visible symptom ?

No repro, no syzbot report, no packetdrill test.

For the CRIU flow the out-of-order queue is empty, because TCP_QUEUE_SEQ
only lets you move rcv_nxt while the socket is in TCP_CLOSE state.

You need CAP_NET_ADMIN, and you have to enable TCP_REPAIR on a live
ESTABLISHED socket that already has segments queued out of order, to
even get there. And the state you describe is transient: the next
in-order segment hits tcp_data_queue(), which already calls
tcp_ofo_queue() and tcp_sack_remove(). tcp_fin(), tcp_prune_ofo_queue(),
tcp_disconnect() and tcp_close() all purge the rb-tree as well.

So this is a "Fixes:" tag on a 2012 commit, sending a behavior change of
a privileged path to all stable trees, for something that is not
demonstrated to be a bug.


> ---
> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> index 92bc60716f33..8a7c38a4b1ec 100644
> --- a/net/ipv4/tcp_input.c
> +++ b/net/ipv4/tcp_input.c
> @@ -5589,6 +5589,11 @@ int tcp_send_rcvq(struct sock *sk, struct msghdr *msg, size_t size)
>                 WARN_ON_ONCE(fragstolen); /* should not happen */
>                 __kfree_skb(skb);
>         }
> +       if (!RB_EMPTY_ROOT(&tcp_sk(sk)->out_of_order_queue)) {
> +               tcp_ofo_queue(sk);

And this is actively dangerous: you are running the input path from
sendmsg().
If one of the out-of-order skbs carries a FIN and the injected bytes
close the hole, tcp_ofo_queue() calls tcp_fin(), and we get
ESTABLISHED -> CLOSE_WAIT, RCV_SHUTDOWN, SOCK_DONE,
skb_rbtree_purge(&tp->out_of_order_queue), tcp_sack_reset(),
inet_csk_schedule_ack() and the sk_state_change()/sk_wake_async()
wakeups, from a sendmsg() that still returns size to the caller.
A repair tool would see a successful write and a shut down read side.
Also, tcp_ofo_queue() computes dsack_high from tp->rcv_nxt, which you
just advanced with locally injected bytes. So we would record and later
advertise DSACK for ranges the peer never retransmitted, and feed its
undo logic. tp->rx_opt.dsack is not cleared here either, the existing
caller clears it at the beginning of tcp_data_queue().


> +               if (tcp_sk(sk)->rx_opt.num_sacks)
> +                       tcp_sack_remove(tcp_sk(sk));
> +       }
>         return size;
>
>  err_free:

And you copied two of the five steps tcp_data_queue() performs:
no ICSK_ACK_NOW, no tcp_fast_path_check(), no tcp_data_ready(), so bytes
can be moved to sk_receive_queue with no wakeup.

If you really want to address this, do not call the input path. Peer
data sitting out of order is meaningless once the receive stream is
being rewritten, so purging the rb-tree and resetting the SACK state
would be the sane thing to do. But please come with a repro or a
packetdrill test first, showing an actual problem.

pw-bot: reject