Re: [PATCH 2/2] selftests/rdma: Add netkit-based automated test script for RXE

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
- [Medium] selftests/rdma: cleanup() in rxe_pkg_netkit.sh deletes built test binary
- [Medium] selftests/rdma: incomplete resource teardown in cleanup() on error exit
- [Medium] selftests/rdma: rxe_pkg_netkit.sh discards all stdout and trace output
- [Medium] selftests/rdma: missing prerequisite and KSFT_SKIP checks in rxe_pkg_netkit.sh
- [Medium] selftests/rdma: out-of-bounds and uninitialized reads in rxe_netkit_dump
- [Medium] selftests/rdma: missing ring_buffer__new() check and cleanup in main()
--

commit e9728fe036ee321c65ecc0a2cfb9996d436c85d4
Author: Zhu Yanjun <yanjun.zhu@linux.dev>

selftests/rdma: Add netkit-based automated test script for RXE

This commit adds a shell script to automate functional and tracing tests
for the Soft-RoCE (RXE) driver over netkit network interfaces using rping
and rxe_netkit_dump.

> diff --git a/tools/testing/selftests/rdma/rxe_pkg_netkit.sh b/tools/testing/selftests/rdma/rxe_pkg_netkit.sh
> new file mode 100755
> index 0000000000000..6b544d9f49ae8
> --- /dev/null
> +++ b/tools/testing/selftests/rdma/rxe_pkg_netkit.sh
> @@ -0,0 +1,63 @@
> +#!/bin/sh
> +# SPDX-License-Identifier: GPL-2.0 OR Linux-OpenIB
> +# Copyright (c) 2026 Zhu Yanjun <yanjun.zhu@linux.dev>
> +
> +exec > /dev/null
> +
> +set -e

[Severity: Medium]
What happens if this test is run in an environment without root privileges,
mounted tracefs (/sys/kernel/debug/tracing/trace), kernel support or loaded
modules for netkit and rdma_rxe, or userspace utilities (ip, rdma, rping)?

Because set -e is enabled without checking prerequisites or sourcing
ktap_helpers.sh, any missing feature or command causes the script to abort
with exit status 1 instead of skipping with KSFT_SKIP (4).

> +
> +CUR_WD="$(dirname "$0")"
> +[ "$CUR_WD" = "." ] && CUR_WD="$(pwd)"
> +
> +cleanup() {
> +    set +e
> +    [ -n "$DUMP_PID" ] && kill -9 "$DUMP_PID" 2>/dev/null
> +    unset DUMP_PID
> +    ip netns delete ns0 2>/dev/null
> +    rm -f "$CUR_WD/rxe_netkit_dump" "$CUR_WD/rxe_pkg_kernel_dump.o" "$CUR_WD/rxe_pkg_kernel.skel.h"
> +}

[Severity: Medium]
Should cleanup() be deleting rxe_netkit_dump, rxe_pkg_kernel_dump.o, and
rxe_pkg_kernel.skel.h?

Because cleanup() is registered via trap cleanup EXIT, running
rxe_pkg_netkit.sh once deletes the compiled rxe_netkit_dump binary and build
artifacts from the working or installed kselftest directory. Since the script
does not build rxe_netkit_dump itself and invokes it in the background:

    "$CUR_WD/rxe_netkit_dump" &

any subsequent run of the test will fail to execute rxe_netkit_dump unless the
test suite is rebuilt, and the parent shell will not catch the missing binary
under set -e because the background subshell launch itself returns 0.

> +
> +trap cleanup EXIT
> +
> +# Clear tracing buffer
> +echo > /sys/kernel/debug/tracing/trace
> +
> +# Create network namespace and netkit peer links
> +ip netns add ns0
> +ip link add nk0 type netkit mode l2 peer name nk1
> +ip link set nk0 up
> +ip link set nk1 up
> +
> +# Configure namespace interface
> +ip link set nk0 netns ns0
> +ip netns exec ns0 ip addr add 10.0.0.1/24 dev nk0
> +ip netns exec ns0 ip link set nk0 up
> +
> +# Configure host interface
> +ip addr add 10.0.0.2/24 dev nk1
> +
> +# Start dump utility in background
> +"$CUR_WD/rxe_netkit_dump" &
> +DUMP_PID=$!

[Severity: Medium]
Does launching rxe_netkit_dump and running rping over nk1 expose unchecked
allocations and out-of-bounds reads in the helper utility?

First, in tools/testing/selftests/rdma/rxe_pkg_user_dump.c:main(), if
if_nametoindex("nk1") returns 0 after rxe_pkg_kernel_dump__open_and_load(),
main() returns 1 directly without jumping to cleanup to call
rxe_pkg_kernel_dump__destroy(skel):

    skel = rxe_pkg_kernel_dump__open_and_load();
    ...
    ifindex = if_nametoindex("nk1");
    if (ifindex == 0) {
        perror("if_nametoindex nk1 failed");
        return 1;
    }

Later in main(), ring_buffer__new() is passed directly to ring_buffer__poll()
without a NULL check:

    ringbuf = ring_buffer__new(bpf_map__fd(skel->maps.rb),
                   handle_event, NULL, NULL);

    while (1) {
        err = ring_buffer__poll(ringbuf, 100 /* timeout */);

If ring_buffer__new() returns NULL, ring_buffer__poll() in
tools/lib/bpf/ringbuf.c immediately dereferences rb->epoll_fd.

Second, in tools/testing/selftests/rdma/rxe_pkg_kernel_dump.c,
handle_netkit_ingress() captures every ETH_P_IP packet into a 512-byte ringbuf
record using:

    __u32 len = skb->len & 0x1FF;
    ...
    void *reserve = bpf_ringbuf_reserve(&rb, 512, 0);

Using & 0x1FF wraps packet lengths >= 512 modulo 512 instead of clamping to
512, and leaves trailing bytes in the 512-byte ringbuf slot uninitialized.

When rping runs here, it first establishes an RDMA CM connection over TCP/IPv4
before sending RoCEv2 UDP traffic. In
tools/testing/selftests/rdma/rxe_pkg_user_dump.c:print_bpf_output(), the
size parameter is ignored, iph->protocol and the UDP port are not checked,
rxe_opcode[bth->opcode].name is passed to printf() even when it is NULL on
non-RoCEv2 packets or uninitialized ringbuf bytes, and the RXE_PAYLOAD loop:

    int len = ntohs(udp_hdr->len) - rxe_opcode[bth->opcode].length;

    for (int j = 0; j < len; j++) {

reads cur_p[j] without bounding against the captured buffer size. Also in
rxe_pkg_user_dump.c, atmeth_va(), atmeth_swap_add(), atmeth_comp(), and
atmack_orig() apply 32-bit ntohl() to 64-bit __be64 fields.

> +
> +# Add RXE links over netkit devices
> +ip netns exec ns0 rdma link add rxe0 type rxe netdev nk0
> +rdma link add rxe1 type rxe netdev nk1
> +
> +# Start rping server in namespace
> +ip netns exec ns0 rping -s -a 10.0.0.1 -C 3 &
> +
> +# Add wait time
> +sleep 1
> +
> +# Run rping client on host
> +rping -c -a 10.0.0.1 -C 3
> +
> +# Tear down RDMA links and netkit devices
> +ip netns exec ns0 rdma link del rxe0
> +rdma link del rxe1
> +ip link del nk1
> +ip netns delete ns0

[Severity: Medium]
Can an error prior to this teardown block leak the background rping server,
the ns0 network namespace, the rxe0 and rxe1 RDMA links, and the nk0 and nk1
netkit interfaces?

The rping server is started in the background without saving its PID, and the
teardown of rxe0, rxe1, and nk1 only happens on the normal completion path
above. If an earlier command such as the client rping -c fails under set -e,
execution jumps to cleanup(), which only kills DUMP_PID and runs
ip netns delete ns0.

Because ip netns delete ns0 only unlinks the namespace mount without killing
processes inside it, the live rping -s process keeps ns0, nk0, rxe0, and the
host peers nk1 and rxe1 pinned in the kernel, causing subsequent test runs to
fail.

> +
> +# Display kernel trace output
> +cat /sys/kernel/debug/tracing/trace

[Severity: Medium]
Is all output from cat /sys/kernel/debug/tracing/trace and rxe_netkit_dump
intentionally discarded here?

Because exec > /dev/null is at the top of the script, all standard output for
the script and its child processes goes to /dev/null. In addition,
rxe_netkit_dump is launched asynchronously in the background without waiting
for the BPF program to load and attach to nk1 before rping starts, and nothing
verifies that either the BPF packet dump or the kernel trace buffer captured
any events.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261019041912.29488-1-yanjun.zhu@linux.dev?part=2