Re: [PATCH v3 06/58] objtool/klp: Grow the klp test harness vocabulary

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] objtool/klp: run_checksum failures swallowed in subshells mask test failures
--

commit 3b088080b3fc17cb3c6bcdacd83b5a2febf34918
Author: Song Liu [off-list ref]

objtool/klp: Grow the klp test harness vocabulary

Adds assertions for the output produced by klp diff, as well as for the
input objects. Includes helpers to extract and verify checksums recorded
for livepatch symbols.

[ ... ]

+checksum_of()
+{
+	local obj="$workdir/$1" sym="$2" off
+
+	run_checksum
+
+	off="$($READELF -rW "$obj" 2>/dev/null |
+	       awk -v s="$sym" '/rela\.discard\.sym_checksum/,/^$/ {
+			if ($5 == s) { print $1; exit }
+	       }')"
+
+	[ -n "$off" ] || return 1
+
+	$OBJCOPY -O binary --only-section=.discard.sym_checksum \
+		"$obj" "$workdir/checksums.bin" 2>/dev/null || return 1
+
+	dd if="$workdir/checksums.bin" bs=1 skip=$((16#$off + 8)) count=8 \
+		status=none | od -An -tx1 | tr -d ' \n'
+}
+
+# assert_checksum_differs <symbol> / assert_checksum_matches <symbol>
+#
+# Compare what klp checksum recorded for a symbol in the original against the
+# patched object.  This is what decides whether klp diff treats a function as
+# changed, so a test asserting only that the right functions were cloned cannot
+# tell a correct checksum from one which happens to differ.
+checksum_pair()
+{
+	orig_checksum="$(checksum_of "$orig_obj" "$1")"
+	patched_checksum="$(checksum_of "$patched_obj" "$1")"

[Severity: Medium]
Will run_checksum failures be swallowed by these subshells, potentially
masking test failures?

If objtool klp checksum fails for orig_obj, it triggers the call chain
checksum_pair() -> checksum_of() -> run_checksum() -> fail() in
tools/objtool/tests/lib.sh. Since this occurs within a subshell, fail()
exits only the subshell and its stdout (the "not ok..." string) is
captured into orig_checksum instead of terminating the test. No
.checksummed marker is created.

When checksum_of() runs for patched_obj, run_checksum() attempts to process
orig_obj again and fails identically, capturing the exact same failure
string into patched_checksum.

Both variables will contain the identical "not ok..." error string.

+
+	[ -n "$orig_checksum" ] ||
+		fail "no checksum recorded for '$1' in $orig_obj"
+	[ -n "$patched_checksum" ] ||
+		fail "no checksum recorded for '$1' in $patched_obj"
+}
+
+assert_checksum_differs()
+{
+	checksum_pair "$1"
+
+	[ "$orig_checksum" != "$patched_checksum" ] ||
+		fail "checksum for '$1' unchanged at $orig_checksum, expected it to differ"
+}
+
+assert_checksum_matches()
+{
+	checksum_pair "$1"
+
+	[ "$orig_checksum" = "$patched_checksum" ] ||

As a result, assert_checksum_matches() evaluates to true here, and the test
silently passes despite a toolchain failure.

+		fail "checksum for '$1' changed from $orig_checksum to" \
+		     "$patched_checksum, expected no change"
+}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260914230355.3422996-1-song@kernel.org?part=6