On Fri, Nov 29, 2024 at 5:29 AM Anton Protopopov <aspsk@xxxxxxxxxxxxx> wrote:
>
> Add a new set of tests to test the new field in PROG_LOAD-related
> part of bpf_attr: fd_array_cnt.
>
> Add the following test cases:
>
> * fd_array_cnt/no-fd-array: program is loaded in a normal
> way, without any fd_array present
>
> * fd_array_cnt/fd-array-ok: pass two extra non-used maps,
> check that they're bound to the program
>
> * fd_array_cnt/fd-array-dup-input: pass a few extra maps,
> only two of which are unique
>
> * fd_array_cnt/fd-array-ref-maps-in-array: pass a map in
> fd_array which is also referenced from within the program
>
> * fd_array_cnt/fd-array-trash-input: pass array with some trash
>
> * fd_array_cnt/fd-array-with-holes: pass an array with holes (fd=0)
>
> * fd_array_cnt/fd-array-2big: pass too large array
>
> All the tests above are using the bpf(2) syscall directly,
> no libbpf involved.
>
> Signed-off-by: Anton Protopopov <aspsk@xxxxxxxxxxxxx>
> ---
> kernel/bpf/verifier.c | 30 +-
> .../selftests/bpf/prog_tests/fd_array.c | 340 ++++++++++++++++++
> 2 files changed, 355 insertions(+), 15 deletions(-)
> create mode 100644 tools/testing/selftests/bpf/prog_tests/fd_array.c
>
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index d172f6974fd7..7102d85f580d 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -22620,7 +22620,7 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr, bpfptr_t uattr, __u3
> env->ops = bpf_verifier_ops[env->prog->type];
> ret = init_fd_array(env, attr, uattr);
> if (ret)
> - goto err_free_aux_data;
> + goto err_release_maps;
>
> env->allow_ptr_leaks = bpf_allow_ptr_leaks(env->prog->aux->token);
> env->allow_uninit_stack = bpf_allow_uninit_stack(env->prog->aux->token);
> @@ -22773,11 +22773,11 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr, bpfptr_t uattr, __u3
> copy_to_bpfptr_offset(uattr, offsetof(union bpf_attr, log_true_size),
> &log_true_size, sizeof(log_true_size))) {
> ret = -EFAULT;
> - goto err_release_maps;
> + goto err_ext;
> }
>
> if (ret)
> - goto err_release_maps;
> + goto err_ext;
>
> if (env->used_map_cnt) {
> /* if program passed verifier, update used_maps in bpf_prog_info */
> @@ -22787,7 +22787,7 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr, bpfptr_t uattr, __u3
>
> if (!env->prog->aux->used_maps) {
> ret = -ENOMEM;
> - goto err_release_maps;
> + goto err_ext;
> }
>
> memcpy(env->prog->aux->used_maps, env->used_maps,
> @@ -22801,7 +22801,7 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr, bpfptr_t uattr, __u3
> GFP_KERNEL);
> if (!env->prog->aux->used_btfs) {
> ret = -ENOMEM;
> - goto err_release_maps;
> + goto err_ext;
> }
>
> memcpy(env->prog->aux->used_btfs, env->used_btfs,
> @@ -22817,15 +22817,7 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr, bpfptr_t uattr, __u3
>
> adjust_btf_func(env);
>
> -err_release_maps:
> - if (!env->prog->aux->used_maps)
> - /* if we didn't copy map pointers into bpf_prog_info, release
> - * them now. Otherwise free_used_maps() will release them.
> - */
> - release_maps(env);
> - if (!env->prog->aux->used_btfs)
> - release_btfs(env);
> -
> +err_ext:
> /* extension progs temporarily inherit the attach_type of their targets
> for verification purposes, so set it back to zero before returning
> */
> @@ -22838,7 +22830,15 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr, bpfptr_t uattr, __u3
> err_unlock:
> if (!is_priv)
> mutex_unlock(&bpf_verifier_lock);
> -err_free_aux_data:
> +err_release_maps:
> + if (!env->prog->aux->used_maps)
> + /* if we didn't copy map pointers into bpf_prog_info, release
> + * them now. Otherwise free_used_maps() will release them.
> + */
> + release_maps(env);
> + if (!env->prog->aux->used_btfs)
> + release_btfs(env);
> +
> vfree(env->insn_aux_data);
> kvfree(env->insn_hist);
verifier.c hunk shouldn't be mixed with the selftests.
Looks like it should be in patch 3?
Not sure what it does though.
