Re: [PATCH v3 21/58] objtool/klp: Add test for rejecting module-owned static call keys

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] objtool/klp test: manual .static_call_sites generation violates test design
- [Low] objtool/klp test: test-static-call-module-key.sh misses validation assertions
--

commit 07aa87afa035746af6f6fbcdd7b1840b00a88b3e
Author: Puranjay Mohan <puranjay@kernel.org>

objtool/klp: Add test for rejecting module-owned static call keys

This patch adds a test to verify that klp diff correctly accepts vmlinux-owned
static call keys and rejects module-owned ones. It introduces a C fixture
implementing the static call site and a bash script to test both paths.

> diff --git a/tools/objtool/tests/generic/fixtures/static_call.c b/tools/objtool/tests/generic/fixtures/static_call.c
> index 0000000000000..5975986c92eb0
> --- /dev/null
> +++ b/tools/objtool/tests/generic/fixtures/static_call.c
[ ... ]
> @@ -0,0 +1,53 @@
[ ... ]
> +int target(int x)
> +{
> +#if defined(NEW_CALL) && !defined(PATCHED)
> +	/* The original has no static call at all. */
> +	return x + 1;
> +#else
> +	__asm__ volatile(
> +		"1:	nop\n\t"
> +		".pushsection	.static_call_sites, \"aw\"\n\t"

[Severity: Low]
Does this manual instantiation of the static_call_sites section bypass objtool's
check generation?

According to klp-write-tests.txt documentation, tests should not hand-write
what objtool generates. This creates a synthetic edge case that artificially
omits sh_entsize and adds .discard.annotate_data boundaries, which might not
reflect how production objects are built.

> +		".balign	8\n\t"
> +		"912:\n\t"
[ ... ]
> diff --git a/tools/objtool/tests/generic/test-static-call-module-key.sh b/tools/objtool/tests/generic/test-static-call-module-key.sh
> index 0000000000000..9ab373e90e10e
> --- /dev/null
> +++ b/tools/objtool/tests/generic/test-static-call-module-key.sh
[ ... ]
> @@ -0,0 +1,25 @@
[ ... ]
> +setup
> +build_pair static_call.c
> +
> +has_input_section orig.o .static_call_sites ||
> +	probe_skip "fixture produced no .static_call_sites on this arch"
> +
> +run_diff
> +assert_patched target

[Severity: Low]
Does this successfully verify that the section was extracted and patched?

Without a check like assert_section .static_call_sites, this test might pass
vacuously if klp diff silently drops the static call section.

> +
> +rm -f "$workdir/out.o"
> +build_pair static_call.c -DMODNAME='"klp_testmod"'
> +run_diff 255
> +
> +diff_log | grep -q 'unsupported static call key __SCK__klp_test_call' ||
> +	fail "expected rejection, got: $(diff_log | tail -1)"
> +

[Severity: Low]
Should there be a check here to ensure the malformed output object wasn't
leaked?

Usually module rejection paths are validated with something like
[ -e "$workdir/out.o" ] && fail to ensure the file was not produced.

> +pass "module-owned static call key rejected, vmlinux-owned accepted"

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