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

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

[Octavian Purdila]

This series fixes a bug in tcp_wmem_schedule() where a negative
sk->sk_forward_alloc under TCP memory pressure causes tcp_wmem_schedule()
to return a negative copy length when sk->sk_wmem_queued already reaches
or exceeds sysctl_tcp_wmem[0].

sk->sk_forward_alloc 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 SKBs. When a subsequent send hits memory pressure
in sk_wmem_schedule() with sk->sk_wmem_queued >= sysctl_tcp_wmem[0],
tcp_wmem_schedule() returns min(copy, sk->sk_forward_alloc) < 0.
Callers such as tcp_sendmsg_locked() then pass that negative copy length
to skb_copy_to_page_nocache() -> copy_from_iter_full() ->
check_copy_size(), triggering WARN_ON_ONCE(bytes > INT_MAX) and
returning -EFAULT instead of waiting for memory or returning -EAGAIN.

Patch 1 clamps the return value of tcp_wmem_schedule() to [0, copy] so
a negative sk->sk_forward_alloc is treated as 0 available bytes.
Patch 2 adds a packetdrill selftest that reproduces the negative
sk->sk_forward_alloc condition via tcp_collapse() under memory pressure
and verifies that a non-blocking write fails with EAGAIN rather than
EFAULT.

Octavian Purdila (2):
  tcp: clamp return value of tcp_wmem_schedule() to non-negative
  selftests: net: packetdrill: add test for negative sk_forward_alloc in
    tcp_wmem_schedule()

 net/ipv4/tcp.c                                |   2 +-
 .../testing/selftests/net/packetdrill/config  |   1 +
 .../tcp_wmem_schedule_negative_fwd_alloc.pkt  | 116 ++++++++++++++++++
 3 files changed, 118 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/net/packetdrill/tcp_wmem_schedule_negative_fwd_alloc.pkt

-- 
2.56.0.176.g3575e65682-goog

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

[Octavian Purdila]

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)
and returning -EFAULT instead of waiting for memory or returning
-EAGAIN.

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
Signed-off-by: Octavian Purdila <tavip@google.com>
---
 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)
-- 
2.56.0.176.g3575e65682-goog

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

[Kuniyuki Iwashima]

On Thu, Oct 15, 2026 at 12:37 AM Octavian Purdila <tavip@google.com> wrote:
>
> 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)
> and returning -EFAULT instead of waiting for memory or returning
> -EAGAIN.
>
> 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
> Signed-off-by: Octavian Purdila <tavip@google.com>

Reviewed-by: Kuniyuki Iwashima <kuniyu@google.com>

Basically sk->sk_forward_alloc should not be negative, and
I was able to avoid that by refactoring tcp_collapse(), but it was
a bit invasive for stable and late in rc8, so I'll post it in the next cycle.

$ git diff --stat HEAD~4
 net/ipv4/tcp_input.c | 158 ...
 1 file changed, 109 insertions(+), 49 deletions(-)

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

[Eric Dumazet]

Le ven. 16 oct. 2026 à 09:42, Kuniyuki Iwashima <kuniyu@google.com> a écrit :
>
> On Thu, Oct 15, 2026 at 12:37 AM Octavian Purdila <tavip@google.com> wrote:
> >
> > 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)
> > and returning -EFAULT instead of waiting for memory or returning
> > -EAGAIN.
> >
> > 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
> > Signed-off-by: Octavian Purdila <tavip@google.com>
>
> Reviewed-by: Kuniyuki Iwashima <kuniyu@google.com>
>

Reviewed-by: Eric Dumazet <edumazet@kernel.org>

> Basically sk->sk_forward_alloc should not be negative, and
> I was able to avoid that by refactoring tcp_collapse(), but it was
> a bit invasive for stable and late in rc8, so I'll post it in the next cycle.
>
> $ git diff --stat HEAD~4
>  net/ipv4/tcp_input.c | 158 ...
>  1 file changed, 109 insertions(+), 49 deletions(-)

[PATCH net 2/2] selftests: net: packetdrill: add test for negative sk_forward_alloc in tcp_wmem_schedule()

[Octavian Purdila]

Add a packetdrill test that drives sk->sk_forward_alloc negative via
tcp_collapse() on the receive queue while sk->sk_wmem_queued exceeds
sysctl_tcp_wmem[0], and verifies that a subsequent non-blocking write()
under TCP memory pressure fails with EAGAIN rather than returning a
negative copy length from tcp_wmem_schedule() and failing with EFAULT.

Assisted-by: LLM
Signed-off-by: Octavian Purdila <tavip@google.com>
---
 .../testing/selftests/net/packetdrill/config  |   1 +
 .../tcp_wmem_schedule_negative_fwd_alloc.pkt  | 116 ++++++++++++++++++
 2 files changed, 117 insertions(+)
 create mode 100644 tools/testing/selftests/net/packetdrill/tcp_wmem_schedule_negative_fwd_alloc.pkt

diff --git a/tools/testing/selftests/net/packetdrill/config b/tools/testing/selftests/net/packetdrill/config
index b7df8bf309200..dc3cd72a8da8c 100644
--- a/tools/testing/selftests/net/packetdrill/config
+++ b/tools/testing/selftests/net/packetdrill/config
@@ -1,6 +1,7 @@
 CONFIG_HZ=1000
 CONFIG_HZ_1000=y
 CONFIG_IPV6=y
+CONFIG_MEMCG=y
 CONFIG_NET_NS=y
 CONFIG_NET_SCH_FIFO=y
 CONFIG_NET_SCH_FQ=y
diff --git a/tools/testing/selftests/net/packetdrill/tcp_wmem_schedule_negative_fwd_alloc.pkt b/tools/testing/selftests/net/packetdrill/tcp_wmem_schedule_negative_fwd_alloc.pkt
new file mode 100644
index 0000000000000..db232b54c554e
--- /dev/null
+++ b/tools/testing/selftests/net/packetdrill/tcp_wmem_schedule_negative_fwd_alloc.pkt
@@ -0,0 +1,116 @@
+// SPDX-License-Identifier: GPL-2.0
+// Test that tcp_wmem_schedule() clamps negative sk->sk_forward_alloc to 0
+// when sk_wmem_schedule() fails under memory pressure and
+// sk->sk_wmem_queued >= sysctl_tcp_wmem[0], returning EAGAIN rather than EFAULT.
+
+--mtu=65535
+--tolerance_usecs=100000
+
+`set -e
+./defaults.sh
+ip link set dev tun0 mtu 65535
+sysctl -q net.ipv4.tcp_rmem="4096 131072 6291456"
+sysctl -q net.ipv4.tcp_wmem="4096 16384 4194304"
+cgroot=$(mount -t cgroup2 | head -1 | awk '{print $3}')
+[ -n "$cgroot" ]
+echo +memory > "${cgroot}/cgroup.subtree_control"
+mkdir -p "${cgroot}/pkt_${PPID}"
+echo $PPID > "${cgroot}/pkt_${PPID}/cgroup.procs"
+`
+
+// Create listening socket in pkt_${PPID} memcg and establish connection
+0.000 socket(..., SOCK_STREAM, IPPROTO_TCP) = 3
++0.000 setsockopt(3, SOL_SOCKET, SO_REUSEADDR, [1], 4) = 0
++0.000 bind(3, ..., ...) = 0
++0.000 listen(3, 1) = 0
+
++0.000 < S 0:0(0) win 65535 <mss 36000,nop,wscale 7>
++0.000 > S. 0:0(0) ack 1 <...>
++0.000 < . 1:1(0) ack 1 win 512
++0.000 accept(3, ..., ...) = 4
+
+// Move packetdrill back to its original cgroup so only the sockets remain charged to pkt_${PPID}
++0.000 `cgroot=$(mount -t cgroup2 | head -1 | awk '{print $3}')
+runner_pid=$(awk '/^PPid:/ {print $2}' /proc/$PPID/status)
+orig_cg=$(sed -n 's/^0:://p' /proc/$runner_pid/cgroup)
+echo $PPID > "${cgroot}${orig_cg}/cgroup.procs"`
+
++0.000 setsockopt(4, SOL_SOCKET, SO_SNDBUF, [2097152], 4) = 0
++0.000 setsockopt(4, SOL_SOCKET, SO_RCVBUF, [145536], 4) = 0
+
+// 1. Warm up rcv_ssthresh to full window (~256 KB)
++0.000 < P. 1:36001(36000) ack 1 win 512
++0.000 > . 1:1(0) ack 36001
++0.000 read(4, ..., 36000) = 36000
+
++0.000 < P. 36001:72001(36000) ack 1 win 512
++0.000 > . 1:1(0) ack 72001
++0.000 read(4, ..., 36000) = 36000
+
++0.000 < P. 72001:108001(36000) ack 1 win 512
++0.000 > . 1:1(0) ack 108001
++0.000 read(4, ..., 36000) = 36000
+
++0.000 < P. 108001:144001(36000) ack 1 win 512
++0.000 > . 1:1(0) ack 144001
++0.000 read(4, ..., 36000) = 36000
+
+// 2. Queue 500 KB on TX so sk_wmem_queued >= sysctl_tcp_wmem[0]
++0.000 write(4, ..., 500000) = 500000
+
+// 3a. Inject 6 high-density 36 KB segments (216 KB payload, 222.7 KB truesize)
+//     and read 100 bytes from the head SKB so before(skb->seq, tp->copied_seq) is true.
++0.000 < P. 144001:180001(36000) ack 1 win 512
++0.000 read(4, ..., 100) = 100
+
++0.000 < P. 180001:216001(36000) ack 1 win 512
++0.000 < P. 216001:252001(36000) ack 1 win 512
++0.000 < P. 252001:288001(36000) ack 1 win 512
++0.000 < P. 288001:324001(36000) ack 1 win 512
++0.000 < P. 324001:360001(36000) ack 1 win 512
+
+// 3b. Inject 22 smaller 1000-byte segments into the remaining advertised rcv_wnd.
+//     Their higher truesize pushes sk_rmem_alloc > sk_rcvbuf (291072),
+//     triggering tcp_prune_queue -> tcp_collapse on the 216 KB of 36 KB SKBs
+//     and driving sk->sk_forward_alloc negative (-32768).
++0.000 < P. 360001:361001(1000) ack 1 win 512
++0.000 < P. 361001:362001(1000) ack 1 win 512
++0.000 < P. 362001:363001(1000) ack 1 win 512
++0.000 < P. 363001:364001(1000) ack 1 win 512
++0.000 < P. 364001:365001(1000) ack 1 win 512
++0.000 < P. 365001:366001(1000) ack 1 win 512
++0.000 < P. 366001:367001(1000) ack 1 win 512
++0.000 < P. 367001:368001(1000) ack 1 win 512
++0.000 < P. 368001:369001(1000) ack 1 win 512
++0.000 < P. 369001:370001(1000) ack 1 win 512
++0.000 < P. 370001:371001(1000) ack 1 win 512
++0.000 < P. 371001:372001(1000) ack 1 win 512
++0.000 < P. 372001:373001(1000) ack 1 win 512
++0.000 < P. 373001:374001(1000) ack 1 win 512
++0.000 < P. 374001:375001(1000) ack 1 win 512
++0.000 < P. 375001:376001(1000) ack 1 win 512
++0.000 < P. 376001:377001(1000) ack 1 win 512
++0.000 < P. 377001:378001(1000) ack 1 win 512
++0.000 < P. 378001:379001(1000) ack 1 win 512
++0.000 < P. 379001:380001(1000) ack 1 win 512
++0.000 < P. 380001:381001(1000) ack 1 win 512
++0.000 < P. 381001:382001(1000) ack 1 win 512
+
+// 4. Cap the socket memcg at its current usage and expect -EAGAIN on non-blocking write.
++0.010 `cgroot=$(mount -t cgroup2 | head -1 | awk '{print $3}')
+echo 0 > "${cgroot}/pkt_${PPID}/memory.high"
+cat "${cgroot}/pkt_${PPID}/memory.current" > "${cgroot}/pkt_${PPID}/memory.max"`
++0.000 fcntl(4, F_SETFL, O_RDWR|O_NONBLOCK) = 0
++0.000 write(4, ..., 16384) = -1 EAGAIN (Resource temporarily unavailable)
+
++0.000 close(4) = 0
++0.000 close(3) = 0
+
+// Untimed cleanup command executed by packetdrill on both pass and early failure.
+`cgroot=$(mount -t cgroup2 | head -1 | awk '{print $3}')
+[ -n "$cgroot" ] || exit 0
+runner_pid=$(awk '/^PPid:/ {print $2}' /proc/$PPID/status)
+orig_cg=$(sed -n 's/^0:://p' /proc/$runner_pid/cgroup)
+echo $PPID > "${cgroot}${orig_cg}/cgroup.procs" 2>/dev/null || true
+echo $$ > "${cgroot}${orig_cg}/cgroup.procs" 2>/dev/null || true
+rmdir "${cgroot}/pkt_${PPID}"`
-- 
2.56.0.176.g3575e65682-goog

Re: [PATCH net 2/2] selftests: net: packetdrill: add test for negative sk_forward_alloc in tcp_wmem_schedule()

[Kuniyuki Iwashima]

On Thu, Oct 15, 2026 at 12:37 AM Octavian Purdila <tavip@google.com> wrote:
>
> Add a packetdrill test that drives sk->sk_forward_alloc negative via
> tcp_collapse() on the receive queue while sk->sk_wmem_queued exceeds
> sysctl_tcp_wmem[0], and verifies that a subsequent non-blocking write()
> under TCP memory pressure fails with EAGAIN rather than returning a
> negative copy length from tcp_wmem_schedule() and failing with EFAULT.
>
> Assisted-by: LLM
> Signed-off-by: Octavian Purdila <tavip@google.com>

Reviewed-by: Kuniyuki Iwashima <kuniyu@google.com>

Thanks !

Re: [PATCH net 2/2] selftests: net: packetdrill: add test for negative sk_forward_alloc in tcp_wmem_schedule()

[Eric Dumazet]

Le ven. 16 oct. 2026 à 09:54, Kuniyuki Iwashima <kuniyu@google.com> a écrit :
>
> On Thu, Oct 15, 2026 at 12:37 AM Octavian Purdila <tavip@google.com> wrote:
> >
> > Add a packetdrill test that drives sk->sk_forward_alloc negative via
> > tcp_collapse() on the receive queue while sk->sk_wmem_queued exceeds
> > sysctl_tcp_wmem[0], and verifies that a subsequent non-blocking write()
> > under TCP memory pressure fails with EAGAIN rather than returning a
> > negative copy length from tcp_wmem_schedule() and failing with EFAULT.
> >
> > Assisted-by: LLM
> > Signed-off-by: Octavian Purdila <tavip@google.com>
>
> Reviewed-by: Kuniyuki Iwashima <kuniyu@google.com>
>
> Thanks !

Are you sure this test is passing on arches with 6K pages (as some
arm64 and ppc64 platforms) ?
We might need to add a check to only run it on 4K platforms.

Re: [PATCH net 2/2] selftests: net: packetdrill: add test for negative sk_forward_alloc in tcp_wmem_schedule()

[Eric Dumazet]

Le ven. 16 oct. 2026 à 10:22, Eric Dumazet <edumazet@kernel.org> a écrit :
>
> Le ven. 16 oct. 2026 à 09:54, Kuniyuki Iwashima <kuniyu@google.com> a écrit :
> >
> > On Thu, Oct 15, 2026 at 12:37 AM Octavian Purdila <tavip@google.com> wrote:
> > >
> > > Add a packetdrill test that drives sk->sk_forward_alloc negative via
> > > tcp_collapse() on the receive queue while sk->sk_wmem_queued exceeds
> > > sysctl_tcp_wmem[0], and verifies that a subsequent non-blocking write()
> > > under TCP memory pressure fails with EAGAIN rather than returning a
> > > negative copy length from tcp_wmem_schedule() and failing with EFAULT.
> > >
> > > Assisted-by: LLM
> > > Signed-off-by: Octavian Purdila <tavip@google.com>
> >
> > Reviewed-by: Kuniyuki Iwashima <kuniyu@google.com>
> >
> > Thanks !
>
> Are you sure this test is passing on arches with 6K pages (as some
> arm64 and ppc64 platforms) ?

This was meant to read : 64K pages.

> We might need to add a check to only run it on 4K platforms.

Re: [PATCH net 2/2] selftests: net: packetdrill: add test for negative sk_forward_alloc in tcp_wmem_schedule()

[Octavian Purdila]

On Fri, Oct 16, 2026 at 1:23 AM Eric Dumazet <edumazet@kernel.org> wrote:
>
> Le ven. 16 oct. 2026 à 10:22, Eric Dumazet <edumazet@kernel.org> a écrit :
> >
> > Le ven. 16 oct. 2026 à 09:54, Kuniyuki Iwashima <kuniyu@google.com> a écrit :
> > >
> > > On Thu, Oct 15, 2026 at 12:37 AM Octavian Purdila <tavip@google.com> wrote:
> > > >
> > > > Add a packetdrill test that drives sk->sk_forward_alloc negative via
> > > > tcp_collapse() on the receive queue while sk->sk_wmem_queued exceeds
> > > > sysctl_tcp_wmem[0], and verifies that a subsequent non-blocking write()
> > > > under TCP memory pressure fails with EAGAIN rather than returning a
> > > > negative copy length from tcp_wmem_schedule() and failing with EFAULT.
> > > >
> > > > Assisted-by: LLM
> > > > Signed-off-by: Octavian Purdila <tavip@google.com>
> > >
> > > Reviewed-by: Kuniyuki Iwashima <kuniyu@google.com>
> > >
> > > Thanks !
> >
> > Are you sure this test is passing on arches with 6K pages (as some
> > arm64 and ppc64 platforms) ?
>
> This was meant to read : 64K pages.
>
> > We might need to add a check to only run it on 4K platforms.

Ah, yes it would not pass. Thanks! I plan to add a check like the one
below in v2:

diff --git a/tools/testing/selftests/net/packetdrill/ksft_runner.sh
b/tools/testing/selftests/net/packetdrill/ksft_runner.sh
index 0a97d5ae34699..e2a418357bf1c 100755
--- a/tools/testing/selftests/net/packetdrill/ksft_runner.sh
+++ b/tools/testing/selftests/net/packetdrill/ksft_runner.sh
@@ -52,6 +52,12 @@ if [[ -n "${KSFT_MACHINE_SLOW}" ]]; then
  failfunc=ktap_test_xfail
 fi

+pagesize=$(grep -E '^--define=REQUIRES_PAGESIZE=' $script | cut -d
'=' -f 3 | tr -d '"')
+if [[ -n "$pagesize" && "$(getconf PAGESIZE)" != "$pagesize" ]]; then
+ ktap_skip_all "requires ${pagesize}B PAGE_SIZE"
+ exit "$KSFT_SKIP"
+fi
+
 ip_versions=$(grep -E '^--ip_version=' $script | cut -d '=' -f 2)
 if [[ -z $ip_versions ]]; then
  ip_versions="ipv4 ipv6 ipv4-mapped-ipv6"
diff --git a/tools/testing/selftests/net/packetdrill/tcp_wmem_schedule_negative_fwd_alloc.pkt
b/tools/testing/selftests/net/packetdrill/tcp_wmem_schedule_negative_fwd_alloc.pkt
index 8e31db546f432..5e6a40e740b42 100644
--- a/tools/testing/selftests/net/packetdrill/tcp_wmem_schedule_negative_fwd_alloc.pkt
+++ b/tools/testing/selftests/net/packetdrill/tcp_wmem_schedule_negative_fwd_alloc.pkt
@@ -3,6 +3,7 @@
 // when sk_wmem_schedule() fails under memory pressure and
 // sk->sk_wmem_queued >= sysctl_tcp_wmem[0], returning EAGAIN rather
than EFAULT.

+--define=REQUIRES_PAGESIZE="4096"
 --mtu=65535
 --tolerance_usecs=100000

Re: [PATCH net 2/2] selftests: net: packetdrill: add test for negative sk_forward_alloc in tcp_wmem_schedule()

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 6 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 3 · Low: 3

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] When cgroup2 or memcg is missing, the new test reports FAIL
  instead of SKIP.
- [Medium] The test relies on SO_SNDBUF=2097152 (line 38) and
  SO_RCVBUF=145536 (line 39) taking effect exactly.
- [Medium] The memcg cap at step 4 is racy, and the comment at line 32
  ('so only the sockets remain charged to pkt_${PPID}') is wrong.
- [Low] tools/testing/selftests/net/packetdrill/config adds
  `CONFIG_MEMCG=y` but not `CONFIG_CGROUPS=y`.
- [Low] The init command enables the memory controller for all children
  of the cgroup2 root (`echo +memory >…
- [Low] Teardown depends on packetdrill exiting normally.

--

selftests: net: packetdrill: add test for negative sk_forward_alloc in tcp_wmem_schedule()

This adds a packetdrill test that drives sk_forward_alloc negative via
tcp_collapse() while sk_wmem_queued exceeds tcp_wmem[0]. It then caps the
socket's memcg and checks that a non-blocking write() fails with EAGAIN
rather than EFAULT. CONFIG_MEMCG=y is also added to the packetdrill config
fragment.

> diff --git a/tools/testing/selftests/net/packetdrill/config b/tools/testing/selftests/net/packetdrill/config
> index b7df8bf30920..dc3cd72a8da8 100644
> --- a/tools/testing/selftests/net/packetdrill/config
> +++ b/tools/testing/selftests/net/packetdrill/config
> @@ -1,6 +1,7 @@
>  CONFIG_HZ=1000
>  CONFIG_HZ_1000=y
>  CONFIG_IPV6=y
> +CONFIG_MEMCG=y

[Severity: Low]
Should this fragment also add CONFIG_CGROUPS=y?

In init/Kconfig, MEMCG sits inside the CGROUPS block:

menuconfig CGROUPS
...
if CGROUPS
...
config MEMCG
	bool "Memory controller"

If this fragment is merged onto a base config with CGROUPS=n,
merge_config only prints a warning and MEMCG is dropped. The new test
then has no memory controller to use.

Other fragments that need memcg, such as
tools/testing/selftests/cgroup/config, list both CONFIG_CGROUPS=y and
CONFIG_MEMCG=y.

[ ... ]

> diff --git a/tools/testing/selftests/net/packetdrill/tcp_wmem_schedule_negative_fwd_alloc.pkt b/tools/testing/selftests/net/packetdrill/tcp_wmem_schedule_negative_fwd_alloc.pkt
> new file mode 100644
> index 000000000000..db232b54c554
> --- /dev/null
> +++ b/tools/testing/selftests/net/packetdrill/tcp_wmem_schedule_negative_fwd_alloc.pkt
> @@ -0,0 +1,116 @@

[ ... ]

> +`set -e
> +./defaults.sh
> +ip link set dev tun0 mtu 65535
> +sysctl -q net.ipv4.tcp_rmem="4096 131072 6291456"
> +sysctl -q net.ipv4.tcp_wmem="4096 16384 4194304"
> +cgroot=$(mount -t cgroup2 | head -1 | awk '{print $3}')
> +[ -n "$cgroot" ]

[Severity: Medium]
What happens on systems where this check or the cgroup writes below
fail?

The init command runs under set -e. Any of these makes it exit non-zero
and abort packetdrill:

- no cgroup2 mount (hybrid v1 setups, minimal VMs), so the
  [ -n "$cgroot" ] check fails
- the memory controller is bound to cgroup v1, so the +memory write
  gets ENOENT
- the test runs in a container whose cgroup namespace root holds
  processes, so the +memory write gets EBUSY
- cgroupfs is read-only, so the writes and the mkdir fail

ksft_runner.sh treats any non-zero packetdrill exit as a failure:

	&& ktap_test_pass $ip_version || $failfunc $ip_version

Here failfunc=ktap_test_fail. A missing optional feature is therefore
reported as three failures (ipv4, ipv6, ipv4-mapped-ipv6) instead of a
skip. No other packetdrill test depends on cgroups.

Could these prerequisites be detected and reported as a skip?

> +echo +memory > "${cgroot}/cgroup.subtree_control"

[Severity: Low]
Does anything turn the memory controller off at the root again?

The cleanup command at the end of the script moves tasks back and runs
rmdir "${cgroot}/pkt_${PPID}". It never writes -memory. On hosts where
memory was not already enabled in the root cgroup.subtree_control, memcg
accounting stays on for every existing and future top-level cgroup after
the test finishes.

Everything else the test changes is either per-netns or removed
explicitly. This looks like the only setup step with no matching
teardown.

Could the previous state be saved here and restored in cleanup?

> +mkdir -p "${cgroot}/pkt_${PPID}"
> +echo $PPID > "${cgroot}/pkt_${PPID}/cgroup.procs"
> +`

[Severity: Low]
Can a cgroup left over from an earlier run get reused here?

Packetdrill runs the cleanup command itself. If packetdrill is killed by
a signal (a kselftest timeout or SIGKILL), pkt_<pid> stays behind with
the limits set in step 4:

	echo 0 > "${cgroot}/pkt_${PPID}/memory.high"
	cat "${cgroot}/pkt_${PPID}/memory.current" > "${cgroot}/pkt_${PPID}/memory.max"

mkdir -p succeeds on an existing directory, and nothing resets
memory.high or memory.max before packetdrill is moved in. unshare -n
does not create a PID namespace. A later run that gets the same PID
would then do its socket setup and page faults under a near-zero cap.

Would a plain mkdir avoid this? Resetting memory.high and memory.max to
max after creating the directory would also work.

[ ... ]

> ++0.000 setsockopt(4, SOL_SOCKET, SO_SNDBUF, [2097152], 4) = 0
> ++0.000 setsockopt(4, SOL_SOCKET, SO_RCVBUF, [145536], 4) = 0

[Severity: Medium]
Does the test rely on these sizes being granted exactly?

sk_setsockopt() in net/core/sock.c clamps both values without reporting
it, and still returns 0:

		val = min_t(u32, val, READ_ONCE(sysctl_wmem_max));
	...
		__sock_set_rcvbuf(sk, min_t(u32, val, READ_ONCE(sysctl_rmem_max)));

net.core.wmem_max and net.core.rmem_max are global and read-only inside
a child netns. The test cannot raise them from its unshare -n
environment.

Suppose the host has lowered net.core.wmem_max, for example to 212992,
which gives an sk_sndbuf of 425984. The blocking
write(4, ..., 500000) in step 2 would then sit in
sk_stream_wait_memory(). Every injected segment carries ack 1, so no
send space is ever freed, and the script never reaches step 4.

Even if it did reach step 4, __sk_mem_raise_allocated() force charges
in this case:

		if (sk->sk_wmem_queued + size >= sk->sk_sndbuf) {
			/* Force charge with __GFP_NOFAIL */

The final write would then succeed instead of returning EAGAIN. A
lowered rmem_max would likewise change the 291072 byte sk_rcvbuf that
the collapse step depends on.

Packetdrill runs privileged. Would SO_SNDBUFFORCE and SO_RCVBUFFORCE, or
a getsockopt() check, make this deterministic?

[ ... ]

> +// 4. Cap the socket memcg at its current usage and expect -EAGAIN on non-blocking write.
> ++0.010 `cgroot=$(mount -t cgroup2 | head -1 | awk '{print $3}')
> +echo 0 > "${cgroot}/pkt_${PPID}/memory.high"
> +cat "${cgroot}/pkt_${PPID}/memory.current" > "${cgroot}/pkt_${PPID}/memory.max"`
> ++0.000 fcntl(4, F_SETFL, O_RDWR|O_NONBLOCK) = 0
> ++0.000 write(4, ..., 16384) = -1 EAGAIN (Resource temporarily unavailable)

[Severity: Medium]
Is this cap reliable enough for the EAGAIN check?

An earlier comment in the script says:

  // Move packetdrill back to its original cgroup so only the sockets remain charged to pkt_${PPID}

Cgroup v2 does not move existing charges when a task migrates. Whatever
packetdrill allocated while in pkt_${PPID} stays charged there. The same
goes for the sh, awk and sed helpers it forked. Any of those charges can
be released at a later point.

In mm/memcontrol.c, memory_high_write() calls drain_all_stock() once.
drain_all_stock() can skip the drain entirely, and for remote CPUs it
only queues work:

	if (!mutex_trylock(&percpu_charge_mutex))
		return;
	...
			else
				schedule_drain_work(cpu, &memcg_st->work);

memory_max_write() returns straight away, because max equals the
current usage:

		if (nr_pages <= max)
			break;

try_charge_memcg() uses the per-CPU stock before checking any limit:

	if (consume_stock(memcg, nr_pages))
		return 0;

Stock left for pkt_${PPID} on packetdrill's CPU could satisfy the
16384 byte charge. So could a remote drain or an uncharge that completes
after memory.current is read. Either way the charge in
tcp_wmem_schedule()->sk_wmem_schedule()->__sk_mem_raise_allocated()
would succeed and write() would return data. The test would then fail
intermittently with no kernel bug involved.

On systems with swap, can memory.high=0 also swap out packetdrill's own
anon pages that are charged to pkt_${PPID}? Swapping them back in would
charge the capped memcg, which has no task that can be killed.
Packetdrill could then stall in repeated page fault OOM retries.

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261015073736.2944215-1-tavip%40google.com

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

[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 0/2] tcp: clamp return value of tcp_wmem_schedule() to non-negative

[Octavian Purdila]

On Thu, Oct 15, 2026 at 12:43 AM <netdev-bot+sinfo@kernel.org> wrote:
>
> 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.
>

The issue was discovered in production, here is an example warning:

------------[ cut here ]------------
WARNING: CPU: 1 PID: 939331 at include/linux/thread_info.h:249
tcp_sendmsg+0x10c1/0x1530
Modules linked in: kvm_amd hdi2_standard_ftl hdi2_megablocks hdi2
xenolith stg elephant_dev_num nv_p2p_glue vfat fat livepatch_v1(OEK)
k10temp i2c_piix4 i2c_via_ipmi i2c_mux_ltc4306 google_bmc_usb gq
cdc_acm xhci_pciJun 18 01:48:26.051200 rdht15 kernel: [66690.105140]
CPU: 1 UID: 368767421 PID: 939331 Comm: DefaultEventMan Kdump: loaded
Tainted: P S      W  OE K     6.12.63-smp-1400.86.0.0 #1
Tainted: [P]=PROPRIETARY_MODULE, [S]=CPU_OUT_OF_SPEC, [W]=WARN,
[O]=OOT_MODULE, [E]=UNSIGNED_MODULE, [K]=LIVEPATCH
Hardware name: Google, Inc.
           Arcadia_IT_80/Arcadia_IT_80, BIOS 34.86.0-0 01/25/2026
RIP: 0010:tcp_sendmsg+0x10c1/0x1530
Code: ee 44 8b 64 24 3c 0f 84 ae f5 ff ff 48 8b 7c 24 48 e8 f3 a7 23
00 41 be f2 ff ff ff 4c 8b 6c 24 10 44 8b 24 24 e9 4a 03 00 00 <0f> 0b
31 c0 e9 7e f5 ff ff 41 80 8f 84 00 00 00 01 e9 00 f6 ff ff
RSP: 0018:ffffbaff64a6fc38 EFLAGS: 00010282
RAX: 0000000000000062 RBX: ffff9c45670d3c00 RCX: 25c2e5210d8ebf01
RDX: 0000000000600000 RSI: 00000000fff8427f RDI: ffff9c43f2fc0062
RBP: ffffffffffe4b17f R08: 0000000000000000 R09: 0000000000000002
R10: ffff9c4c187b6200 R11: 00000000000001b6 R12: 0000000000000000
R13: ffff9c4383ffc1e8 R14: 00000000ffe4b17f R15: ffff9c40fe838600
FS:  00007fe39fbaf700(0000) GS:ffff9c874ce40000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 000010986487a238 CR3: 0000000973fb8006 CR4: 0000000000f70ef0
PKRU: 55555554
Call Trace:
 <TASK>
 [<ffffffffa5c218a2>] __sys_sendmsg+0x292/0x490
 [<ffffffffa5c6281f>] ? restore_sigcontext+0x15f/0x1c0
 [<ffffffffa5c6e1d0>] ? __pfx_flush_tlb_func+0x10/0x10
 [<ffffffffa5c6e965>] ? flush_tlb_local+0x285/0x2a0
 [<ffffffffa5ba1b29>] ? sched_clock+0x9/0x10
 [<ffffffffa5bd7900>] ? sched_clock_cpu+0x10/0x180
 [<ffffffffa5be362a>] ? generic_smp_call_function_single_interrupt+0x19a/0x430
 [<ffffffffa5f51848>] do_syscall_64+0x88/0x340
 [<ffffffffa6000136>] entry_SYSCALL_64_after_hwframe+0x76/0x7e
RIP: 0033:0x5580e3f28a3d.