Re: [PATCH v12 2/8] powerpc/bpf: fix alignment of long branch trampoline address
From: bot+bpf-ci@kernel.org
Date: 2026-08-18 05:45:27
Also in:
bpf, linux-kselftest, stable
Ensure the dummy trampoline address field present between the OOL stub and the long branch stub is 8-byte aligned, for memory compatibility when content loaded to a register.
This isn't a bug, but could the changelog name the concrete failure mode? The actual mechanism is that bpf_arch_text_poke() patches this field via patch_ulong() which returns -EINVAL unless IS_ALIGNED(addr, sizeof(unsigned long)), so attaching a bpf trampoline whose target is out of branch range fails when the field lands at a 4-mod-8 address. The phrase "for memory compatibility when content loaded to a register" is vague - powerpc 'ld' does not require 8-byte alignment; the alignment requirement comes from patch_ulong().
quoted hunk ↗ jump to hunk
diff --git a/arch/powerpc/net/bpf_jit_comp.c b/arch/powerpc/net/bpf_jit_comp.c index 8bee2674d4a9..caea53cde063 100644 --- a/arch/powerpc/net/bpf_jit_comp.c +++ b/arch/powerpc/net/bpf_jit_comp.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -49,11 +49,39 @@ 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; /* + * In the final pass, 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. + * + * dummy_tramp_addr must be 8-byte aligned for load-register + * compatibility. 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 (ctx->idx + ool_stub_sz) is odd, so that + * dummy_tramp_addr lands at an even instruction offset (== 8-byte + * aligned from an 8-byte aligned base). + * + * 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());
Does the new comment block accurately describe the code? Looking at the
comment and the implementation:
(1) Unit mismatch: The comment says "Emit a NOP here if (ctx->idx +
ool_stub_sz) is odd" but ctx->idx is an instruction index while ool_stub_sz
is a byte count (16 or 12). The code actually tests:
IS_ALIGNED((unsigned long)fimage + ctx->idx*4 + ool_stub_sz, SZL)
which properly multiplies ctx->idx by 4 to convert to bytes. The comment's
stated parity expression doesn't match what the code performs.
(2) Alignment assumption contradiction: The comment states dummy_tramp_addr
"lands at an even instruction offset (== 8-byte aligned from an 8-byte
aligned base)", but the preceding sentence explicitly says "The fimage can
be non 8-byte aligned". In fact, bpf_jit_binary_pack_alloc() is called
with alignment 4 in bpf_int_jit_compile(), and the code correctly folds the
real fimage address into the IS_ALIGNED() check rather than assuming an
aligned base.
(3) "In the final pass" is inaccurate: the alignment decision is taken in
every codegen pass where image != NULL. Looking at the loop in
bpf_jit_comp.c starting around bpf_jit_build_prologue(), 2-3 such passes
run, not just the final one.
Also, "the image buffer have 4 spare bytes" is ungrammatical and
understates the case: a fentry-stub block is also emitted mid-body by
bpf_jit_emit_exit_insn()'s fallback path, so a program can contain several
stub sites and therefore several spare NOP slots.
Note: the immediate follow-up commit 1525fbfebfe9 ("powerpc/bpf: Move out
dummy_tramp_addr after Long branch stub") deletes exactly this wording and
replaces the 16:12 constant with 44:40, which suggests the description was
known to be confusing.
---
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/32100929603