Re: [PATCH net v4 1/2] tcp: clamp return value of tcp_wmem_schedule() to non-negative

[Eric Dumazet]

Le lun. 26 oct. 2026 à 04:22, Octavian Purdila <tavip@xxxxxxxxxx> a écrit :
>
> When sk_wmem_schedule(sk, copy) fails under TCP memory pressure and
> sk->sk_wmem_queued already reaches or exceeds sysctl_tcp_wmem[0],
> tcp_wmem_schedule() returns min(copy, sk->sk_forward_alloc).
>
> However, sk->sk_forward_alloc is a signed int that can transiently
> become negative when tcp_collapse() re-packs partially read or high
> payload-to-truesize SKBs in the receive queue and charges newly
> allocated SKBs via sk_mem_charge() before freeing the original SKB.
>
> If tcp_wmem_schedule() returns a negative value, tcp_sendmsg_locked()
> passes that negative copy length to skb_copy_to_page_nocache() ->
> copy_from_iter_full() -> check_copy_size(), where it is converted to an
> unsigned size_t > INT_MAX, triggering a WARN_ON_ONCE(bytes > INT_MAX)
> in production and returning -EFAULT instead of waiting for memory or
> returning -EAGAIN:
>
>   WARNING: CPU: 1 PID: 939331 at include/linux/thread_info.h:249 tcp_sendmsg+0x10c1/0x1530
>   Call Trace:
>    <TASK>
>    __sys_sendmsg+0x292/0x490
>    do_syscall_64+0x88/0x340
>    entry_SYSCALL_64_after_hwframe+0x76/0x7e
>
> Clamp the return value of tcp_wmem_schedule() to [0, copy] so callers
> treat negative sk_forward_alloc as 0 available bytes.
>
> Fixes: 849b425cd091 ("tcp: fix possible freeze in tx path under memory pressure")
> Assisted-by: LLM
> Reviewed-by: Kuniyuki Iwashima <kuniyu@xxxxxxxxxx>
> Reviewed-by: Eric Dumazet <edumazet@xxxxxxxxxx>
> Signed-off-by: Octavian Purdila <tavip@xxxxxxxxxx>
> ---
>  net/ipv4/tcp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index 87ef6d5cbfeba..0e062fcd94a6c 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -1037,7 +1037,7 @@ int tcp_wmem_schedule(struct sock *sk, int copy)
>         left = READ_ONCE(sock_net(sk)->ipv4.sysctl_tcp_wmem[0]) - sk->sk_wmem_queued;
>         if (left > 0)
>                 sk_forced_mem_schedule(sk, min(left, copy));
> -       return min(copy, sk->sk_forward_alloc);
> +       return clamp(sk->sk_forward_alloc, 0, copy);
>  }
>
>  void tcp_free_fastopen_req(struct tcp_sock *tp)

I do wonder if the following fix could be better (no change in the fast path)

Before a complete fix in tcp_collapse() (in net-next)

    tcp: do not let tcp_collapse() drive sk_forward_alloc negative

    tcp_collapse() charges each newly allocated skb with skb_set_owner_r()
    before the skbs it replaces are freed, without scheduling memory first.

    Since sk_mem_uncharge() reclaims eagerly, sk->sk_forward_alloc has no
    cushion and can become negative. This can be durable when the collapsed
    skbs had a high payload/truesize ratio or were partially read, because
    the new skbs can have a bigger total truesize than the old ones.

    A negative sk_forward_alloc then confuses tcp_wmem_schedule(), which
    can return a negative length to tcp_sendmsg_locked(), triggering
    a WARN_ON_ONCE(bytes > INT_MAX) in check_copy_size().

    Call sk_forced_mem_schedule() before charging nskb.

    Fixes: 4890b686f408 ("net: keep sk->sk_forward_alloc as small as possible")

diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 1ddce9c4b7e8cf91bfd4f5361abec3925fe25046..e879bdfddf1abe2ced80b12ea8a40c1f7c958708
100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -5847,6 +5847,10 @@ tcp_collapse(struct sock *sk, struct
sk_buff_head *list, struct rb_root *root,
                        __skb_queue_before(list, skb, nskb);
                else
                        __skb_queue_tail(&tmp, nskb); /* defer rbtree
insertion */
+               /* nskb is charged before the skbs it replaces are freed.
+                * Make sure sk->sk_forward_alloc can not become negative.
+                */
+               sk_forced_mem_schedule(sk, nskb->truesize);
                skb_set_owner_r(nskb, sk);
                mptcp_skb_ext_move(nskb, skb);