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

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

[Octavian Purdila]

This series fixes a bug discovered in production 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) in
production 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.

Changes in v3:
- Fix packetdrill test failure in netdev CI (CONFIG_MAX_SKB_FRAGS=17):
  adjust segment sizes (35 KB) and buffer limits so the receive SKB
  truesize is identical across CONFIG_MAX_SKB_FRAGS=17 and 45, and
  tcp_collapse() deterministically drives sk_forward_alloc below -16 KB
  without invoking the memcg OOM killer.

Changes in v2:
- Include production warning trace in Patch 1 commit message.
- Skip the packetdrill test when PAGE_SIZE != 4096 via
  --define=REQUIRES_PAGESIZE="4096" in ksft_runner.sh.
- Collect Reviewed-by tags from Kuniyuki Iwashima and Eric Dumazet.

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  |   2 +
 .../selftests/net/packetdrill/ksft_runner.sh  |   6 +
 .../tcp_wmem_schedule_negative_fwd_alloc.pkt  | 108 ++++++++++++++++++
 4 files changed, 117 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/net/packetdrill/tcp_wmem_schedule_negative_fwd_alloc.pkt

-- 
2.56.0.307.g5fe7bd680c-goog

[PATCH v3 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)
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
Signed-off-by: Octavian Purdila <tavip@google.com>
Reviewed-by: Kuniyuki Iwashima <kuniyu@google.com>
Reviewed-by: Eric Dumazet <edumazet@kernel.org>
---
 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.307.g5fe7bd680c-goog

[PATCH v3 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>
Reviewed-by: Kuniyuki Iwashima <kuniyu@google.com>
Signed-off-by: Octavian Purdila <tavip@google.com>
---
 .../testing/selftests/net/packetdrill/config  |   2 +
 .../selftests/net/packetdrill/ksft_runner.sh  |   6 +
 .../tcp_wmem_schedule_negative_fwd_alloc.pkt  | 108 ++++++++++++++++++
 3 files changed, 116 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..a14a704dcbd49 100644
--- a/tools/testing/selftests/net/packetdrill/config
+++ b/tools/testing/selftests/net/packetdrill/config
@@ -1,6 +1,8 @@
+CONFIG_CGROUPS=y
 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/ksft_runner.sh b/tools/testing/selftests/net/packetdrill/ksft_runner.sh
index 0a97d5ae34699..dcda2b93c76e0 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
new file mode 100644
index 0000000000000..f8b7386fe77e2
--- /dev/null
+++ b/tools/testing/selftests/net/packetdrill/tcp_wmem_schedule_negative_fwd_alloc.pkt
@@ -0,0 +1,108 @@
+// 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.
+//
+// Buffer and segment sizes are tuned for 4 KB pages and standard
+// CONFIG_MAX_SKB_FRAGS values (17 to 45).
+
+--define=REQUIRES_PAGESIZE="4096"
+--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"
+rmdir "${cgroot}/pkt_${PPID}" 2>/dev/null || true
+mkdir "${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 35000,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 subsequent packetdrill allocations are not 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, [200000], 4) = 0
++0.000 setsockopt(4, SOL_SOCKET, SO_RCVBUF, [112500], 4) = 0
+
+// 1. Warm up rcv_ssthresh to full window
++0.000 < P. 1:35001(35000) ack 1 win 512
++0.000 > . 1:1(0) ack 35001
++0.000 read(4, ..., 35000) = 35000
+
++0.000 < P. 35001:70001(35000) ack 1 win 512
++0.000 > . 1:1(0) ack 70001
++0.000 read(4, ..., 35000) = 35000
+
++0.000 < P. 70001:105001(35000) ack 1 win 512
++0.000 > . 1:1(0) ack 105001
++0.000 read(4, ..., 35000) = 35000
+
++0.000 < P. 105001:140001(35000) ack 1 win 512
++0.000 > . 1:1(0) ack 140001
++0.000 read(4, ..., 35000) = 35000
+
+// 2. Queue 79 KB on TX so sk_wmem_queued >= sysctl_tcp_wmem[0], while leaving
+//    both tail SKB space (26 KB) and sk_page_frag space (19.3 KB) >= 16 KB.
++0.000 write(4, ..., 79000) = 79000
+
+// 3a. Inject 6 high-density 35 KB segments (210 KB payload, 222.7 KB truesize
+//     across 8-page order-3 frags on both CONFIG_MAX_SKB_FRAGS=17 and 45) and
+//     read 100 bytes from the head SKB so before(skb->seq, tp->copied_seq) is true.
++0.000 < P. 140001:175001(35000) ack 1 win 512
++0.000 < P. 175001:210001(35000) ack 1 win 512
++0.000 read(4, ..., 100) = 100
+
++0.000 < P. 210001:245001(35000) ack 1 win 512
++0.000 < P. 245001:280001(35000) ack 1 win 512
++0.000 < P. 280001:315001(35000) ack 1 win 512
++0.000 < P. 315001:350001(35000) ack 1 win 512
+
+// 3b. Inject three smaller segments into the remaining advertised rcv_wnd.
+//     Their higher truesize pushes sk_rmem_alloc > sk_rcvbuf (225000) and
+//     exhausts sk_forward_alloc, triggering tcp_prune_queue() -> tcp_collapse()
+//     on the 3rd segment and driving sk->sk_forward_alloc <= -16.4 KB.
++0.000 < P. 350001:350801(800) ack 1 win 512
++0.000 < P. 350801:351601(800) ack 1 win 512
++0.000 < P. 351601:351901(300) ack 1 win 512
+
+// 4. Drain per-CPU memcg stock via memory.high=0, cap memory.max=0 (O_NONBLOCK
+//    avoids synchronous reclaim/OOM), and expect -EAGAIN on a 16 KB non-blocking
+//    write. Because copy (16 KB) minus sk_forward_alloc (<= -16.4 KB) exceeds
+//    PAGE_ALLOC_COSTLY_ORDER (32 KB), try_charge_memcg() fails without invoking
+//    the memcg OOM killer.
++0.010 `cgroot=$(mount -t cgroup2 | head -1 | awk '{print $3}')
+echo 0 > "${cgroot}/pkt_${PPID}/memory.high"
+printf 0 | dd of="${cgroot}/pkt_${PPID}/memory.max" oflag=nonblock status=none`
++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.307.g5fe7bd680c-goog

Re: [PATCH v3 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 4 potential
issue(s) to consider.

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

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 cgroup v2 or a usable memory controller is missing,
  tcp_wmem_schedule_negative_fwd_alloc.pkt reports FAIL instead of SKIP.
- [Medium] Step 4 only gets EAGAIN if the per-CPU memcg_stock holds no
  leftover charge for pkt_${PPID} when write(4, ..., 16384) charges…
- [Medium] Between the init command (line 23, `echo $PPID >
  pkt_${PPID}/cgroup.procs`) and the move-back command (lines 38-41),…
- [Low] Test teardown does not mirror setup.

--

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

This adds a packetdrill test that pushes sk->sk_forward_alloc negative
through tcp_collapse() while sk_wmem_queued is above sysctl_tcp_wmem[0].
It then checks that a non-blocking write() under memcg pressure fails
with EAGAIN rather than EFAULT. It also adds CONFIG_CGROUPS and
CONFIG_MEMCG to the packetdrill config, and a PAGE_SIZE based skip to
ksft_runner.sh.

> 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..f8b7386fe77e2
> --- /dev/null
> +++ b/tools/testing/selftests/net/packetdrill/tcp_wmem_schedule_negative_fwd_alloc.pkt
> @@ -0,0 +1,108 @@

[ ... ]

> +`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"

[Severity: Medium]
What happens with this init command on a system where cgroup2 is not
mounted, or where the memory controller can't be enabled here? Because of
set -e, the init command aborts if any of these steps fails:

  - no cgroup2 mount, so [ -n "$cgroot" ] fails
  - memory is bound to cgroup v1 in a hybrid setup, CONFIG_MEMCG=n, or
    cgroup_disable=memory, so the cgroup.subtree_control write fails
  - the first cgroup2 mount is a non-root cgroup that holds tasks, which
    is common in containers, so enabling +memory fails with EBUSY

packetdrill then exits non-zero, and ksft_runner.sh counts that as a
failure:

	unshare -n packetdrill ${ip_args[$ip_version]} ${optargs[@]} $script > /dev/null \
		&& ktap_test_pass $ip_version || $failfunc $ip_version

The result is three failures (ipv4, ipv6, ipv4-mapped-ipv6) instead of a
skip. The only new prerequisite check in ksft_runner.sh is for PAGE_SIZE.

Could a matching skip be added for a missing cgroup2 memory controller?

> +rmdir "${cgroot}/pkt_${PPID}" 2>/dev/null || true
> +mkdir "${cgroot}/pkt_${PPID}"
> +echo $PPID > "${cgroot}/pkt_${PPID}/cgroup.procs"
> +`

[ ... ]

> +// Move packetdrill back to its original cgroup so subsequent packetdrill allocations are not 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"`

[Severity: Medium]
Does moving packetdrill back also move the memory charged while it ran in
pkt_${PPID}?

Between the init command's cgroup.procs write and this move-back, any
anonymous pages packetdrill touches for the first time (heap growth for
injected or sniffed packets, or stack) are charged to pkt_${PPID}. Cgroup
v2 does not move charges when a task migrates, so those pages stay
charged to pkt_${PPID}.

In step 4 the memory.high write is a blocking echo, so memory_high_write()
keeps calling:

		reclaimed = try_to_free_mem_cgroup_pages(memcg, nr_pages - high,
					GFP_KERNEL, MEMCG_RECLAIM_MAY_SWAP, NULL);

Socket memory can't be reclaimed, so the memcg never gets under high=0,
and reclaim continues until it stops making progress. If swap or zswap is
available, packetdrill's leftover anon pages can be swapped out at this
point.

memory.max is then set to 0. When packetdrill touches one of those pages
again, mem_cgroup_swapin_charge_folio() charges the memcg recorded in the
swap entry, as long as that memcg is still online:

	memcg = mem_cgroup_from_private_id(id);
	if (!memcg || !css_tryget_online(&memcg->css))
		memcg = get_mem_cgroup_from_mm(mm);

pkt_${PPID} stays online until the final rmdir. try_charge_memcg() can't
reclaim the socket memory, and the memcg OOM killer finds no tasks in
pkt_${PPID}. The fault would return VM_FAULT_OOM and be retried forever.

Can packetdrill livelock here? If the kselftest timeout then kills the
runner, the packetdrill child could be left spinning. Its cleanup command
would never run, and pkt_${PPID} would be left behind.

This depends on packetdrill touching new anonymous pages during that
window, and on swap or zswap being present.

[ ... ]

> +// 4. Drain per-CPU memcg stock via memory.high=0, cap memory.max=0 (O_NONBLOCK
> +//    avoids synchronous reclaim/OOM), and expect -EAGAIN on a 16 KB non-blocking
> +//    write. Because copy (16 KB) minus sk_forward_alloc (<= -16.4 KB) exceeds
> +//    PAGE_ALLOC_COSTLY_ORDER (32 KB), try_charge_memcg() fails without invoking
> +//    the memcg OOM killer.
> ++0.010 `cgroot=$(mount -t cgroup2 | head -1 | awk '{print $3}')
> +echo 0 > "${cgroot}/pkt_${PPID}/memory.high"
> +printf 0 | dd of="${cgroot}/pkt_${PPID}/memory.max" oflag=nonblock status=none`
> ++0.000 fcntl(4, F_SETFL, O_RDWR|O_NONBLOCK) = 0
> ++0.000 write(4, ..., 16384) = -1 EAGAIN (Resource temporarily unavailable)

[Severity: Medium]
Can this write() succeed because of leftover per-CPU memcg stock?

The comment relies on memory.high=0 to drain the stock, but
drain_all_stock() is best-effort. It returns right away if another drain
is running anywhere on the system:

mm/memcontrol.c:drain_all_stock() {
    ...
	/* If someone's already draining, avoid adding running more workers. */
	if (!mutex_trylock(&percpu_charge_mutex))
		return;
    ...
}

For remote CPUs it only queues the drain work and does not wait for it.

The memory.max write uses oflag=nonblock, so memory_max_write() takes the
early exit and never drains:

	if (of->file->f_flags & O_NONBLOCK)
		goto out;

The receive-side charges from step 3 go through try_charge_memcg() with a
MEMCG_CHARGE_BATCH batch, and refill_stock() leaves the surplus on the
charging CPU.

When write() charges memory through
tcp_wmem_schedule()->sk_wmem_schedule()->__sk_mem_raise_allocated()->
mem_cgroup_sk_charge()->try_charge_memcg(), the stock is consumed before
memory.max is checked:

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

consume_stock() accepts any nr_pages up to MEMCG_CHARGE_BATCH, and the
charge here is about 9 pages.

If the stock was not fully drained, the charge would succeed even with
memory.max=0, and write() would return 16384 instead of -1 EAGAIN. Could
this make the test flaky when other memcg activity is running on the
system?

> +
> ++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}"`

[Severity: Low]
The init command writes +memory to the root cgroup.subtree_control, but
this cleanup never restores the previous state. Is it intended that this
system-wide change stays in place after the test, whether it passes or
fails?

pkt_${PPID} is also removed only by this cleanup command, which never runs
if packetdrill is killed from outside, for example by the kselftest
timeout. The rmdir at the start of the init command only removes a
directory left by the same PID:

	rmdir "${cgroot}/pkt_${PPID}" 2>/dev/null || true

ksft_runner.sh starts a new unshare -n packetdrill for each of the three
IP versions, and each one gets a new PID. Can stale pkt_* cgroups pile up
across killed runs?

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