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

[sashiko-bot@kernel.org]

Sashiko AI review found 4 potential issue(s):
- [Medium] Logic error in `asan_validate_addr` fails to validate the ASAN violation address.
- [Medium] `print_asan_map_state` missing `__noasan` attribute triggers secondary ASAN violations.
- [Low] Missing header includes in `userspace.h` and `st_asan_common.h` cause compilation errors.
- [Low] Standalone builds in the `libarena/` directory fail unconditionally on older Clang versions.
--

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

selftests/bpf: Add ASAN support for libarena selftests

Expand the arena library selftest infrastructure to support
address sanitization. Add the compiler flags necessary to
compile the library under ASAN when supported.

> diff --git a/tools/testing/selftests/bpf/libarena/Makefile b/tools/testing/selftests/bpf/libarena/Makefile
> index e85b3ad96890..5e2ab514805e 100644
> --- a/tools/testing/selftests/bpf/libarena/Makefile
> +++ b/tools/testing/selftests/bpf/libarena/Makefile
[ ... ]
> @@ -53,17 +61,32 @@ CFLAGS += $(INCLUDES)
>  vpath %.bpf.c $(LIBARENA)/src $(LIBARENA)/selftests
>  vpath %.c $(LIBARENA)/src $(LIBARENA)/selftests
>  
> +skeletons: libarena.skel.h libarena_asan.skel.h
> +.PHONY: skeletons

Will this unconditionally build libarena_asan.skel.h when running make
directly in the libarena/ directory? The parent BPF Makefile guards ASAN
compilation with CLANG_HAS_ARENA_ASAN, but that guard is missing here. Could
this cause the build to fail on older Clang versions?

[ ... ]
> diff --git a/tools/testing/selftests/bpf/libarena/include/libarena/userspace.h b/tools/testing/selftests/bpf/libarena/include/libarena/userspace.h
> index 0438a751d5fd..88b68ac73cca 100644
> --- a/tools/testing/selftests/bpf/libarena/include/libarena/userspace.h
> +++ b/tools/testing/selftests/bpf/libarena/include/libarena/userspace.h
[ ... ]
> @@ -97,3 +102,31 @@ static inline int libarena_get_globals_pages(int arena_get_globals_fd,
>  	free(vec);
>  	return 0;
>  }
> +
> +static inline int libarena_asan_init(int arena_asan_init_fd,
> +				     int asan_init_fd,
> +				     size_t arena_all_pages)
> +{
> +	LIBBPF_OPTS(bpf_test_run_opts, opts);
> +	struct asan_init_args args;

This isn't a bug, but struct asan_init_args is instantiated here without
including <libarena/asan.h>. Could this cause compilation errors if the
header is included independently?

[ ... ]
> 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 000000000000..1d3edc4372ac
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/libarena/selftests/st_asan_common.h
> @@ -0,0 +1,52 @@
> +// SPDX-License-Identifier: LGPL-2.1 OR BSD-2-Clause
> +/* Copyright (c) 2026 Meta Platforms, Inc. and affiliates. */
> +
> +#pragma once
> +

Similar to the above, this header uses macros and variables like
asan_violated, ASAN_GRANULE, and arena_stdout but does not include
<libarena/common.h> or <libarena/asan.h>.

> +#define ST_PAGES 64
> +
> +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");
> +}

Since addr is an address that triggered an ASAN violation, does this need
the __noasan attribute? Otherwise, the memory read *(s8a *)(addr) will trigger
another ASAN violation.

> +
> +/*
> + * 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) {

If a violation is expected when cond is true, does this code also need to
check if asan_violated == (u64)addr? It looks like it only verifies that
any violation occurred, which could allow tests to erroneously pass if a
completely unrelated ASAN violation happens.

> +		asan_violated = 0;
> +		return 0;
> +	}

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