Re: [PATCH v2 2/2] powerpc64/bpf: Fix build break for arch_bpf_timed_may_goto

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The assembly trampoline `arch_bpf_timed_may_goto` allocates a fixed 112-byte stack frame and stores caller-saved BPF registers at offsets `32(r1)` to `72(r1)`. On PowerPC64 Big Endian (ELFv1 ABI), this writes directly into the caller's TOC save area and the callee's Parameter Save Area (PSA), causing silent stack corruption.
--

--- Patch [2]: [PATCH v2 2/2] powerpc64/bpf: Fix build break for arch_bpf_timed_may_goto ---
commit fc9f3132494d759b31f127f67b0e0815b4960f74
Author: Saket Kumar Bhaskar <skb99@linux.ibm.com>

powerpc64/bpf: Fix build break for arch_bpf_timed_may_goto

This commit fixes a link-time failure in the powerpc64 JIT for timed may_goto
when CONFIG_PPC_KERNEL_PCREL is enabled. It replaces a direct bl instruction
with the CFUNC() macro to properly annotate the call and avoid TOC restoration
linker errors.

> diff --git a/arch/powerpc/net/bpf_timed_may_goto.S b/arch/powerpc/net/bpf_timed_may_goto.S
> index 6fd8b1c9f4ac8..84ecf6fa7f5dc 100644
> --- a/arch/powerpc/net/bpf_timed_may_goto.S
> +++ b/arch/powerpc/net/bpf_timed_may_goto.S
> @@ -36,7 +36,7 @@ SYM_FUNC_START(arch_bpf_timed_may_goto)

[Severity: High]
This isn't a bug introduced by this patch, but does the stack frame allocation
in arch_bpf_timed_may_goto violate the PowerPC64 Big Endian (ELFv1) ABI?

Before this hunk, the trampoline allocates a fixed 112-byte stack frame and
saves caller-saved BPF registers:

	stdu    r1, -112(r1)

	/* Save BPF registers R0 - R5 (r8, r3-r7) */
	std     r3, 32(r1)
	std     r4, 40(r1)
	std     r5, 48(r1)
	std     r6, 56(r1)
	std     r7, 64(r1)
	std     r8, 72(r1)

Because STACK_FRAME_MIN_SIZE is exactly 112 bytes in ELFv1, writing to
offsets 40 through 72 places the saved registers directly into the TOC
save area (40) and the Parameter Save Area (48-72), which the ABI
reserves for linkage stubs and the callee.

When bpf_check_timed_may_goto executes, the ELFv1 ABI permits the C
compiler to spill its first argument into the PSA slot at 48(r1). Will
this silently overwrite BPF register R3 (stored at 48)? Similarly, could
linkage stubs clobber R2 by writing to the TOC save area at 40(r1)?

Since the BPF verifier assumes these registers are preserved by the
trampoline, this register clobbering could invalidate the verifier's
safety tracking at runtime and lead to out-of-bounds accesses.

>  	 * BPF_REG_FP is r31; BPF_REG_AX is r12 (stack offset in bytes).
>  	 */
>  	add     r3, r31, r12
> -	bl      bpf_check_timed_may_goto
> +	bl      CFUNC(bpf_check_timed_may_goto)
>  
>  	/* Put return value back into AX */
>  	mr      r12, r3

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1785387718.git.skb99@linux.ibm.com?part=2