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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] m68k/bpf: Hardcoded prologue size in tail call causes offset mismatch on ColdFire
- [High] m68k/bpf: Silent JIT failure in extra_pass allows execution of incompletely JITed programs
--

commit 2847ebf4b1948509dbfb51929065f4ab0a56fcf6
Author: Kuan-Wei Chiu <visitorckw@gmail.com>

m68k, bpf: Add initial BPF JIT compiler support

This commit adds a BPF JIT compiler for the m68k architecture. It generates
machine code targeting m68020+ and coldfire processors with an MMU, while
excluding plain 68000/68010 and non-MMU coldfire processors.

> diff --git a/arch/m68k/net/bpf_jit_comp.c b/arch/m68k/net/bpf_jit_comp.c
> new file mode 100644
> --- /dev/null
> +++ b/arch/m68k/net/bpf_jit_comp.c
[ ... ]
> +static void emit_tail_call(const struct bpf_insn *insn, struct jit_ctx *ctx)
> +{
[ ... ]
> +	emit_16(ctx, 0x2068);					/* movea.l d16(%a0), %a0 */
> +	emit_16(ctx, offsetof(struct bpf_prog, bpf_func));
> +
> +	emit_16(ctx, 0x4ee8);					/* jmp d16(%a0) */
> +	emit_16(ctx, 44);

[Severity: High]
Does this hardcoded displacement of 44 bytes account for the ColdFire prologue?

In build_prologue(), the ColdFire prologue takes 48 bytes due to an expanded
register saving sequence:

	if (CPU_IS_COLDFIRE) {
		emit_16(ctx, 0x4fef);				/* lea -16(%sp), %sp */
		emit_16(ctx, 0xfff0);
		emit_16(ctx, 0x48d7);				/* movem.l d2-d5, (%sp) */
		emit_16(ctx, 0x003c);
	} else {

If a BPF program executes a tail call on a ColdFire processor, the tail call
will jump to byte 44, unintentionally executing the last 4 bytes of the
prologue. This writes an uncontrolled %d0 value into BPF_R10_HI, corrupting
it.

Since BPF programs can be loaded by unprivileged users, can this bypass
verifier bounds tracking and lead to arbitrary memory access if the program
performs 64-bit pointer arithmetic on R10?

[ ... ]
> +struct bpf_prog *bpf_int_jit_compile(struct bpf_verifier_env *env, struct bpf_prog *prog)
> +{
> +	struct bpf_binary_header *header;
> +	struct jit_ctx ctx;
> +	unsigned int image_size;
> +	u8 *image_ptr;
> +	bool extra_pass = prog->jited;
> +
> +	if (!prog->jit_requested)
> +		return prog;
> +
> +	if (prog->jited && !prog->aux->func)
> +		return prog;
> +
> +	memset(&ctx, 0, sizeof(ctx));
> +	ctx.prog = prog;
> +
> +	ctx.offsets = kvcalloc(prog->len + 1, sizeof(int), GFP_KERNEL);
> +	if (!ctx.offsets)
> +		return prog;

[Severity: High]
Can a failure to allocate ctx.offsets during an extra_pass lead to the
execution of an incompletely JITed program?

If kvcalloc fails during the extra_pass in bpf_int_jit_compile(), it returns
prog directly without clearing prog->bpf_func. Because prog->bpf_func remains
unchanged from the first pass, jit_subprogs() assumes the JIT compilation
succeeded. 

The BPF program could then be executed with a JIT image containing unresolved
subprogram call addresses, which resolve to a 0 address, resulting in a kernel
panic when executed.

Since unprivileged users can trigger this by loading BPF programs during
memory pressure, can this cause a denial of service?

> +
> +	build_prologue(&ctx);
> +	if (build_body(&ctx) < 0)
> +		goto out_off;

[Severity: High]
Similarly, if the dry run of build_body() fails, it jumps to out_off which
frees ctx.offsets and returns prog without clearing the BPF function pointer.
Can this also hide the failure from jit_subprogs() when loading a BPF program
with subprograms?

> +	build_epilogue(&ctx);
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260927205501.672807-1-visitorckw@gmail.com?part=1