Re: [PATCH RFC bpf-next 5/8] bpf, x86: emit KASAN checks into x86 JITed programs

Alexis Lothoré
2026-04-29T21:59:43Z

On Sat Apr 25, 2026 at 8:08 AM CEST, sashiko-bot wrote:
> Sashiko AI review found 3 potential issue(s):
> - [High] Missing KASAN instrumentation for immediate-based memory stores (BPF_ST | BPF_MEM) and incorrect write detection.
> - [High] Missing KASAN instrumentation for standard atomic memory operations (BPF_STX | BPF_ATOMIC).
> - [High] Incorrect IP calculation in emit_kasan_check() leads to wrong jump offsets and kernel crashes when IBT is enabled.
> --
>
> commit bdea14177f5d099598de93e92ea9aecf3714ba64
> Author: Alexis Lothoré (eBPF Foundation) <alexis.lothore@bootlin.com>
>
> bpf, x86: emit KASAN checks into x86 JITed programs
>
> This commit inserts KASAN shadow memory checks before load and store
> operations in JIT-compiled BPF programs. It aims to detect memory safety bugs
> while explicitly skipping stack accesses and faulting PROBE instructions.
>
>> diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
>> index b90103bd00809..111fe1d551219 100644
>> --- a/arch/x86/net/bpf_jit_comp.c
>> +++ b/arch/x86/net/bpf_jit_comp.c
> [ ... ]
>> @@ -2242,6 +2244,11 @@ st: if (is_imm8(insn->off))
>> case BPF_STX | BPF_MEM | BPF_H:
>> case BPF_STX | BPF_MEM | BPF_W:
>> case BPF_STX | BPF_MEM | BPF_DW:
>> + err = emit_kasan_check(&prog, dst_reg, insn,
>> + image + addrs[i - 1],
>> + accesses_stack);
>> + if (err)
>> + return err;
>> emit_stx(&prog, BPF_SIZE(insn->code), dst_reg, src_reg, insn->off);
>> break;
>
> Is the correct instruction pointer being passed to emit_kasan_check() here?
>
> Earlier in do_jit(), the local ip variable is updated to account for
> conditionally emitted prefixes:
>
> if (bpf_insn_is_indirect_target(env, bpf_prog, i - 1))
> EMIT_ENDBR();
>
> ip = image + addrs[i - 1] + (prog - temp);
>
> Passing image + addrs[i - 1] directly ignores the size of the ENDBR prefix.
> Could this cause the relative jump offset in emit_call() to be off by 4
> bytes, leading to a kernel crash when branching to the KASAN function?

I have CONFIG_X86_KERNEL_IBT=y in my testing kernel, so if that's indeed
a valid scenario, I may have been lucky (or rather, unlucky) to not see the
corresponding crash while running the full selftests set. I'll dig into
that.

> Additionally, looking at the code surrounding this hunk:
>
> /* ST: *(u8*)(dst_reg + off) = imm */
> case BPF_ST | BPF_MEM | BPF_B:
> if (is_ereg(dst_reg))
> EMIT2(0x41, 0xC6);
> else
> EMIT1(0xC6);
> goto st;
> [ ... ]
> case BPF_ST | BPF_MEM | BPF_DW:
> EMIT2(add_1mod(0x48, dst_reg), 0xC7);
>
> st: if (is_imm8(insn->off))
> EMIT2(add_1reg(0x40, dst_reg), insn->off);
> else
> EMIT1_off32(add_1reg(0x80, dst_reg), insn->off);
>
> EMIT(imm32, bpf_size_to_x86_bytes(BPF_SIZE(insn->code)));
> break;
>
> Are immediate-based memory stores (BPF_ST | BPF_MEM) meant to be skipped?

As mentioned in the previous comments, BPF_ST/BPF_LD are indeed meant to
be skipped.

> Unlike BPF_STX, there is no emit_kasan_check() added here, which seems to
> allow uninstrumented writes to kernel memory.
>
> If instrumentation is added here, there might be a second issue. Looking at
> the helper:
>
> static int emit_kasan_check(..., struct bpf_insn *insn, ...)
> {
> bool is_write = BPF_CLASS(insn->code) == BPF_STX;
> ...
> }
>
> Since BPF_ST has a different class, would this evaluate to false and
> incorrectly emit a read check instead of a write check?
>
> [ ... ]
>> @@ -2390,6 +2397,12 @@ st: if (is_imm8(insn->off))
>> /* populate jmp_offset for JAE above to jump to start_of_ldx */
>> start_of_ldx = prog;
>> end_of_jmp[-1] = start_of_ldx - end_of_jmp;
>> + } else {
>> + err = emit_kasan_check(&prog, src_reg, insn,
>> + image + addrs[i - 1],
>> + accesses_stack);
>> + if (err)
>> + return err;
>> }
>> if (BPF_MODE(insn->code) == BPF_PROBE_MEMSX ||
>> BPF_MODE(insn->code) == BPF_MEMSX)
>
> Further down in the function, atomic operations are handled:
>
> case BPF_STX | BPF_ATOMIC | BPF_W:
> case BPF_STX | BPF_ATOMIC | BPF_DW:
> if (insn->imm == (BPF_AND | BPF_FETCH) ||
> insn->imm == (BPF_OR | BPF_FETCH) ||
> insn->imm == (BPF_XOR | BPF_FETCH)) {
> bool is64 = BPF_SIZE(insn->code) == BPF_DW;
>
> Should standard atomic memory operations be instrumented as well?
>
> While the commit message notes that BPF_PROBE_ATOMIC instructions are skipped
> because they handle faults, standard BPF_STX | BPF_ATOMIC operations perform
> memory modifications without fault-handling semantics.
>
> Since emit_kasan_check() is omitted here, could an invalid base pointer used
> in an atomic operation bypass KASAN and corrupt memory?

Atomics were skipped for the RFC, but that's a point I want to tackle
in the next revision.

--
Alexis Lothoré, Bootlin
Embedded Linux and Kernel engineering