Re: [PATCH bpf-next] selftests/bpf: Track test_xdp_features DUT processes

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The test script deadlocks indefinitely during cleanup if terminated while the DUT server is blocked in I/O.
- [High] The cleanup routine sends `SIGTERM` to historical PIDs that have already exited, risking the termination of unrelated host processes if PIDs wrap around.
--

--- Patch [1]: [PATCH bpf-next] selftests/bpf: Track test_xdp_features DUT processes ---
commit 3a3cec0883174021af4192cc572fb4e4f76dbd7c
Author: Bochao Cao <[email protected]>

selftests/bpf: Track test_xdp_features DUT processes

test_xdp_features.sh waits for any xdp_features listener to appear and
uses pidof during cleanup. A concurrent test can therefore make another
test proceed before its own DUT is ready, and cleanup kills every
xdp_features process on the host. Record each DUT PID when it is spawned
and only terminate and reap recorded children.

Closes: https://bugs.debian.org/1136522

> diff --git a/tools/testing/selftests/bpf/test_xdp_features.sh b/tools/testing/selftests/bpf/test_xdp_features.sh
> index 0aa71c4455c05..f3aa4d1d52be2 100755
> --- a/tools/testing/selftests/bpf/test_xdp_features.sh
> +++ b/tools/testing/selftests/bpf/test_xdp_features.sh

[ ... ]

> @@ -31,32 +32,53 @@ setup() {
>  }
>  
>  cleanup() {
> -	ip link del v1 2> /dev/null
> -	ip netns del ${NS} 2> /dev/null
> -	[ "$(pidof xdp_features)" = "" ] || kill $(pidof xdp_features) 2> /dev/null
> +	for pid in "${dut_pids[@]}"; do
> +		kill "$pid" 2> /dev/null || true

[Severity: High]
Does this risk killing unrelated host processes if PIDs wrap around?

The script spawns a background xdp_features process for each test and records
its PID in dut_pids. During a normal test, the tester process sends a
CMD_STOP, causing the DUT to exit cleanly, at which point the OS reaps
the PID.

At the end of the script, cleanup blindly loops over all recorded PIDs (most
of which have already exited) and unconditionally executes kill "$pid". If
the system is busy and any of those freed PIDs have been reassigned to
unrelated host processes, they will receive SIGTERM as root.

> +		wait "$pid" 2> /dev/null || true

[Severity: High]
Can this script deadlock indefinitely during cleanup?

If the script fails or receives a signal, the EXIT trap runs cleanup, sending
SIGTERM to the DUT processes and calling wait on the PID.

The xdp_features server uses glibc's signal(), which defaults to SA_RESTART:

tools/testing/selftests/bpf/xdp_features.c:main() {
    ...
    signal(SIGTERM, sig_handler);
    ...
}

When the server receives SIGTERM, its handler sets exiting = true, but the
blocking accept() or recv() syscall is automatically restarted by the kernel.
The server remains blocked forever and never exits, causing the script's
wait to hang indefinitely.

> +	done
> +	ip link del v1 2> /dev/null || true
> +	ip netns del "${NS}" 2> /dev/null || true
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260805-xdp-dut-process-lifecycle-gmail-v1-1-45984df8d295@gmail.com?part=1