Re: [RFC PATCH v2 11/11] ptwrite uprobes: Add kernel self tests

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] kselftests/uprobes: test unconditionally skips due to undefined FLT_V variable
- [Medium] kselftests/uprobes: unaligned_nopfn site does not cross 8-byte boundary, invalidating test
- [Low] kselftests/uprobes: 64-bit inline assembly causes build failure on 32-bit x86
--

commit af4e3ff18548d1ab18ec7d6c2b846c7b5c15b34e
Author: Andi Kleen <ak@kernel.org>
Subject: ptwrite uprobes: Add kernel self tests

This commit adds selftests for PTWRITE uprobes to exercise instruction
punning, multinop patching, and probe lifecycle on x86 architectures.

> diff --git a/tools/testing/selftests/uprobes/Makefile b/tools/testing/selftests/uprobes/Makefile
> new file mode 100644
> index 0000000000000..ee97442f8c9dc
> --- /dev/null
> +++ b/tools/testing/selftests/uprobes/Makefile
> @@ -0,0 +1,14 @@
[ ... ]
> +TEST_PROGS := run_ptw.sh
> +
> +ifneq ($(filter x86 x86_64,$(ARCH)),)
> +TEST_GEN_FILES := ptw_probe
> +else

[Severity: Low]
Will this fail to build on 32-bit x86 architectures? The Makefile enables test
compilation for the x86 architecture filter, which includes 32-bit builds.
However, ptw_probe.c uses strict 64-bit inline assembly like movabs, %rax,
and %rsp. When compiled with a 32-bit toolchain, won't the assembler reject
these instructions?

> diff --git a/tools/testing/selftests/uprobes/ptw_probe.c b/tools/testing/selftests/uprobes/ptw_probe.c
> new file mode 100644
> index 0000000000000..b232fbd4d2aa0
> --- /dev/null
> +++ b/tools/testing/selftests/uprobes/ptw_probe.c
> @@ -0,0 +1,156 @@
[ ... ]
> +static __attribute__((noipa)) uint64_t
> +unaligned_nopfn(uint64_t a)
> +{
> +	asm volatile(".p2align 3, 0x90\n\t"
> +		     "nop\n\t"
> +		     ".globl nopfn_unaligned_site\n\t"
> +		     "nopfn_unaligned_site:\n\t"
> +		     "nop\n\tnop\n\tnop\n\tnop\n\tnop" ::: "memory");
> +	return a * 13 + 5;
> +}

[Severity: Medium]
Does this actually create an unaligned sequence that crosses an 8-byte
boundary?

The assembly code aligns to 8 bytes with .p2align 3 and adds a 1-byte nop,
placing the start of the 5-byte target sequence at offset 1. Since offsets 1
through 5 fit perfectly within a single 8-byte word, the kernel's atomic
patch path can handle it safely.

Won't the kernel accept this probe instead of rejecting it, causing test 11 in
run_ptw.sh to incorrectly report a failure when it expects the kernel to
reject the probe?

> diff --git a/tools/testing/selftests/uprobes/run_ptw.sh b/tools/testing/selftests/uprobes/run_ptw.sh
> new file mode 100755
> index 0000000000000..a16b7adeabb74
> --- /dev/null
> +++ b/tools/testing/selftests/uprobes/run_ptw.sh
> @@ -0,0 +1,219 @@
[ ... ]
> +PUN_V=$(objdump -d "$BIN" | awk '/^[0-9a-f]+ <punfn>:/{print $1;exit}' | tr -d ':')
> +JCC_V=$(objdump -d "$BIN" |
> +	awk '/^[0-9a-f]+ <jcc8>:/ {f=1; next} f&&/jne/{print $1; exit}' |
> +	tr -d ':')
> +NOP_V=$(objdump -d "$BIN" | awk '/^[0-9a-f]+ <nopfn_site>:/{print $1;exit}' | tr -d ':')
> +NOP5_V=$(objdump -d "$BIN" | awk '/^[0-9a-f]+ <nop5>:/{print $1;exit}' | tr -d ':')
> +UNOP_V=$(objdump -d "$BIN" |
> +	awk '/^[0-9a-f]+ <nopfn_unaligned_site>:/{print $1;exit}' | tr -d ':')
> +RZ_V=$(objdump -d "$BIN" | awk '/^[0-9a-f]+ <rz_probe_site>:/{print $1;exit}' | tr -d ':')
> +if [ -z "$PUN_V" ] || [ -z "$JCC_V" ] || [ -z "$FLT_V" ] ||
> +   [ -z "$NOP_V" ] || [ -z "$NOP5_V" ] || [ -z "$UNOP_V" ] ||
> +   [ -z "$RZ_V" ]; then
> +	echo "1..0 # SKIP unable to resolve ptw_probe symbols"
> +	exit 0
> +fi

[Severity: Medium]
Is the FLT_V variable supposed to be defined or populated before this check?

The script tests for the existence of an undefined variable FLT_V, which
causes this condition to always evaluate to true. Doesn't this mean the entire
ptwrite uprobe selftest unconditionally skips execution and reports
'unable to resolve ptw_probe symbols' on every run, resulting in no test
coverage?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260917230127.924985-2-ak@kernel.org?part=11