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

On Thu, Jul 30, 2026 at 05:56:41AM +0000, [email protected] wrote:
> 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 <[email protected]>
> 
> 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.
> 
Since this is not related to the change in current patch, will fix this
in another set of patches.

-Saket
> >  	 * 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/[email protected]?part=2