Hi,
On 2/28/2025 8:33 AM, Emil Tsalapatis wrote:
> Add selftests for the bpf_cpumask_fill helper that sets a bpf_cpumask to
> a bit pattern provided by a BPF program.
>
> Signed-off-by: Emil Tsalapatis (Meta) <emil@xxxxxxxxxxxxxxx>
> ---
> .../selftests/bpf/prog_tests/verifier.c | 2 +
> .../selftests/bpf/progs/cpumask_success.c | 23 ++++++
> .../selftests/bpf/progs/verifier_cpumask.c | 77 +++++++++++++++++++
> 3 files changed, 102 insertions(+)
> create mode 100644 tools/testing/selftests/bpf/progs/verifier_cpumask.c
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/verifier.c b/tools/testing/selftests/bpf/prog_tests/verifier.c
> index 8a0e1ff8a2dc..4dd95e93bd7e 100644
> --- a/tools/testing/selftests/bpf/prog_tests/verifier.c
> +++ b/tools/testing/selftests/bpf/prog_tests/verifier.c
> @@ -23,6 +23,7 @@
> #include "verifier_cgroup_storage.skel.h"
> #include "verifier_const.skel.h"
> #include "verifier_const_or.skel.h"
> +#include "verifier_cpumask.skel.h"
> #include "verifier_ctx.skel.h"
> #include "verifier_ctx_sk_msg.skel.h"
> #include "verifier_d_path.skel.h"
> @@ -155,6 +156,7 @@ void test_verifier_cgroup_skb(void) { RUN(verifier_cgroup_skb); }
> void test_verifier_cgroup_storage(void) { RUN(verifier_cgroup_storage); }
> void test_verifier_const(void) { RUN(verifier_const); }
> void test_verifier_const_or(void) { RUN(verifier_const_or); }
> +void test_verifier_cpumask(void) { RUN(verifier_cpumask); }
Why is a new file necessary ? Is it more reasonable to add these success
and failure test cases in cpumask_success.c and cpumask_failure.c ?
> void test_verifier_ctx(void) { RUN(verifier_ctx); }
> void test_verifier_ctx_sk_msg(void) { RUN(verifier_ctx_sk_msg); }
> void test_verifier_d_path(void) { RUN(verifier_d_path); }
> diff --git a/tools/testing/selftests/bpf/progs/cpumask_success.c b/tools/testing/selftests/bpf/progs/cpumask_success.c
> index 80ee469b0b60..f252aa2f3090 100644
> --- a/tools/testing/selftests/bpf/progs/cpumask_success.c
> +++ b/tools/testing/selftests/bpf/progs/cpumask_success.c
> @@ -770,3 +770,26 @@ int BPF_PROG(test_refcount_null_tracking, struct task_struct *task, u64 clone_fl
> bpf_cpumask_release(mask2);
> return 0;
> }
> +
> +SEC("syscall")
> +__success
> +int BPF_PROG(test_fill_reject_small_mask)
> +{
> + struct bpf_cpumask *local;
> + u8 toofewbits;
> + int ret;
> +
> + local = create_cpumask();
> + if (!local)
> + return 0;
> +
> + /* The kfunc should prevent this operation */
> + ret = bpf_cpumask_fill((struct cpumask *)local, &toofewbits, sizeof(toofewbits));
> + if (ret != -EACCES)
> + err = 2;
The check may not be true when running local with a smaller NR_CPUS. It
will be more reasonable to adjust the size according to the value of
nr_cpu_ids.
> +
> + bpf_cpumask_release(local);
> +
> + return 0;
> +}
> +
> diff --git a/tools/testing/selftests/bpf/progs/verifier_cpumask.c b/tools/testing/selftests/bpf/progs/verifier_cpumask.c
> new file mode 100644
> index 000000000000..bb84dd36beac
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/verifier_cpumask.c
> @@ -0,0 +1,77 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2025 Meta Platforms, Inc. and affiliates. */
> +
> +#include <vmlinux.h>
> +#include <bpf/bpf_tracing.h>
> +#include <bpf/bpf_helpers.h>
> +#include "bpf_misc.h"
> +
> +#include "cpumask_common.h"
> +
> +#define CPUMASK_TEST_MASKLEN (8 * sizeof(u64))
> +
> +u64 bits[CPUMASK_TEST_MASKLEN];
> +
> +SEC("syscall")
> +__success
> +int BPF_PROG(test_cpumask_fill)
> +{
> + struct bpf_cpumask *mask;
> + int ret;
> +
> + mask = bpf_cpumask_create();
> + if (!mask) {
> + err = 1;
> + return 0;
> + }
> +
> + ret = bpf_cpumask_fill((struct cpumask *)mask, bits, CPUMASK_TEST_MASKLEN);
> + if (!ret)
> + err = 2;
It would be better to also test the cpu bits in the cpumask after
bpf_cpumask_fill() is expected.
> +
> + if (mask)
> + bpf_cpumask_release(mask);
The "if (mask)" check is unnecessary.
> +
> + return 0;
> +}
> +
> +SEC("syscall")
> +__description("bpf_cpumask_fill: invalid cpumask target")
> +__failure __msg("type=scalar expected=fp")
> +int BPF_PROG(test_cpumask_fill_cpumask_invalid)
> +{
> + struct bpf_cpumask *invalid = (struct bpf_cpumask *)0x123456;
> + int ret;
> +
> + ret = bpf_cpumask_fill((struct cpumask *)invalid, bits, CPUMASK_TEST_MASKLEN);
> + if (!ret)
> + err = 2;
> +
> + return 0;
> +}
> +
> +SEC("syscall")
> +__description("bpf_cpumask_fill: invalid cpumask source")
> +__failure __msg("leads to invalid memory access")
> +int BPF_PROG(test_cpumask_fill_bpf_invalid)
> +{
> + void *garbage = (void *)0x123456;
> + struct bpf_cpumask *local;
> + int ret;
> +
> + local = create_cpumask();
> + if (!local) {
> + err = 1;
> + return 0;
> + }
> +
> + ret = bpf_cpumask_fill((struct cpumask *)local, garbage, CPUMASK_TEST_MASKLEN);
> + if (!ret)
> + err = 2;
> +
> + bpf_cpumask_release(local);
> +
> + return 0;
> +}
> +
> +char _license[] SEC("license") = "GPL";
