On 08/09/26 1:53 pm, Hari Bathini wrote:
On 31/08/26 1:42 pm, [email protected] wrote:
diff --git a/arch/powerpc/net/bpf_jit_comp.c b/arch/powerpc/net/
bpf_jit_comp.c
index 11981d2270a9d..8ca36a933c7ae 100644
--- a/arch/powerpc/net/bpf_jit_comp.c
+++ b/arch/powerpc/net/bpf_jit_comp.c
[ ... ]
@@ -49,11 +49,35 @@ asm (
" .popsection ;"
);
-void bpf_jit_build_fentry_stubs(u32 *image, struct codegen_context
*ctx)
+void bpf_jit_build_fentry_stubs(u32 *image, u32 *fimage, struct
codegen_context *ctx)
{
int ool_stub_idx, long_branch_stub_idx;
+ int ool_stub_sz;
/*
+ * Align the mis-aligned dummy_tramp_addr field in the fimage.
+ * The alignment NOP must appear before OOL stub, to make
+ * ool_stub_idx & long_branch_stub_idx constant from end.
+ *
+ * The fimage can be non 8-byte aligned, so final alignment depends
+ * on start of fimage and the stub's instruction count offset. The
+ * OOL stub size is 4 instructions (with
CONFIG_PPC_FTRACE_OUT_OF_LINE)
+ * or 3 instructions (without) before dummy_tramp_addr.
+ *
+ * Emit a NOP here if address is not SZL aligned.
+ *
+ * In pass=0 when image==NULL, conservatively account for space
+ * required to accommodate alignment NOP. In case final pass skips
+ * emitting alignment NOP, the image buffer have 4 spare bytes and
+ * jited_len signifies correct program size.
+ */
+
+ ool_stub_sz = IS_ENABLED(CONFIG_PPC_FTRACE_OUT_OF_LINE) ? 16 : 12;
+ if (!image || !IS_ALIGNED((unsigned long)fimage + ctx->idx*4 +
ool_stub_sz, SZL))
+ EMIT(PPC_RAW_NOP());
Can the conditional alignment NOP here mask program length convergence?
The preceding commit (5175364d6174 "powerpc/bpf: fix buffer overflow in
JIT for large BPF programs") added a convergence check:
if (pass >= CODEGEN_MIN_PASSES && proglen == prev_proglen)
break;
to bpf_int_jit_compile(). But the alignment decision above forces the
stub block to end at a fixed residue mod 8: the field address (fimage +
ctx->idx*4 + ool_stub_sz) is aligned to SZL, and exactly SZL/4 + 7
instructions follow it. So fimage + proglen is congruent to a constant
mod 8 every pass, meaning proglen can only change in multiples of 8.
When the body shrinks by an odd multiple of 4 bytes between passes, the
NOP can absorb it and keep proglen unchanged. The body can shrink when
an exit goes from out-of-range (emitting a full epilogue) to in-range
(emitting a single branch):
arch/powerpc/net/bpf_jit_comp.c:bpf_jit_emit_exit_insn() {
if (exit_addr && is_offset_in_branch_range(...)) {
PPC_JMP(exit_addr); // 1 instruction
} else {
...
bpf_jit_build_epilogue(image, fimage, ctx); // N instructions
}
}
The shrink is (N-1)*4 bytes. When N-1 is odd, this is 4 mod 8. The NOP
appearance or disappearance compensates, making proglen identical across
passes even though addrs[] shifted.
Because forward branches use addrs[] from the previous pass (addrs[j]
for target j > current i is computed in the prior pass), those branches
would land (N-1)*4 bytes past the intended target.
The comment at arch/powerpc/net/bpf_jit.h:132-138 documents that the
PowerPC JIT avoids pass-to-pass size changes by padding the short branch
case with a NOP, specifically to prevent this scenario. Does
reintroducing
an address-dependent, pass-varying size risk incorrect branches?
(Note: commit 0cd8bd7da278 later in this series reworks the stub layout
and is described as a layout improvement rather than a fix for this
commit,
which may provide additional context.)
The review correctly identifies a potential mechanism by which a 4-byte
alignment NOP could mask a size change. However, that mechanism only
causes a convergence failure if the total code-size reduction between
passes is exactly compensated by the NOP (i.e. effectively only 4
bytes). In this JIT, changing an exit from the inline epilogue to a
branch removes the entire epilogue, so the size reduction is much
larger than 4 bytes. The NOP can only compensate for 4 bytes; it
cannot hide the remaining reduction. Therefore the proposed
convergence failure does not apply to this code.
With the above said:
Reviewed-by: Hari Bathini <[email protected]>
