Re: [PATCH 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:
- [Critical] The assembly trampoline `arch_bpf_timed_may_goto` saves BPF registers in the callee's parameter save area, exposing them to silent corruption by the C function `bpf_check_timed_may_goto`.
--

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

powerpc64/bpf: Fix build break for arch_bpf_timed_may_goto

With CONFIG_PPC_KERNEL_PCREL enabled, calling bpf_check_timed_may_goto()
using a bl instruction results in a link-time failure. This commit replaces
the direct 'bl' call with the CFUNC() macro to properly annotate the call,
avoiding the TOC restoration linker error.

> 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)
>  	 * 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)

[Severity: Critical]
This is a pre-existing issue, but does this function corrupt the saved BPF
registers by placing them in the callee's parameter save area?

In arch_bpf_timed_may_goto, the stack frame is allocated with 112 bytes, and
the BPF registers are saved from offsets 32(r1) to 72(r1):

    stdu    r1, -112(r1)
    ...
    std     r3, 32(r1)
    ...
    std     r8, 72(r1)

According to the PowerPC64 ELF ABI, offsets 32(r1) to 95(r1) in the caller's
frame are the parameter save area reserved for the callee.

When we call the C function bpf_check_timed_may_goto(), the compiler is
allowed to use this parameter save area (e.g., to spill the 'p' argument
when it calls ktime_get_mono_fast_ns()). If it does, it will silently
overwrite the saved BPF registers.

Upon return from bpf_check_timed_may_goto(), arch_bpf_timed_may_goto
restores these registers, which could now hold corrupted values:

    ld      r3, 32(r1)
    ...
    ld      r8, 72(r1)

Could this lead to silent data corruption or panics during BPF program
execution?

>  
>  	/* Put return value back into AX */
>  	mr      r12, r3

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