[PATCH v2 1/2] powerpc64/bpf: Fix build break in bpf_jit_emit_func_call_rel()

[PATCH v2 1/2] powerpc64/bpf: Fix build break in bpf_jit_emit_func_call_rel()

[Saket Kumar Bhaskar]

From: Madhavan Srinivasan <maddy@linux.ibm.com>

With CONFIG_PPC_KERNEL_PCREL enabled, build breaks with below error:

  CC      mm/dmapool.o
  CC      fs/readdir.o
arch/powerpc/net/bpf_jit_comp64.c: In function 'bpf_jit_emit_func_call_rel':
arch/powerpc/net/bpf_jit_comp64.c:475:13: error: unused variable 'ret' [-Werror=unused-variable]
  475 |         int ret;
      |             ^~~

Commit b55b6b9ad76c ("powerpc64/bpf: Add powerpc64 JIT support for timed may_goto")
introduced "ret" at function scope, but it is only used within its
respective conditional blocks. Same holds true for reladdr. Move both
variable declarations to the scopes where they are actually used:
"reladdr" to the CONFIG_PPC_KERNEL_PCREL block and "ret" to the non-PCREL
else block.

Fixes: b55b6b9ad76c ("powerpc64/bpf: Add powerpc64 JIT support for timed may_goto")
Signed-off-by: Madhavan Srinivasan <maddy@linux.ibm.com>
Signed-off-by: Saket Kumar Bhaskar <skb99@linux.ibm.com>
---
 arch/powerpc/net/bpf_jit_comp64.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/net/bpf_jit_comp64.c b/arch/powerpc/net/bpf_jit_comp64.c
index dab106cae22b..fc9db691e820 100644
--- a/arch/powerpc/net/bpf_jit_comp64.c
+++ b/arch/powerpc/net/bpf_jit_comp64.c
@@ -471,8 +471,6 @@ static int bpf_jit_emit_func_call(u32 *image, struct codegen_context *ctx, u64 f
 int bpf_jit_emit_func_call_rel(u32 *image, u32 *fimage, struct codegen_context *ctx, u64 func)
 {
 	unsigned long func_addr = func ? ppc_function_entry((void *)func) : 0;
-	long __maybe_unused reladdr;
-	int ret;
 
 	/* bpf to bpf call, func is not known in the initial pass. Emit 5 nops as a placeholder */
 	if (!func) {
@@ -487,6 +485,8 @@ int bpf_jit_emit_func_call_rel(u32 *image, u32 *fimage, struct codegen_context *
 	}
 
 #ifdef CONFIG_PPC_KERNEL_PCREL
+	long reladdr;
+
 	reladdr = func_addr - local_paca->kernelbase;
 
 	/*
@@ -525,7 +525,7 @@ int bpf_jit_emit_func_call_rel(u32 *image, u32 *fimage, struct codegen_context *
 	EMIT(PPC_RAW_BCTRL());
 #else
 	if (core_kernel_text(func_addr)) {
-		ret = bpf_jit_emit_func_call(image, ctx, func_addr, _R12);
+		int ret = bpf_jit_emit_func_call(image, ctx, func_addr, _R12);
 		if (ret)
 			return ret;
 	} else {
-- 
2.54.0

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

[Saket Kumar Bhaskar]

With CONFIG_PPC_KERNEL_PCREL enabled, calling bpf_check_timed_may_goto()
using a bl instruction results in a link-time failure:

arch/powerpc/net/bpf_timed_may_goto.o: in function `arch_bpf_timed_may_goto':
(.text+0x28): call to `bpf_check_timed_may_goto' lacks nop, can't restore toc

Use CFUNC() macro instead of direct 'bl' to properly annotate the call
to bpf_check_timed_may_goto(). On PCREL builds, CFUNC() expands to
'bl name@notoc', informing the linker that TOC restoration is not
needed, avoiding the "lacks nop, can't restore toc" linker error.

Fixes: b55b6b9ad76c ("powerpc64/bpf: Add powerpc64 JIT support for timed may_goto")
Signed-off-by: Saket Kumar Bhaskar <skb99@linux.ibm.com>
Reviewed-by: Christophe Leroy (CS GROUP) <chleroy@kernel.org>
---
 arch/powerpc/net/bpf_timed_may_goto.S | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/net/bpf_timed_may_goto.S b/arch/powerpc/net/bpf_timed_may_goto.S
index 6fd8b1c9f4ac..84ecf6fa7f5d 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)
 
 	/* Put return value back into AX */
 	mr      r12, r3
-- 
2.54.0

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

[sashiko-bot]

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