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

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


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/33368376983