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) <[email protected]>
> ---
> 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.
> [...]
