Re: [PATCH net-next v2 5/6] selftests/vsock: test the guest vsock device network namespace

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] kselftest/vsock: missing TEST_FILES breaks make install
- [Medium] netlink: vsock dev-netns-set violates uAPI by not replying
- [Medium] kselftest/vsock: subshell termination leaks ssh and socat processes
--

commit 3122f274adb12a4633dc3d5fee699cdb3ce877cf
Author: Bobby Eshleman <bobbyeshleman@meta.com>

selftests/vsock: test the guest vsock device network namespace

Add tests for guest vsock device namespace assignment, including ns
destruction and the device moving between namespaces.

> diff --git a/tools/testing/selftests/vsock/vmtest.sh b/tools/testing/selftests/vsock/vmtest.sh
> index 310dfc2a39ad5..e94cc2fd90ccd 100755
> --- a/tools/testing/selftests/vsock/vmtest.sh
> +++ b/tools/testing/selftests/vsock/vmtest.sh
> @@ -17,6 +17,8 @@ readonly KERNEL_CHECKOUT=$(realpath "${SCRIPT_DIR}"/../../../../)
>  source "${SCRIPT_DIR}"/../kselftest/ktap_helpers.sh
>  
>  readonly VSOCK_TEST="${SCRIPT_DIR}"/vsock_test
> +readonly YNL_CLI="${KERNEL_CHECKOUT}"/tools/net/ynl/pyynl/cli.py
> +readonly VSOCK_SPEC="${KERNEL_CHECKOUT}"/Documentation/netlink/specs/vsock.yaml
>  readonly TEST_GUEST_PORT=51000
>  readonly TEST_HOST_PORT=50000

[Severity: Medium]
Does this break running the tests from an installed location (via make
install)? Since cli.py and vsock.yaml are not added to TEST_FILES in the
selftests Makefile, they won't be copied during installation.

When check_deps() runs in the installed environment, it will fail to find
these files at the hardcoded ${KERNEL_CHECKOUT} paths and unconditionally
skip the test suite.

[ ... ]

> +vm_ns_assign_g2h() {
> +	local ns=$1
> +	local nspid=$2
> +
> +	vm_ns_exec "${ns}" "${nspid}" "python3 /root/ynl/cli.py --no-schema \
> +		--spec /root/ynl/vsock.yaml --do dev-netns-set"
> +}

[Severity: Medium]
Does dev-netns-set in vsock.yaml violate the Netlink subsystem guidelines?
The operation in vsock.yaml is defined as "do: {}" with no reply, but the
guidelines require that all new operations must reply with a full message
(such as the assigned netns ID) rather than just an ACK to ensure future
extensibility.

[ ... ]

> +host_send_to_guest() {
> +	local ns=$1
> +	local nspid=$2
> +	local port=$3
> +	local outfile=$4
> +	local cmd="socat -u VSOCK-LISTEN:${port} STDOUT"
> +	local dst="VSOCK-CONNECT:${VSOCK_CID}:${port}"
> +	local pid
> +
> +	vm_ns_exec "${ns}" "${nspid}" "${cmd}" > "${outfile}" 2>/dev/null &
> +	pid=$!

[Severity: Medium]
Does this leak the ssh client and remote socat processes? vm_ns_exec is a
bash function, so backgrounding it means $! captures the local subshell's
PID rather than the ssh client's PID.

When terminate_pids sends SIGTERM to the subshell, the subshell exits, but
bash does not forward the signal to its children (the ssh client).

The orphaned ssh process keeps the connection open. Because no PTY is
allocated, the remote sshd does not send SIGHUP, leaving the remote socat
process running and holding the port open, which could cause EADDRINUSE
failures in subsequent shared VM tests.

[ ... ]

> +test_ns_guest_assign_g2h_netns_old_conn_send_fails() {
> +	local gap=$(( WAIT_PERIOD * 3 ))
> +	local port=12346
> +	local outfile
> +	local result
> +	local sender
> +	local nspid
> +	local pid
> +
> +	nspid=$(vm_ns_start "init_ns")
> +	if [[ -z "${nspid}" ]]; then
> +		log_host "failed to create a namespace inside the guest"
> +		return "${KSFT_FAIL}"
> +	fi
> +
> +	outfile=$(mktemp)
> +	socat -u VSOCK-LISTEN:"${port}" STDOUT > "${outfile}" 2>/dev/null &
> +	pid=$!
> +	host_wait_for_listener "init_ns" "${port}" "vsock"
> +
> +	# Send a message, wait, then send another. While waiting, assign the
> +	# device to a namespace. Confirm the second message does not arrive.
> +	vm_ssh "init_ns" -- \
> +		"(echo FIRST; sleep ${gap}; echo SECOND) |" \
> +		"socat -u STDIN VSOCK-CONNECT:2:${port}" &>/dev/null &
> +	sender=$!

[Severity: Medium]
Similarly here, does backgrounding the vm_ssh bash function cause it to
orphan the ssh and socat processes when sender is killed?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260921-vsock-guest-ns-v2-0-693bd78fde9e@meta.com?part=5