Re: [PATCH bpf-next v8 5/8] selftests/bpf: Add ASAN support for libarena selftests

[sashiko-bot@kernel.org]

Sashiko AI review found 2 potential issue(s):
- [Medium] The debug function `print_asan_map_state()` unsafely dereferences a potentially poisoned pointer, triggering a secondary ASAN violation.
- [Medium] The global variable `asan_violated` is not reset on error paths, breaking test isolation and causing cascading failures.
--

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

selftests/bpf: Add ASAN support for libarena selftests

This commit expands the libarena selftest infrastructure to support address
sanitization, adding the necessary compiler flags to compile the library
under ASAN.

> diff --git a/tools/testing/selftests/bpf/libarena/selftests/st_asan_common.h b/tools/testing/selftests/bpf/libarena/selftests/st_asan_common.h
> new file mode 100644
> index 0000000000000..58b1b0ae6a778
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/libarena/selftests/st_asan_common.h

[ ... ]

> +static inline void print_asan_map_state(void __arena *addr)
> +{
> +	arena_stdout("%s:%d ASAN %p -> (val: %x gran: %x set: [%s])",
> +			__func__, __LINE__, addr,
> +			*(s8a *)(addr), ASAN_GRANULE(addr),
> +			asan_shadow_set(addr) ? "yes" : "no");
> +}

Does reading *(s8a *)(addr) risk triggering a secondary ASAN violation?

If addr points to unmapped memory outside the arena, the dereference could
trigger a BPF exception and terminate the test before the debug output is
printed. The __noasan attribute might be needed here to avoid interception
by ASAN instrumentation.

> +/*
> + * Emit an error and force the current function to exit if the ASAN
> + * violation state is unexpected. Reset the violation state after.
> + */
> +static inline int asan_validate_addr(bool cond, void __arena *addr)
> +{
> +	if ((asan_violated != 0) == cond) {
> +		asan_violated = 0;
> +		return 0;
> +	}
> +
> +	arena_stdout("%s:%d ASAN asan_violated %lx", __func__, __LINE__,
> +			(u64)asan_violated);
> +	print_asan_map_state(addr);
> +	return -EINVAL;
> +}

Should asan_violated be reset to 0 before returning -EINVAL here?

BPF global variables persist across executions on the same skeleton. If left
non-zero, subsequent tests might fail their ASAN validation checks.

> +
> +static inline int asan_validate(void)
> +{
> +	if (!asan_violated)
> +		return 0;
> +
> +	arena_stdout("%s:%d Found ASAN violation at %lx", __func__, __LINE__,
> +			asan_violated);
> +
> +	return -EINVAL;
> +}

Similarly, does asan_violated need to be reset to 0 here to prevent cascading
failures in subsequent tests?

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