Re: [PATCH bpf-next v5 04/10] bpf, x86: add helper to emit kasan checks in x86 JITed programs

[Ihor Solodrai <ihor.solodrai@linux.dev>]

On 7/9/26 3:01 AM, Alexis Lothoré (eBPF Foundation) wrote:
> Add the emit_kasan_check() function that emits KASAN shadow memory
> checks before memory accesses in JIT-compiled BPF programs. The
> implementation relies on the existing __asan_{load,store}X functions
> from KASAN subsystem. The helper:
> - ensures that the kasan instrumention is actually needed: if the
>   instruction being processed accesses the program stack, we skip the
>   instrumentation, as those accesses are already protected with page
>   guards
> - saves registers. This includes caller-saved registers, but also
>   temporary registers, as those were possibly used by the
>   affected program. Theoretically, r10 and r11 should be saved as well,
>   but the number of called function and their scope being limited, they
>   are skipped for the sake of reducing the overhead

My clanker got very excited about r10 and r11 not being saved, just
like sashiko. So I looked at this a bit closer. TL;DR it's fine

I built the kernel with gcc 11.5, gcc 15.2 and clang 22 and
disassembled __asan_{load,store}{1,2,4,8}: none of them touch r10 or
r11. gcc goes up to r8, clang uses nothing above rdi.

However, kasan_check_range() does use r10/r11.  And looks like we are
ok only because the fixed-size helpers inline the shadow check and
never call kasan_check_range().

If any of the fixed-size 1/2/4/8 helpers ever actually call
kasan_check_range(), r10 may get clobbered.

Maybe we should leave a comment on emit_kasan_check() about this?..

> - computes the accessed address and stores it in %rdi
> - calls the relevant function, depending on the instruction being a load
>   or a store, and the size of the access.
> - restores registers
> 
> The special care needed when inserting this instrumentation comes at the
> cost of a non negligeable increase in JITed code size. For example, a
> bare
> 
>   mov 	0x0(%si),rbx # Load in rbx content at address stored in rsi
> 
> becomes
> 
>   push    %rax
>   push    %rcx
>   push    %rdx
>   push    %rsi
>   push    %rdi
>   push    %r8
>   push    %r9
>   mov     %rsi,%rdi
>   call    0xffffffff81da0a60 <__asan_load8>
>   pop     %r9
>   pop     %r8
>   pop     %rdi
>   pop     %rsi
>   pop     %rdx
>   pop     %rcx
>   pop     %rax
>   mov     0x0(%rsi),rbx
> 
> Signed-off-by: Alexis Lothoré (eBPF Foundation) <alexis.lothore@bootlin.com>
> ---
> Changes in v3:
> - skip kasan instrumentation if there is no verifier env (cBPF)
> - move helper up in the file
> 
> Changes in v2:
> - move asan functions declaration directly into jit compiler, and guard
>   them with IS_ENABLED
> - remove faulty stack alignment, no arg is passed to kasan funcs on the
>   stack anyway
> - make sure to emit call depth accounting code
> - do not save unneeded registers
> - update helper signature to let caller configure some values (eg:
>   is_write)
> ---
>  arch/x86/net/bpf_jit_comp.c | 95 +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 95 insertions(+)
> 
> diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
> index de7515ea1bea..d35f58350d71 100644
> --- a/arch/x86/net/bpf_jit_comp.c
> +++ b/arch/x86/net/bpf_jit_comp.c
> @@ -21,6 +21,17 @@
>  #include <asm/unwind.h>
>  #include <asm/cfi.h>
>  
> +#if IS_ENABLED(CONFIG_BPF_JIT_KASAN)
> +void __asan_load1(void *p);
> +void __asan_store1(void *p);
> +void __asan_load2(void *p);
> +void __asan_store2(void *p);
> +void __asan_load4(void *p);
> +void __asan_store4(void *p);
> +void __asan_load8(void *p);
> +void __asan_store8(void *p);
> +#endif
> +
>  static bool all_callee_regs_used[4] = {true, true, true, true};
>  
>  static u8 *emit_code(u8 *ptr, u32 bytes, unsigned int len)
> @@ -1110,6 +1121,90 @@ static void maybe_emit_1mod(u8 **pprog, u32 reg, bool is64)
>  	*pprog = prog;
>  }
>  
> +static int emit_kasan_check(struct bpf_verifier_env *env, u8 **pprog,
> +			    u32 addr_reg, struct bpf_insn *insn, u8 *ip,
> +			    bool is_write, bool accesses_stack_only)


nit: IMO it's not nice to introduce functions with no caller as a
separate patch: it triggers -Wunused-function on this commit, which
may or may not be used in a build. I'd fold this in patch #6 where the
function is actually called.

> [...]