Re: [PATCH bpf-next v8 4/8] selftests/bpf: Add arena ASAN runtime to libarena

[sashiko-bot@kernel.org]

Sashiko AI review found 5 potential issue(s):
- [High] Silent failure and incomplete shadow map updates in `asan_memset` and `first_nonzero_byte` due to BPF loop limits.
- [High] `__asan_memcpy`, `__asan_memmove`, and `__asan_memset` stubs silently drop memory operations, causing uninitialized data usage.
- [High] Concurrent execution of `asan_init` corrupts the shadow map base address due to a TOCTOU race condition.
- [High] ASAN memory access hooks lack runtime initialization checks, leading to false positives and invalid memory access.
- [Medium] Missing BTF anchors for variable-sized and intrinsic ASAN memory stubs.
--

commit b77770f7adb86bf4a4f23e524cb1dfc5e9e8a79e
Author: Emil Tsalapatis <emil@etsalapatis.com>

selftests/bpf: Add arena ASAN runtime to libarena

This adds an address sanitizer (ASAN) runtime to the arena library.
The ASAN runtime implements functions injected into BPF binaries by LLVM
sanitization when ASAN is enabled.

> diff --git a/tools/testing/selftests/bpf/libarena/include/asan.h b/tools/testing/selftests/bpf/libarena/include/asan.h
> new file mode 100644
> index 0000000000000..eb9fc69d9eb04
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/libarena/include/asan.h
[ ... ]
> +/*
> + * Force LLVM to emit BTF information for the stubs,
> + * because the ASAN pass in LLVM by itself doesn't.
> + */
> +#define ASAN_LOAD_STORE_SIZE(size)		\
> +	__asan_store##size,			\
> +	__asan_store##size##_noabort,		\
> +	__asan_load##size,			\
> +	__asan_load##size##_noabort,		\
> +	__asan_report_store##size,		\
> +	__asan_report_store##size##_noabort,	\
> +	__asan_report_load##size,		\
> +	__asan_report_load##size##_noabort
> +
> +__attribute__((used))
> +static void (*__asan_btf_anchors[])(intptr_t) = {
> +	ASAN_LOAD_STORE_SIZE(1),
> +	ASAN_LOAD_STORE_SIZE(2),
> +	ASAN_LOAD_STORE_SIZE(4),
> +	ASAN_LOAD_STORE_SIZE(8),
> +};

Are there missing anchors for __asan_loadN, __asan_storeN, __asan_memcpy,
__asan_memmove, and __asan_memset here? The LLVM ASAN pass can inject
calls to these for variable-sized or intrinsic struct accesses, which
could lead to BPF linking failures if their BTF information is stripped.

> diff --git a/tools/testing/selftests/bpf/libarena/src/asan.bpf.c b/tools/testing/selftests/bpf/libarena/src/asan.bpf.c
> new file mode 100644
> index 0000000000000..7e0aaab3803f9
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/libarena/src/asan.bpf.c
[ ... ]
> +__weak int asan_memset(s8a __arg_arena *dst, s8 val, size_t size)
> +{
> +	size_t i;
> +
> +	for (i = zero; i < size && can_loop; i++)
> +		dst[i] = val;
> +
> +	return 0;
> +}

Does this silently leave the remainder of the shadow memory unpoisoned if
the loop terminates early due to the can_loop BPF runtime limit? If a
large allocation causes this to hit the budget, returning 0 might cause
callers like asan_unpoison to proceed, leading to false positives on
subsequent accesses to the remainder.

[ ... ]
> +static __always_inline u64 first_nonzero_byte(u64 addr, size_t size)
> +{
> +	while (size && can_loop) {
> +		if (unlikely(*(s8a *)addr))
> +			return addr;
> +		addr += 1;
> +		size -= 1;
> +	}
> +
> +	return SHADOW_ALL_ZEROES;
> +}

Similarly, could this return SHADOW_ALL_ZEROES inappropriately if the loop
terminates early from the can_loop limit? This might mask use-after-free
bugs if the scan is incomplete.

[ ... ]
> +static __always_inline bool check_region_inline(intptr_t ptr, size_t size,
> +						u32 flags)
> +{
> +	s8a *addr = (s8a *)(u64)ptr;
> +	bool is_poisoned, is_valid;

Should this function check if the ASAN runtime has been initialized,
perhaps by calling asan_ready()? If a BPF program executes before
userspace invokes asan_init, __asan_shadow_memory_dynamic_address would
remain 0. This could cause the checks to read from offset 0 in user memory
instead of the shadow map, leading to unpredictable false positives.

[ ... ]
> +/*
> + * We do not currently have memcpy/memmove/memset intrinsics
> + * in LLVM. Do not implement sanitization.
> + */
> +void *__asan_memcpy(void *d, const void *s, size_t n)
> +{
> +	arena_stderr("ASAN: Unexpected %s call", __func__);
> +	return NULL;
> +}
> +
> +void *__asan_memmove(void *d, const void *s, size_t n)
> +{
> +	arena_stderr("ASAN: Unexpected %s call", __func__);
> +	return NULL;
> +}
> +
> +void *__asan_memset(void *p, int c, size_t n)
> +{
> +	arena_stderr("ASAN: Unexpected %s call", __func__);
> +	return NULL;
> +}

If the LLVM ASAN pass intercepts intrinsic memory operations and replaces
them with these external calls, will the actual memory operations be
completely dropped? Returning NULL here without performing the copy or set
might leave the destination structs uninitialized, which could corrupt
application data.

[ ... ]
> +SEC("syscall")
> +__weak __noasan int asan_init(struct asan_init_args *args)
> +{
> +	u64 globals_pages = args->arena_globals_pages;
> +	u64 all_pages = args->arena_all_pages;
> +	u64 shadow_map, shadow_pgoff;
> +	u64 shadow_pages;
> +
> +	if (asan_inited)
> +		return 0;
[ ... ]
> +	shadow_map = (u64)bpf_arena_alloc_pages(
> +		&arena, (void __arena *)__asan_shadow_memory_dynamic_address,
> +		shadow_pages, NUMA_NO_NODE, 0);
> +	if (!shadow_map) {
> +		arena_stderr("Could not allocate shadow map\n");
> +
> +		__asan_shadow_memory_dynamic_address = 0;
> +
> +		return -ENOMEM;
> +	}
> +
> +	asan_inited = true;
> +
> +	return 0;
> +}

Is there a TOCTOU race condition here that could corrupt the shadow map
base address?

If two userspace threads bypass the if (asan_inited) check simultaneously,
both could attempt bpf_arena_alloc_pages() at the same target address. The
second thread's allocation would fail, taking the error path and setting
__asan_shadow_memory_dynamic_address to 0, while the first thread might
have already set asan_inited to true.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260421165037.4736-1-emil@etsalapatis.com?part=4