On Wed, Jan 3, 2024 at 7:33 AM Barret Rhoden <brho@xxxxxxxxxx> wrote:
>
> When accessing an array, even if you insert your own bounds check,
> sometimes the compiler will remove the check, or modify it such that the
> verifier no longer knows your access is within bounds.
>
> The compiler is even free to make a copy of a register, check the copy,
> and use the original to access the array. The verifier knows the *copy*
> is within bounds, but not the original register!
>
> Signed-off-by: Barret Rhoden <brho@xxxxxxxxxx>
> ---
> tools/testing/selftests/bpf/Makefile | 2 +-
> .../bpf/prog_tests/test_array_elem.c | 112 ++++++++++
> .../selftests/bpf/progs/array_elem_test.c | 195 ++++++++++++++++++
> tools/testing/selftests/bpf/progs/bpf_misc.h | 43 ++++
> 4 files changed, 351 insertions(+), 1 deletion(-)
> create mode 100644 tools/testing/selftests/bpf/prog_tests/test_array_elem.c
> create mode 100644 tools/testing/selftests/bpf/progs/array_elem_test.c
>
I'm curious how bpf_cmp_likely/bpf_cmp_unlikely (just applied to
bpf-next) compares to this?
> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> index 617ae55c3bb5..651d4663cc78 100644
> --- a/tools/testing/selftests/bpf/Makefile
> +++ b/tools/testing/selftests/bpf/Makefile
> @@ -34,7 +34,7 @@ LIBELF_CFLAGS := $(shell $(PKG_CONFIG) libelf --cflags 2>/dev/null)
> LIBELF_LIBS := $(shell $(PKG_CONFIG) libelf --libs 2>/dev/null || echo -lelf)
>
> CFLAGS += -g $(OPT_FLAGS) -rdynamic \
> - -Wall -Werror \
> + -dicks -Wall -Werror \
what does this magic argument do? )
> $(GENFLAGS) $(SAN_CFLAGS) $(LIBELF_CFLAGS) \
> -I$(CURDIR) -I$(INCLUDE_DIR) -I$(GENDIR) -I$(LIBDIR) \
> -I$(TOOLSINCDIR) -I$(APIDIR) -I$(OUTPUT)
> diff --git a/tools/testing/selftests/bpf/prog_tests/test_array_elem.c b/tools/testing/selftests/bpf/prog_tests/test_array_elem.c
> new file mode 100644
> index 000000000000..c953636f07c9
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/test_array_elem.c
> @@ -0,0 +1,112 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2024 Google LLC. */
> +#include <test_progs.h>
> +#include "array_elem_test.skel.h"
> +
> +#define NR_MAP_ELEMS 100
> +
> +/*
> + * Helper to load and run a program.
> + * Call must define skel, map_elems, and bss_elems.
> + * Destroy the skel when you're done.
> + */
> +#define load_and_run(PROG) ({
does this have to be a macro? Can you write it as a function?
\
> + int err; \
> + skel = array_elem_test__open(); \
> + if (!ASSERT_OK_PTR(skel, "array_elem_test open")) \
> + return; \
> + bpf_program__set_autoload(skel->progs.x_ ## PROG, true); \
> + err = array_elem_test__load(skel); \
> + if (!ASSERT_EQ(err, 0, "array_elem_test load")) { \
> + array_elem_test__destroy(skel); \
> + return; \
> + } \
> + err = array_elem_test__attach(skel); \
> + if (!ASSERT_EQ(err, 0, "array_elem_test attach")) { \
> + array_elem_test__destroy(skel); \
> + return; \
> + } \
> + for (int i = 0; i < NR_MAP_ELEMS; i++) \
> + skel->bss->lookup_indexes[i] = i; \
> + map_elems = bpf_map__mmap(skel->maps.arraymap); \
> + ASSERT_OK_PTR(map_elems, "mmap"); \
> + bss_elems = skel->bss->bss_elems; \
> + skel->bss->target_pid = getpid(); \
> + usleep(1); \
> +})
> +
[...]
