Re: [PATCH v13 4/8] powerpc/bpf: fix alignment of long branch trampoline address

On 08/09/26 1:53 pm, Hari Bathini wrote:

On 31/08/26 1:42 pm, bot+bpf-ci@kernel.org wrote:

quoted

quoted

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

[ ... ]

quoted

@@ -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 <hbathini@linux.ibm.com>