Re: [PATCH bpf-next v2 2/2] selftests/bpf: Add trampolines multi-level pointer params test coverage

[bot+bpf-ci@kernel.org]

[-- Attachment #1: Type: text/plain, Size: 2741 bytes --]

> diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
> index 178c4738e63b..19c82ae9bfe6 100644
> --- a/net/bpf/test_run.c
> +++ b/net/bpf/test_run.c

[ ... ]

> +static void *create_bad_kaddr(void)
> +{
> +	/*
> +	 * Try to get an address that passes kernel range checks but causes
> +	 * a page fault handler invocation if accessed from a BPF program.
> +	 */
> +#if defined(CONFIG_ARCH_HAS_SET_MEMORY) && defined(CONFIG_X86)

The first branch requires both CONFIG_ARCH_HAS_SET_MEMORY and
CONFIG_X86.  The second branch requires CONFIG_ARCH_HAS_SET_DIRECT_MAP.

PowerPC selects ARCH_HAS_SET_MEMORY (arch/powerpc/Kconfig) but not
ARCH_HAS_SET_DIRECT_MAP, so neither branch compiles in and the function
always returns NULL on that architecture.

> +	void *addr = vmalloc(PAGE_SIZE);
> +
> +	if (!addr)
> +		return NULL;
> +	/* Make it non-present - any access will fault */
> +	if (set_memory_np((unsigned long)addr, 1)) {
> +		vfree(addr);
> +		return NULL;
> +	}
> +	return addr;
> +#elif defined(CONFIG_ARCH_HAS_SET_DIRECT_MAP)

[ ... ]

> +#endif
> +	return NULL;
> +}

[ ... ]

> +static void free_bad_kaddr(void *addr)
> +{
> +	if (!addr)
> +		return;
> +
> +	/*
> +	 * Free an invalid test address created by get_invalid_address().
> +	 * Restores the page to present state before freeing.
> +	 */

The comment references get_invalid_address() but the function that
creates the address is create_bad_kaddr().

> +#if defined(CONFIG_ARCH_HAS_SET_MEMORY) && defined(CONFIG_X86)

[ ... ]

> +#endif
> +}

[ ... ]

> +		} else if (!strcmp(attach_name, "bpf_fentry_test13_pptr")) {
> +			kaddr = create_bad_kaddr();
> +			WARN_ON(!kaddr);

Since create_bad_kaddr() returns NULL on PowerPC (which supports BPF
trampolines via HAVE_DYNAMIC_FTRACE_WITH_ARGS), would this WARN_ON fire
every time the bpf_fentry_test13_pptr test runs on that architecture?

> +			CONSUME(bpf_fentry_test13_pptr(kaddr));
> +			CONSUME(bpf_fentry_test13_pptr((void **)19));
> +			CONSUME(bpf_fentry_test13_pptr(ERR_PTR(-ENOMEM)));
> +			break;

[ ... ]


---
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/22118172993

AI-authorship-score: low
AI-authorship-explanation: Stale comment referencing a renamed function and an architecture coverage gap with WARN_ON are characteristic of human-authored code that underwent refactoring.
issues-found: 2
issue-severity-score: medium
issue-severity-explanation: WARN_ON(!kaddr) fires unconditionally on PowerPC where create_bad_kaddr() always returns NULL, producing kernel warnings on every test run and potential panics with panic_on_warn.