Re: [PATCH bpf-next v2 4/5] selftests/bpf: Fix kprobe_multi_bench_attach test failure with LTO kernel

[Jiri Olsa <olsajiri@gmail.com>]

On Thu, Mar 21, 2024 at 01:01:19PM -0700, Yonghong Song wrote:
> In my locally build clang LTO kernel (enabling CONFIG_LTO and
> CONFIG_LTO_CLANG_THIN), kprobe_multi_bench_attach/kernel subtest
> failed like:
>   test_kprobe_multi_bench_attach:PASS:get_syms 0 nsec
>   test_kprobe_multi_bench_attach:PASS:kprobe_multi_empty__open_and_load 0 nsec
>   libbpf: prog 'test_kprobe_empty': failed to attach: No such process
>   test_kprobe_multi_bench_attach:FAIL:bpf_program__attach_kprobe_multi_opts unexpected error: -3
>   #117/1   kprobe_multi_bench_attach/kernel:FAIL
> 
> There are multiple symbols in /sys/kernel/debug/tracing/available_filter_functions
> are renamed in /proc/kallsyms due to cross file inlining. One example is for
>   static function __access_remote_vm in mm/memory.c.
> In a non-LTO kernel, we have the following call stack:
>   ptrace_access_vm (global, kernel/ptrace.c)
>     access_remote_vm (global, mm/memory.c)
>       __access_remote_vm (static, mm/memory.c)
> 
> With LTO kernel, it is possible that access_remote_vm() is inlined by
> ptrace_access_vm(). So we end up with the following call stack:
>   ptrace_access_vm (global, kernel/ptrace.c)
>     __access_remote_vm (static, mm/memory.c)
> The compiler renames __access_remote_vm to __access_remote_vm.llvm.<hash>
> to prevent potential name collision.
> 
> The kernel bpf_kprobe_multi_link_attach() and ftrace_lookup_symbols() try
> to find addresses based on /proc/kallsyms, hence the current test failed
> with LTO kenrel.
> 
> This patch removed __access_remote_vm and other similar functions from
> kprobe_multi_attach by checking if the symbol like __access_remote_vm
> does not exist in kallsyms with LTO kernel. The test succeeded after this change:
>   #117/1   kprobe_multi_bench_attach/kernel:OK
> 
> Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
> ---
>  .../selftests/bpf/prog_tests/kprobe_multi_test.c     | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c b/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
> index 05000810e28e..f6130f4f3d88 100644
> --- a/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
> +++ b/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
> @@ -345,6 +345,9 @@ static int get_syms(char ***symsp, size_t *cntp, bool kernel)
>  	FILE *f;
>  	int err = 0;
>  
> +	if (!ASSERT_OK(load_kallsyms(), "load_kallsyms"))
> +		return -EINVAL;
> +
>  	/*
>  	 * The available_filter_functions contains many duplicates,
>  	 * but other than that all symbols are usable in kprobe multi
> @@ -393,6 +396,15 @@ static int get_syms(char ***symsp, size_t *cntp, bool kernel)
>  		if (!strncmp(name, "__ftrace_invalid_address__",
>  			     sizeof("__ftrace_invalid_address__") - 1))
>  			continue;
> +		/*
> +		 * In certain cases, e.g., clang lto kernel, the 'name' here
> +		 * may be different from the one in /proc/kallsyms due to
> +		 * /proc/kallsyms name might be "<name>.llvm.<hash>" instead
> +		 * of "<name>". Exclude these 'name's since they will cause
> +		 * later kprobe_multi_attach failure.
> +		 */
> +		if (ksym_get_addr(name) == 0)
> +			continue;

curious how many symbols like that are there?

Acked-by: Jiri Olsa <jolsa@kernel.org>

jirka

>  
>  		err = hashmap__add(map, name, 0);
>  		if (err == -EEXIST) {
> -- 
> 2.43.0
> 
>