Re: [bpf-next 4/4] selftests/bpf: vmtest.sh: preserve command quoting when running in the VM

--===============7125088897932189663==
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit

> diff --git a/tools/testing/selftests/bpf/vmtest.sh b/tools/testing/selftests/bpf/vmtest.sh
> index 9ca8022853933..6a3d026d76bd6 100755
> --- a/tools/testing/selftests/bpf/vmtest.sh
> +++ b/tools/testing/selftests/bpf/vmtest.sh
> @@ -428,8 +428,17 @@ main()
>
>  	if [[ $# -eq 0  && "${debug_shell}" == "no" ]]; then
>  		echo "No command specified, will run ${DEFAULT_COMMAND} in the vm"
> -	else
> -		command="$@"
> +	elif [[ $# -gt 0 ]]; then
> +		# Quote each argument so the command survives into the guest: the
> +		# host expands ${command} into the generated init script, which
> +		# the guest bash then parses as shell source. Without the %q
> +		# escapes an argument with a space or a glob (e.g. -a 'verifier_*')
> +		# is re-split and expanded against /root/bpf there.
> +		#
> +		# Skip this when there is no command: printf '%q ' would still
> +		# apply the format once and emit '', which is not the empty
> +		# command that -s (debug shell) expects.
> +		command=$(printf '%q ' "$@")
>  	fi
>
>  	local kconfig_file="${OUTPUT_DIR}/latest.config"

The commit message states:

>     Note this makes the trailing command strictly an argv rather than a shell
>     snippet: passing it pre-quoted as one word, e.g.
>
>             vmtest.sh -- "./test_progs -t foo"
>
>     no longer works, and neither does embedding guest-side shell syntax such
>     as ';' or a redirection. Neither form is documented - usage() and
>     README.rst both show the command unquoted - and 'sh -c ...' still works.

Is the claim about README.rst accurate? Looking at
tools/testing/selftests/bpf/README.rst lines 106-115, there is a
documented vmtest.sh invocation whose trailing command is explicitly
quoted:

  $ PLATFORM=riscv64 CROSS_COMPILE=riscv64-linux-gnu- \
    tools/testing/selftests/bpf/vmtest.sh \
    -l <path of local rootfs image> -- \
    ./test_progs -d \
        \"$(cat tools/testing/selftests/bpf/DENYLIST.riscv64 | ... | tr -s '\n' ',' )\"

The `\"` are escaped in the user's host shell, so vmtest.sh's main()
receives argv[3] as the literal 10-character string `",exceptions,"`
including the double quotes.

Under the old `command="$@"`, the guest bash's second parse of the init
script performed quote removal on those literal quotes, so test_progs
received `-d ,exceptions,`. That is exactly what the `\"` in the recipe
are for: they survive the host and are consumed by the guest.

With `command=$(printf '%q ' "$@")` the quotes are escaped
(`\",exceptions,\"`) and reach the binary verbatim, so test_progs now
receives `-d ",exceptions,"`.

Downstream that argument goes to parse_arg() ARG_TEST_NAME_GLOB_DENYLIST
-> parse_test_list() (tools/testing/selftests/bpf/testing_helpers.c),
which strtok_r()s on ',' and hands each field to insert_test() with
is_glob_pattern=true. So the filter list becomes three globs instead of
one: ["] [exceptions] ["], two of which can never match a test name in
glob_match() (test_progs.c).

For DENYLIST.riscv64 as it stands today the damage is confined to those
two junk filters, because the file's first line is a comment and
therefore reduces to an empty leading field.

The same recipe applied to a denylist file whose first line is a real
entry silently drops that entry. With DENYLIST.asan-shaped input:

  OLD guest -> test_progs -d [*arena*,task_local_data,uprobe_multi_test,]
    filters: [*arena*] [task_local_data] [uprobe_multi_test]
  NEW guest -> test_progs -d ["*arena*,task_local_data,uprobe_multi_test,"]
    filters: ["*arena*] [task_local_data] [uprobe_multi_test] ["]

`"*arena*` matches nothing, so the *arena* tests are no longer denied
and run anyway, a silent loss of denylist coverage rather than an error.

Should the commit message be corrected to acknowledge that README.rst
does document a quoted invocation that breaks with this change, and
should README.rst lines 109-115 be updated in the same patch to remove
the now-harmful `\"`?


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/30836252778
--===============7125088897932189663==--