Re: [PATCHv3 bpf-next 4/8] selftests/bpf: Test const kfunc memory arguments

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] selftests/bpf: kfunc_call_test_const_mem() fails due to MEM_WRITE in verifier
--

commit 0e976b113cb623dc07d1d416bf2badf98c0f26f7
Author: Jiri Olsa <jolsa@kernel.org>

selftests/bpf: Test const kfunc memory arguments

This commit adds BPF selftests and bpf_testmod kfuncs to verify passing
read-only global data, writable stack data, and const scalar structs to
const-qualified kfunc memory arguments, while rejecting read-only data
passed to writable memory arguments.

> diff --git a/tools/testing/selftests/bpf/progs/kfunc_call_test.c b/tools/testing/selftests/bpf/progs/kfunc_call_test.c
> index 8e6560c31e781..4f4a6713aa6e5 100644
> --- a/tools/testing/selftests/bpf/progs/kfunc_call_test.c
> +++ b/tools/testing/selftests/bpf/progs/kfunc_call_test.c
> @@ -5,6 +5,9 @@
>  #include "bpf_misc.h"
>  #include "../test_kmods/bpf_testmod_kfunc.h"
>  
> +static const char kfunc_const_data[] = "read-only source";
> +static const struct prog_test_pass1 kfunc_const_struct = { .x0 = 1 };
> +
>  static struct bpf_spin_lock kfunc_call_lock SEC(".data.A");
>  
>  SEC("tc")
> @@ -208,6 +211,17 @@ int kfunc_call_test_pass(struct __sk_buff *skb)
>  	return 0;
>  }
>  
> +SEC("tc")
> +int kfunc_call_test_const_mem(struct __sk_buff *skb)
> +{
> +	char stack_data[] = "read-write";
> +
> +	bpf_kfunc_call_test_const_mem(kfunc_const_data, sizeof(kfunc_const_data));
> +	bpf_kfunc_call_test_const_mem(stack_data, sizeof(stack_data));
> +	bpf_kfunc_call_test_const_struct(&kfunc_const_struct);
> +	return 0;
> +}

[Severity: Medium]
Does kfunc_call_test_const_mem() pass verification without a corresponding
verifier change to preserve const qualification on kfunc memory arguments?

In kernel/bpf/verifier.c, get_kfunc_arg_type() strips modifiers from the
pointed-to type via btf_type_skip_modifiers() and unconditionally marks
pointer+size and fixed-size memory arguments as writable:

kernel/bpf/verifier.c:get_kfunc_arg_type() {
	ref_id_ptr = &t->type;
	ref_t = btf_type_skip_modifiers(meta->btf, *ref_id_ptr, &ref_id);
	while (*ref_id_ptr != ref_id)
		ref_id_ptr = &btf_type_by_id(meta->btf, *ref_id_ptr)->type;
	...
	} else if (arg + 1 < nargs &&
		 (is_kfunc_arg_mem_size(meta->btf, &args[arg + 1]) ||
		  is_kfunc_arg_const_mem_size(meta->btf, &args[arg + 1]))) {
		...
		arg_type = ARG_PTR_TO_MEM | MEM_WRITE;
	} else if (btf_type_is_struct(ref_t))
		arg_type = ARG_PTR_TO_BTF_ID;
	else {
		...
		arg_type = ARG_PTR_TO_MEM | MEM_FIXED_SIZE | MEM_WRITE;
	}
}

Similarly, for bpf_kfunc_call_test_const_struct(), get_kfunc_arg_type()
records the argument as a BTF struct pointer with proto->arg_btf_id[arg]
pointing past the const modifier, and resolve_func_arg_type() then rewrites
the scalar struct argument to writable memory:

kernel/bpf/verifier.c:resolve_func_arg_type() {
	...
	*arg_type = ARG_PTR_TO_MEM | MEM_FIXED_SIZE | MEM_WRITE |
		    (*arg_type & (PTR_MAYBE_NULL | MEM_UNINIT));
}

When kfunc_call_test_const_mem() passes the .rodata globals kfunc_const_data
and &kfunc_const_struct to bpf_kfunc_call_test_const_mem() and
bpf_kfunc_call_test_const_struct(), check_func_arg() requests write access
via func_arg_access_type() -> check_helper_mem_access() ->
check_map_access_type(), which rejects the program with "write into map
forbidden".

Because kfunc_call_test_const_mem() is an autoloaded program in
kfunc_call_test.c, won't this also cause kfunc_call_test__open_and_load()
and kfunc_call_test_lskel__open_and_load() in verify_success() to fail for
all subtests using the kfunc_call_test skeleton?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261105102343.311331-5-jolsa@kernel.org?part=1