Re: [PATCH bpf-next] m68k, bpf: Add initial BPF JIT compiler support

On Wed, Apr 29, 2026 at 01:13:19PM +0000, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [Critical] The BPF JIT compiler for m68k suffers from a 16-bit integer overflow when emitting memory instructions for 64-bit values (BPF_DW), resulting in an out-of-bounds read or write by approximately -64KB.
> - [High] BPF-to-BPF function calls will crash the kernel because `emit_call` hardcodes `extra_pass` to `false`.
> --
> 
> commit 700ecd8180ff5bd538d3f6e6f0d7655894abb49b
> Author: Kuan-Wei Chiu <[email protected]>
> 
> m68k, bpf: Add initial BPF JIT compiler support
> 
> Add a BPF JIT compiler for the m68k architecture. Generating m68k machine
> code targeting m68020+ processors.
> 
> > --- /dev/null
> > +++ b/arch/m68k/net/bpf_jit_comp.c
> [ ... ]
> > +static void emit_ldx(const struct bpf_insn *insn, struct jit_ctx *ctx)
> > +{
> [ ... ]
> > +	switch (BPF_SIZE(insn->code)) {
> [ ... ]
> > +	case BPF_DW:
> > +		emit_16(ctx, 0x2028 | (d_hi << 9));		/* move.l d16(%a0), d_hi */
> > +		emit_16(ctx, insn->off);
> > +		emit_16(ctx, 0x2028 | (d_lo << 9));		/* move.l d16+4(%a0), d_lo */
> > +		emit_16(ctx, insn->off + 4);
> > +		break;
> > +	}
> 
> Could adding 4 to insn->off cause a 16-bit signed integer overflow here?
> 
> The BPF instruction offset (insn->off) is a signed 16-bit integer that can
> reach up to 32767 for large map accesses. If insn->off is between 32764 and
> 32767, insn->off + 4 evaluates to a value exceeding the 16-bit signed maximum
> (e.g., 32768, which becomes 0x8000).
> 
> Because m68k hardware sign-extends 16-bit displacements, would emitting
> 0x8000 result in a negative 32-bit displacement (-32768), leading to an
> out-of-bounds memory access roughly 64KB behind the intended safe offset?
> 
> This same pattern appears to be present in emit_stx() and emit_st() for the
> BPF_DW cases as well.

Ack.
Will fix this by emitting a lea 4(%a0), %a0 instruction before the
second memory access.

> 
> [ ... ]
> > +static int emit_call(const struct bpf_insn *insn, struct jit_ctx *ctx)
> > +{
> > +	const s8 arg_regs[] = { BPF_REG_5, BPF_REG_4, BPF_REG_3, BPF_REG_2, BPF_REG_1 };
> > +	const s8 *tmp1 = bpf2m68k[TMP_REG_1];
> > +	u64 func_addr;
> > +	bool fixed;
> > +	int i, err;
> > +
> > +	err = bpf_jit_get_func_addr(ctx->prog, insn, false, &func_addr, &fixed);
> 
> Will this cause the JIT to emit jumps to address 0x0 for BPF-to-BPF function
> calls?
> 
> Calling bpf_jit_get_func_addr() with extra_pass hardcoded to false causes it
> to return a NULL function address for BPF-to-BPF pseudo-calls. Since the real
> addresses are not resolved, does this result in the JIT unconditionally
> emitting jsr (0), which would crash the kernel when executed?

Ack. Will fix this by passing ctx->target != NULL as the argument to
bpf_jit_get_func_addr().

Regards,
Kuan-Wei

> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1