Re: [RFC bpf-next v1 8/8] selftests/bpf: test no_caller_saved_registers spill/fill removal

Andrii Nakryiko
2024-07-02T00:42:56Z

On Sat, Jun 29, 2024 at 2:48 AM Eduard Zingerman <eddyz87@gmail.com> wrote:
>
> Tests for no_caller_saved_registers processing logic
> (see verifier.c:match_and_mark_nocsr_pattern()):
> - a canary positive test case;
> - various tests with broken patterns;
> - tests with read/write fixed/varying stack access that violate nocsr
> stack access contract;
> - tests with multiple subprograms.
>
> Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
> ---
> .../selftests/bpf/prog_tests/verifier.c | 7 +
> .../selftests/bpf/progs/verifier_nocsr.c | 437 ++++++++++++++++++
> 2 files changed, 444 insertions(+)
> create mode 100644 tools/testing/selftests/bpf/progs/verifier_nocsr.c
>

You don't seem to have a case where offset is not a multiple of 8
(though it would have to be a sub-register spill which would be
"rejected" anyway, so not sure if there is anything to add here)

> diff --git a/tools/testing/selftests/bpf/prog_tests/verifier.c b/tools/testing/selftests/bpf/prog_tests/verifier.c
> index 6816ff064516..8e056c36c549 100644
> --- a/tools/testing/selftests/bpf/prog_tests/verifier.c
> +++ b/tools/testing/selftests/bpf/prog_tests/verifier.c
> @@ -53,6 +53,7 @@
> #include "verifier_movsx.skel.h"
> #include "verifier_netfilter_ctx.skel.h"
> #include "verifier_netfilter_retcode.skel.h"
> +#include "verifier_nocsr.skel.h"
> #include "verifier_precision.skel.h"
> #include "verifier_prevent_map_lookup.skel.h"
> #include "verifier_raw_stack.skel.h"
> @@ -171,6 +172,12 @@ void test_verifier_meta_access(void) { RUN(verifier_meta_access); }
> void test_verifier_movsx(void) { RUN(verifier_movsx); }
> void test_verifier_netfilter_ctx(void) { RUN(verifier_netfilter_ctx); }
> void test_verifier_netfilter_retcode(void) { RUN(verifier_netfilter_retcode); }
> +void test_verifier_nocsr(void)
> +{
> +#if defined(__x86_64__)
> + RUN(verifier_nocsr);
> +#endif /* __x86_64__ */

maybe #else <mark-as-skipped> ?

> +}
> void test_verifier_precision(void) { RUN(verifier_precision); }
> void test_verifier_prevent_map_lookup(void) { RUN(verifier_prevent_map_lookup); }
> void test_verifier_raw_stack(void) { RUN(verifier_raw_stack); }
> diff --git a/tools/testing/selftests/bpf/progs/verifier_nocsr.c b/tools/testing/selftests/bpf/progs/verifier_nocsr.c
> new file mode 100644
> index 000000000000..5ddc2c97ada6
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/verifier_nocsr.c
> @@ -0,0 +1,437 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <linux/bpf.h>
> +#include <bpf/bpf_helpers.h>
> +#include "bpf_misc.h"
> +
> +#define __xlated_bpf_get_smp_processor_id \
> + __xlated(": w0 = ") \

how will this work for no_alu32 mode?

> + __xlated(": r0 = &(void __percpu *)(r0)") \
> + __xlated(": r0 = *(u32 *)(r0 +0)")
> +
> +SEC("raw_tp")
> +__xlated("4: r5 = 5")
> +__xlated_bpf_get_smp_processor_id
> +__xlated("8: exit")
> +__success
> +__naked void simple(void)
> +{
> + asm volatile (
> + "r1 = 1;"
> + "r2 = 2;"
> + "r3 = 3;"
> + "r4 = 4;"
> + "r5 = 5;"
> + "*(u64 *)(r10 - 16) = r1;"
> + "*(u64 *)(r10 - 24) = r2;"
> + "*(u64 *)(r10 - 32) = r3;"
> + "*(u64 *)(r10 - 40) = r4;"
> + "*(u64 *)(r10 - 48) = r5;"
> + "call %[bpf_get_smp_processor_id];"
> + "r5 = *(u64 *)(r10 - 48);"
> + "r4 = *(u64 *)(r10 - 40);"
> + "r3 = *(u64 *)(r10 - 32);"
> + "r2 = *(u64 *)(r10 - 24);"
> + "r1 = *(u64 *)(r10 - 16);"
> + "exit;"
> + :
> + : __imm(bpf_get_smp_processor_id)
> + : __clobber_all);
> +} …