Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [Medium] bpf_prog_fill_jited_linfo() calculates incorrect line info addresses because ctx.offset includes the CFI prefix size. -- commit 1909b8cf9ec25c69a7f49627fb1a30b3d8e545d4 Author: Jose Fernandez (Anthropic) <[email protected]> arm64: cfi: Use CONFIG_ARM64_FUNCTION_PREFIX_NOPS for the type hash offset This patch modifies the CFI prefix to use CONFIG_ARM64_FUNCTION_PREFIX_NOPS instead of hardcoding four bytes. It adjusts cfi_get_offset(), SYM_TYPED_FUNC_START, and the BPF JIT to account for the dynamic number of NOPs required by kCFI kernels with CALL_OPS or ThinLTO and BTI. > diff --git a/arch/arm64/net/bpf_jit_comp.c b/arch/arm64/net/bpf_jit_comp.c > index c18e005a41dbe..7ad14d9847138 100644 > --- a/arch/arm64/net/bpf_jit_comp.c > +++ b/arch/arm64/net/bpf_jit_comp.c [ ... ] > @@ -185,8 +185,12 @@ static inline void emit_bti(u32 insn, struct jit_ctx > *ctx) > > static inline void emit_kcfi(u32 hash, struct jit_ctx *ctx) > { > - if (IS_ENABLED(CONFIG_CFI)) > - emit_u32_data(hash, ctx); > + if (!IS_ENABLED(CONFIG_CFI)) > + return; > + > + emit_u32_data(hash, ctx); > + for (int i = 0; i < CONFIG_ARM64_FUNCTION_PREFIX_NOPS; i++) > + emit(A64_NOP, ctx); > } [Severity: Medium] This is a pre-existing issue, but does expanding the CFI prefix length cause bpf_prog_fill_jited_linfo() to calculate incorrect line info addresses by adding the CFI prefix twice? When loading any BPF program with line info on an arm64 kernel built with kCFI, ctx.offset measures from the start of the JIT buffer, which includes the CFI prefix emitted here. However, in bpf_int_jit_compile() in arch/arm64/net/bpf_jit_comp.c, the offsets are passed to bpf_prog_fill_jited_linfo(): arch/arm64/net/bpf_jit_comp.c:bpf_int_jit_compile() { ... for (i = 0; i <= prog->len; i++) ctx.offset[i] *= AARCH64_INSN_SIZE; bpf_prog_fill_jited_linfo(prog, ctx.offset + 1); ... } Since bpf_prog_fill_jited_linfo() adds these offsets to prog->bpf_func, which is already shifted past the CFI prefix by cfi_get_offset(), the CFI prefix length ends up being added twice. The bug existed prior to this patch (off by 4 bytes) but this patch exacerbates it to 16 or 24 bytes depending on the number of NOPs. -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=5
