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

[Eric Dumazet]

Le mar. 27 oct. 2026 à 03:28, Kuniyuki Iwashima <kuniyu@xxxxxxxxxx> a écrit :
>
> On Mon, 26 Oct 2026 13:06:56 +0100, Eric Dumazet wrote:
> > 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)
>
> Exactly, this looks better too, we can avoid negative
> sk_forward_alloc and clamp().
>
> The packetdrill will need to be adjusted as it triggers cgroup
> OOM with this approach, or we could defer the test to net-next too.
>

Let's defer the test. We also have an upcoming fix for tcp_collapse() anyway.

Octavian, could you resend a v5 (sorry for that) ?

Thanks!

pw-bot: cr