Hi Abhishek,
On 16/06/26 10:17 pm, [email protected] wrote:
From: Abhishek Dubey <[email protected]>
During size calculation in pass-0, exit_addr is 0 since addrs[fp->len]
is not yet populated. bpf_jit_emit_exit_insn() treats a zero exit_addr
as in-range and skips bpf_jit_build_epilogue(), so the alternate inline
epilogue instructions are not counted in alloclen.
In later passes, if the real exit_addr falls outside the 32MB branch
range, the full inline epilogue is emitted into the already-allocated
buffer, writing past its end and corrupting adjacent memory.
Fix by ensuring exit_addr is non-zero before treating it as in-range,
so pass-0 always falls through to bpf_jit_build_epilogue() and
conservatively accounts for all epilogue instructions in alloclen.
Also range check alt_exit_addr directly in the else-if condition.
Since exit_addr handling now falls through to the epilogue, two
related issues in bpf_int_jit_compile() must also be addressed:
1. Reset cgctx.alt_exit_addr before the second size-calculation pass.
Without this, a stale alt_exit_addr from the first pass causes the
second pass to emit a single jump instead of the full epilogue,
undercounting alloclen and introducing the overflow.
2. Recompute addrs[fp->len] at the end of each code-generation pass.
The larger body from pass-0 might shrink in later passes; a stale
addrs[fp->len] would leave exit branching past the real epilogue
into the padding.
Reported-by: [email protected]
Closes:
https://lore.kernel.org/bpf/[email protected]/T/#mfcb23909d977b949727cca4f59ee56a13fd69b92
Fixes: d243b62b7bd3 ("powerpc64/bpf: Add support for bpf trampolines")
Cc: [email protected]
Signed-off-by: Abhishek Dubey <[email protected]>
---
arch/powerpc/net/bpf_jit_comp.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/net/bpf_jit_comp.c b/arch/powerpc/net/bpf_jit_comp.c
index 1c274df2b4f7..d48bc722d0dc 100644
--- a/arch/powerpc/net/bpf_jit_comp.c
+++ b/arch/powerpc/net/bpf_jit_comp.c
@@ -128,11 +128,10 @@ void bpf_jit_build_fentry_stubs(u32 *image, u32 *fimage,
struct codegen_context
int bpf_jit_emit_exit_insn(u32 *image, u32 *fimage, struct codegen_context
*ctx,
int tmp_reg, long
exit_addr)
{
- if (!exit_addr || is_offset_in_branch_range(exit_addr - (ctx->idx *
4))) {
+ if (exit_addr && is_offset_in_branch_range(exit_addr - (long)(ctx->idx
* 4))) {
PPC_JMP(exit_addr);
- } else if (ctx->alt_exit_addr) {
- if (WARN_ON(!is_offset_in_branch_range((long)ctx->alt_exit_addr -
(ctx->idx * 4))))
- return -1;
+ } else if (ctx->alt_exit_addr && is_offset_in_branch_range(
+ (long)(ctx->alt_exit_addr) - (long)(ctx->idx * 4))) {
PPC_JMP(ctx->alt_exit_addr);
} else {
ctx->alt_exit_addr = ctx->idx * 4;
@@ -303,6 +302,7 @@ struct bpf_prog *bpf_int_jit_compile(struct
bpf_verifier_env *env, struct bpf_pr
*/
if (cgctx.seen & SEEN_TAILCALL ||
!is_offset_in_branch_range((long)cgctx.idx * 4)) {
cgctx.idx = 0;
+ cgctx.alt_exit_addr = 0;
if (bpf_jit_build_body(fp, NULL, NULL, &cgctx, addrs, 0, false))
goto out_err;
}
@@ -347,6 +347,7 @@ struct bpf_prog *bpf_int_jit_compile(struct
bpf_verifier_env *env, struct bpf_pr
bpf_jit_binary_pack_free(fhdr, hdr);
goto out_err;
}
+ addrs[fp->len] = cgctx.idx * 4;
Something like the below is needed to fix the potential branching issues
due to code shrinkage, reported by sashiko-bot in the other thread:
diff --git a/arch/powerpc/net/bpf_jit.h b/arch/powerpc/net/bpf_jit.h
index af510da12d8e..09cb7128c46b 100644
--- a/arch/powerpc/net/bpf_jit.h
+++ b/arch/powerpc/net/bpf_jit.h
@@ -14,6 +14,13 @@
#include <asm/ppc-opcode.h>
#include <linux/build_bug.h>
+/*
+ * We need at least 2 passes for proper code generation, and may need
+ * additional passes if code size changes between passes.
+ */
+#define CODEGEN_MIN_PASSES 2
+#define CODEGEN_MAX_PASSES 3
+
#ifdef CONFIG_PPC64_ELF_ABI_V1
#define FUNCTION_DESCR_SIZE 24
#else
diff --git a/arch/powerpc/net/bpf_jit_comp.c
b/arch/powerpc/net/bpf_jit_comp.c
index 904d5fe157bd..f8c1d1a39a90 100644
--- a/arch/powerpc/net/bpf_jit_comp.c
+++ b/arch/powerpc/net/bpf_jit_comp.c
@@ -335,8 +335,10 @@ struct bpf_prog *bpf_int_jit_compile(struct
bpf_verifier_env *env, struct bpf_pr
code_base = (u32 *)(image + FUNCTION_DESCR_SIZE);
fcode_base = (u32 *)(fimage + FUNCTION_DESCR_SIZE);
- /* Code generation passes 1-2 */
- for (pass = 1; pass < 3; pass++) {
+ /* Code generation passes 1-2+, loop until program size converges. */
+ for (pass = 1; pass <= CODEGEN_MAX_PASSES; pass++) {
+ u32 prev_proglen = proglen;
+
/* Now build the prologue, body code & epilogue for real. */
cgctx.idx = 0;
cgctx.alt_exit_addr = 0;
@@ -350,9 +352,23 @@ struct bpf_prog *bpf_int_jit_compile(struct
bpf_verifier_env *env, struct bpf_pr
addrs[fp->len] = cgctx.idx * 4;
bpf_jit_build_epilogue(code_base, fcode_base, &cgctx);
+ proglen = cgctx.idx * 4;
+
if (bpf_jit_enable > 1)
pr_info("Pass %d: shrink = %d, seen = 0x%x\n", pass,
- proglen - (cgctx.idx * 4), cgctx.seen);
+ prev_proglen - proglen, cgctx.seen);
+
+ /* Check if program size has converged, but ensure minimum
passes */
+ if (pass >= CODEGEN_MIN_PASSES && proglen == prev_proglen)
+ break;
+
+ if (pass == CODEGEN_MAX_PASSES && proglen != prev_proglen) {
+ pr_err("BPF JIT: Program did not converge after %d
passes\n",
+ CODEGEN_MAX_PASSES);
+ bpf_arch_text_copy(&fhdr->size, &hdr->size,
sizeof(hdr->size));
+ bpf_jit_binary_pack_free(fhdr, hdr);
+ goto out_err;
+ }
}
if (bpf_jit_enable > 1)
- Hari
