diff mbox series

[bpf-next,2/2] selftests/bpf: add inline assembly helpers to access array elements

Message ID 20240103153307.553838-3-brho@google.com (mailing list archive)
State Superseded
Delegated to: BPF
Headers show
Series inline asm helpers to access array elements | expand

Checks

Context Check Description
bpf/vmtest-bpf-next-PR success PR summary
bpf/vmtest-bpf-next-VM_Test-11 success Logs for s390x-gcc / build / build for s390x with gcc
bpf/vmtest-bpf-next-VM_Test-16 success Logs for s390x-gcc / test (test_verifier, false, 360) / test_verifier on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-18 success Logs for set-matrix
bpf/vmtest-bpf-next-VM_Test-24 success Logs for x86_64-gcc / test (test_progs_no_alu32_parallel, true, 30) / test_progs_no_alu32_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-21 success Logs for x86_64-gcc / test (test_maps, false, 360) / test_maps on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-17 success Logs for s390x-gcc / veristat
bpf/vmtest-bpf-next-VM_Test-25 success Logs for x86_64-gcc / test (test_progs_parallel, true, 30) / test_progs_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-26 success Logs for x86_64-gcc / test (test_verifier, false, 360) / test_verifier on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-35 success Logs for x86_64-llvm-18 / build / build for x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-28 success Logs for x86_64-llvm-17 / build / build for x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-20 success Logs for x86_64-gcc / build-release
bpf/vmtest-bpf-next-VM_Test-27 success Logs for x86_64-gcc / veristat / veristat on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-37 success Logs for x86_64-llvm-18 / test (test_maps, false, 360) / test_maps on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-22 success Logs for x86_64-gcc / test (test_progs, false, 360) / test_progs on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-30 success Logs for x86_64-llvm-17 / test (test_maps, false, 360) / test_maps on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-36 success Logs for x86_64-llvm-18 / build-release / build for x86_64 with llvm-18 and -O2 optimization
bpf/vmtest-bpf-next-VM_Test-23 success Logs for x86_64-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-33 success Logs for x86_64-llvm-17 / test (test_verifier, false, 360) / test_verifier on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-31 success Logs for x86_64-llvm-17 / test (test_progs, false, 360) / test_progs on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-38 success Logs for x86_64-llvm-18 / test (test_progs, false, 360) / test_progs on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-19 success Logs for x86_64-gcc / build / build for x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-40 success Logs for x86_64-llvm-18 / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-42 success Logs for x86_64-llvm-18 / veristat
bpf/vmtest-bpf-next-VM_Test-34 success Logs for x86_64-llvm-17 / veristat
bpf/vmtest-bpf-next-VM_Test-39 success Logs for x86_64-llvm-18 / test (test_progs_cpuv4, false, 360) / test_progs_cpuv4 on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-32 success Logs for x86_64-llvm-17 / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-41 success Logs for x86_64-llvm-18 / test (test_verifier, false, 360) / test_verifier on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-29 success Logs for x86_64-llvm-17 / build-release / build for x86_64 with llvm-17 and -O2 optimization
bpf/vmtest-bpf-next-VM_Test-14 success Logs for s390x-gcc / test (test_progs, false, 360) / test_progs on s390x with gcc
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for bpf-next, async
netdev/ynl success SINGLE THREAD; Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 8 this patch: 8
netdev/cc_maintainers warning 10 maintainers not CCed: sdf@google.com haoluo@google.com eddyz87@gmail.com kpsingh@kernel.org martin.lau@linux.dev jolsa@kernel.org shuah@kernel.org linux-kselftest@vger.kernel.org mykolal@fb.com john.fastabend@gmail.com
netdev/build_clang success Errors and warnings before: 8 this patch: 8
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 8 this patch: 8
netdev/checkpatch warning CHECK: Please don't use multiple blank lines WARNING: Avoid line continuations in quoted strings WARNING: Avoid unnecessary line continuations WARNING: added, moved or deleted file(s), does MAINTAINERS need updating? WARNING: please, no space before tabs
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
bpf/vmtest-bpf-next-VM_Test-12 success Logs for s390x-gcc / build-release
bpf/vmtest-bpf-next-VM_Test-4 success Logs for aarch64-gcc / build / build for aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-13 success Logs for set-matrix
bpf/vmtest-bpf-next-VM_Test-9 success Logs for aarch64-gcc / test (test_verifier, false, 360) / test_verifier on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-15 success Logs for x86_64-gcc / build-release
bpf/vmtest-bpf-next-VM_Test-6 success Logs for aarch64-gcc / test (test_maps, false, 360) / test_maps on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-2 success Logs for Unittests
bpf/vmtest-bpf-next-VM_Test-8 success Logs for set-matrix
bpf/vmtest-bpf-next-VM_Test-1 success Logs for ShellCheck
bpf/vmtest-bpf-next-VM_Test-7 success Logs for s390x-gcc / build-release
bpf/vmtest-bpf-next-VM_Test-3 success Logs for Validate matrix.py
bpf/vmtest-bpf-next-VM_Test-10 success Logs for x86_64-gcc / build-release
bpf/vmtest-bpf-next-VM_Test-0 success Logs for Lint
bpf/vmtest-bpf-next-VM_Test-5 success Logs for aarch64-gcc / build-release

Commit Message

Barret Rhoden Jan. 3, 2024, 3:33 p.m. UTC 
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@google.com>
---
 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

Comments

John Fastabend Jan. 3, 2024, 5:52 p.m. UTC | #1 
Barret Rhoden 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@google.com>
> ---
>  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
> 
> 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 						\
>  	  $(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

[...]

> diff --git a/tools/testing/selftests/bpf/progs/bpf_misc.h b/tools/testing/selftests/bpf/progs/bpf_misc.h
> index 2fd59970c43a..002bab44cde2 100644
> --- a/tools/testing/selftests/bpf/progs/bpf_misc.h
> +++ b/tools/testing/selftests/bpf/progs/bpf_misc.h
> @@ -135,4 +135,47 @@
>  /* make it look to compiler like value is read and written */
>  #define __sink(expr) asm volatile("" : "+g"(expr))
>  
> +/*
> + * Access an array element within a bound, such that the verifier knows the
> + * access is safe.
> + *
> + * This macro asm is the equivalent of:
> + *
> + *	if (!arr)
> + *		return NULL;
> + *	if (idx >= arr_sz)
> + *		return NULL;
> + *	return &arr[idx];
> + *
> + * The index (___idx below) needs to be a u64, at least for certain versions of
> + * the BPF ISA, since there aren't u32 conditional jumps.
> + */

This is nice, but in practice what we've been doing is making
our maps power of 2 and then just masking them as needed. I think
this is more efficient if you care about performance.

FWIW I'm not opposed to having this though.

> +#define bpf_array_elem(arr, arr_sz, idx) ({				\
> +	typeof(&(arr)[0]) ___arr = arr;					\
> +	__u64 ___idx = idx;						\
> +	if (___arr) {							\
> +		asm volatile("if %[__idx] >= %[__bound] goto 1f;	\
> +			      %[__idx] *= %[__size];		\
> +			      %[__arr] += %[__idx];		\
> +			      goto 2f;				\

+1 for using asm goto :)

> +			      1:;				\
> +			      %[__arr] = 0;			\
> +			      2:				\
> +			      "						\
> +			     : [__arr]"+r"(___arr), [__idx]"+r"(___idx)	\
> +			     : [__bound]"r"((arr_sz)),		        \
> +			       [__size]"i"(sizeof(typeof((arr)[0])))	\
> +			     : "cc");					\
> +	}								\
> +	___arr;								\
> +})
> +
> +/*
> + * Convenience wrapper for bpf_array_elem(), where we compute the size of the
> + * array.  Be sure to use an actual array, and not a pointer, just like with the
> + * ARRAY_SIZE macro.
> + */
> +#define bpf_array_sz_elem(arr, idx) \
> +	bpf_array_elem(arr, sizeof(arr) / sizeof((arr)[0]), idx)
> +
>  #endif
> -- 
> 2.43.0.472.g3155946c3a-goog
> 
>
Andrii Nakryiko Jan. 3, 2024, 7:51 p.m. UTC | #2 
On Wed, Jan 3, 2024 at 7:33 AM Barret Rhoden <brho@google.com> 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@google.com>
> ---
>  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);                                                      \
> +})
> +


[...]
Barret Rhoden Jan. 3, 2024, 8:06 p.m. UTC | #3 
On 1/3/24 14:51, Andrii Nakryiko wrote:
> I'm curious how bpf_cmp_likely/bpf_cmp_unlikely (just applied to
> bpf-next) compares to this?

these work great!

e.g.

         if (bpf_cmp_likely(idx, <, NR_MAP_ELEMS))
                 map_elems[idx] = i;

works fine.  since that's essentially the code that bpf_array_elem() was 
trying to replace, i'd rather just use the new bpf_cmp helpers than have 
the special array_elem helpers.

thanks,

barret
Andrii Nakryiko Jan. 3, 2024, 9:21 p.m. UTC | #4 
On Wed, Jan 3, 2024 at 12:06 PM Barret Rhoden <brho@google.com> wrote:
>
> On 1/3/24 14:51, Andrii Nakryiko wrote:
> > I'm curious how bpf_cmp_likely/bpf_cmp_unlikely (just applied to
> > bpf-next) compares to this?
>
> these work great!
>
> e.g.
>
>          if (bpf_cmp_likely(idx, <, NR_MAP_ELEMS))
>                  map_elems[idx] = i;
>
> works fine.  since that's essentially the code that bpf_array_elem() was
> trying to replace, i'd rather just use the new bpf_cmp helpers than have
> the special array_elem helpers.

ok, cool, thanks for checking! The less special macros, the better.

>
> thanks,
>
> barret
>
>
Barret Rhoden Jan. 4, 2024, 9:32 p.m. UTC | #5 
On 1/3/24 16:21, Andrii Nakryiko wrote:
> On Wed, Jan 3, 2024 at 12:06 PM Barret Rhoden<brho@google.com>  wrote:
>> On 1/3/24 14:51, Andrii Nakryiko wrote:
>>> I'm curious how bpf_cmp_likely/bpf_cmp_unlikely (just applied to
>>> bpf-next) compares to this?
>> these work great!
>>
>> e.g.
>>
>>           if (bpf_cmp_likely(idx, <, NR_MAP_ELEMS))
>>                   map_elems[idx] = i;
>>
>> works fine.  since that's essentially the code that bpf_array_elem() was
>> trying to replace, i'd rather just use the new bpf_cmp helpers than have
>> the special array_elem helpers.
> ok, cool, thanks for checking! The less special macros, the better.

sorry - turns out it only worked in testing.  in my actual program, i 
still run into issues.  the comparison is done, which is what bpf_cmp 
enforces.  but the compiler is discarding the comparison.  i have more 
info in the other thread, but figured i'd mention it here too.  =(
Barret Rhoden Jan. 4, 2024, 9:37 p.m. UTC | #6 
On 1/3/24 14:51, Andrii Nakryiko wrote:
>> +
>> +/*
>> + * 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?

can do.  (if we keep these patches).

i used a macro for the ## PROG below, but i can do something with ints 
and switches to turn on the autoload for a single prog.  or just 
copy-paste the boilerplate.

>> +       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);        \

thanks,

barret
Andrii Nakryiko Jan. 4, 2024, 10:45 p.m. UTC | #7 
On Thu, Jan 4, 2024 at 1:37 PM Barret Rhoden <brho@google.com> wrote:
>
> On 1/3/24 14:51, Andrii Nakryiko wrote:
> >> +
> >> +/*
> >> + * 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?
>
> can do.  (if we keep these patches).
>
> i used a macro for the ## PROG below, but i can do something with ints
> and switches to turn on the autoload for a single prog.  or just
> copy-paste the boilerplate.

why can't you pass the `struct bpf_program *prog` parameter?

>
> >> +       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);        \
>
> thanks,
>
> barret
>
>
diff mbox series

Patch

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 						\
 	  $(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) ({						\
+	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);							\
+})
+
+static void test_access_all(void)
+{
+	struct array_elem_test *skel;
+	int *map_elems;
+	int *bss_elems;
+
+	load_and_run(access_all);
+
+	for (int i = 0; i < NR_MAP_ELEMS; i++)
+		ASSERT_EQ(map_elems[i], i, "array_elem map value not written");
+
+	for (int i = 0; i < NR_MAP_ELEMS; i++)
+		ASSERT_EQ(bss_elems[i], i, "array_elem bss value not written");
+
+	array_elem_test__destroy(skel);
+}
+
+static void test_oob_access(void)
+{
+	struct array_elem_test *skel;
+	int *map_elems;
+	int *bss_elems;
+
+	load_and_run(oob_access);
+
+	for (int i = 0; i < NR_MAP_ELEMS; i++)
+		ASSERT_EQ(map_elems[i], 0, "array_elem map value was written");
+
+	for (int i = 0; i < NR_MAP_ELEMS; i++)
+		ASSERT_EQ(bss_elems[i], 0, "array_elem bss value was written");
+
+	array_elem_test__destroy(skel);
+}
+
+static void test_access_array_map_infer_sz(void)
+{
+	struct array_elem_test *skel;
+	int *map_elems;
+	int *bss_elems __maybe_unused;
+
+	load_and_run(access_array_map_infer_sz);
+
+	for (int i = 0; i < NR_MAP_ELEMS; i++)
+		ASSERT_EQ(map_elems[i], i, "array_elem map value not written");
+
+	array_elem_test__destroy(skel);
+}
+
+
+/* Test that attempting to load a bad program fails. */
+#define test_bad(PROG) ({						\
+	struct array_elem_test *skel;					\
+	int err;							\
+	skel = array_elem_test__open();					\
+	if (!ASSERT_OK_PTR(skel, "array_elem_test open"))		\
+		return;							\
+	bpf_program__set_autoload(skel->progs.x_bad_ ## PROG, true); 	\
+	err = array_elem_test__load(skel);				\
+	ASSERT_ERR(err, "array_elem_test load " # PROG);		\
+	array_elem_test__destroy(skel);					\
+})
+
+void test_test_array_elem(void)
+{
+	if (test__start_subtest("array_elem_access_all"))
+		test_access_all();
+	if (test__start_subtest("array_elem_oob_access"))
+		test_oob_access();
+	if (test__start_subtest("array_elem_access_array_map_infer_sz"))
+		test_access_array_map_infer_sz();
+	if (test__start_subtest("array_elem_bad_map_array_access"))
+		test_bad(map_array_access);
+	if (test__start_subtest("array_elem_bad_bss_array_access"))
+		test_bad(bss_array_access);
+}
diff --git a/tools/testing/selftests/bpf/progs/array_elem_test.c b/tools/testing/selftests/bpf/progs/array_elem_test.c
new file mode 100644
index 000000000000..9d48afc933f0
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/array_elem_test.c
@@ -0,0 +1,195 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2024 Google LLC. */
+#include <stdbool.h>
+#include <linux/types.h>
+#include <linux/bpf.h>
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+#include "bpf_misc.h"
+
+char _license[] SEC("license") = "GPL";
+
+int target_pid = 0;
+
+#define NR_MAP_ELEMS 100
+
+/*
+ * We want to test valid accesses into an array, but we also need to fool the
+ * verifier.  If we just do for (i = 0; i < 100; i++), the verifier knows the
+ * value of i and can tell we're inside the array.
+ *
+ * This "lookup" array is just the values 0, 1, 2..., such that
+ * lookup_indexes[i] == i.  (set by userspace).  But the verifier doesn't know
+ * that.
+ */
+unsigned int lookup_indexes[NR_MAP_ELEMS];
+
+/* Arrays can be in the BSS or inside a map element.  Make sure both work. */
+int bss_elems[NR_MAP_ELEMS];
+
+struct map_array {
+	int elems[NR_MAP_ELEMS];
+};
+
+/*
+ * This is an ARRAY_MAP of a single struct, and that struct is an array of
+ * elements.  Userspace can mmap the map as if it was just a basic array of
+ * elements.  Though if you make an ARRAY_MAP where the *values* are ints, don't
+ * forget that bpf map elements are rounded up to 8 bytes.
+ *
+ * Once you get the pointer to the base of the inner array, you can access all
+ * of the elements without another bpf_map_lookup_elem(), which is useful if you
+ * are operating on multiple elements while holding a spinlock.
+ */
+struct {
+	__uint(type, BPF_MAP_TYPE_ARRAY);
+	__uint(max_entries, 1);
+	__type(key, int);
+	__type(value, struct map_array);
+	__uint(map_flags, BPF_F_MMAPABLE);
+} arraymap SEC(".maps");
+
+static struct map_array *get_map_array(void)
+{
+	int zero = 0;
+
+	return bpf_map_lookup_elem(&arraymap, &zero);
+}
+
+static int *get_map_elems(void)
+{
+	struct map_array *arr = get_map_array();
+
+	if (!arr)
+		return NULL;
+	return arr->elems;
+}
+
+/*
+ * Test that we can access all elements, and that we are accessing the element
+ * we think we are accessing.
+ */
+static void access_all(void)
+{
+	int *map_elems = get_map_elems();
+	int *x;
+
+	for (int i = 0; i < NR_MAP_ELEMS; i++) {
+		x = bpf_array_elem(map_elems, NR_MAP_ELEMS, lookup_indexes[i]);
+		if (x)
+			*x = i;
+	}
+
+	for (int i = 0; i < NR_MAP_ELEMS; i++) {
+		x = bpf_array_sz_elem(bss_elems, lookup_indexes[i]);
+		if (x)
+			*x = i;
+	}
+}
+
+SEC("?tp/syscalls/sys_enter_nanosleep")
+int x_access_all(void *ctx)
+{
+	if ((bpf_get_current_pid_tgid() >> 32) != target_pid)
+		return 0;
+	access_all();
+	return 0;
+}
+
+/*
+ * Helper for various OOB tests.  An out-of-bound access should be handled like
+ * a lookup failure.  Specifically, the verifier should ensure we do not access
+ * outside the array.  Userspace will check that we didn't access somewhere
+ * inside the array.
+ */
+static void set_elem_to_1(long idx)
+{
+	int *map_elems = get_map_elems();
+	int *x;
+
+	x = bpf_array_elem(map_elems, NR_MAP_ELEMS, idx);
+	if (x)
+		*x = 1;
+	x = bpf_array_sz_elem(bss_elems, idx);
+	if (x)
+		*x = 1;
+}
+
+/*
+ * Test various out-of-bounds accesses.
+ */
+static void oob_access(void)
+{
+	set_elem_to_1(NR_MAP_ELEMS + 5);
+	set_elem_to_1(NR_MAP_ELEMS);
+	set_elem_to_1(-1);
+	set_elem_to_1(~0UL);
+}
+
+SEC("?tp/syscalls/sys_enter_nanosleep")
+int x_oob_access(void *ctx)
+{
+	if ((bpf_get_current_pid_tgid() >> 32) != target_pid)
+		return 0;
+	oob_access();
+	return 0;
+}
+
+/*
+ * Test that we can use the ARRAY_SIZE-style helper with an array in a map.
+ *
+ * Note that you cannot infer the size of the array from just a pointer; you
+ * have to use the actual elems[100].  i.e. this will fail and should fail to
+ * compile (-Wsizeof-pointer-div):
+ *
+ *	int *map_elems = get_map_elems();
+ *	x = bpf_array_sz_elem(map_elems, lookup_indexes[i]);
+ */
+static void access_array_map_infer_sz(void)
+{
+	struct map_array *arr = get_map_array();
+	int *x;
+
+	for (int i = 0; i < NR_MAP_ELEMS; i++) {
+		x = bpf_array_sz_elem(arr->elems, lookup_indexes[i]);
+		if (x)
+			*x = i;
+	}
+}
+
+SEC("?tp/syscalls/sys_enter_nanosleep")
+int x_access_array_map_infer_sz(void *ctx)
+{
+	if ((bpf_get_current_pid_tgid() >> 32) != target_pid)
+		return 0;
+	access_array_map_infer_sz();
+	return 0;
+}
+
+
+
+SEC("?tp/syscalls/sys_enter_nanosleep")
+int x_bad_map_array_access(void *ctx)
+{
+	int *map_elems = get_map_elems();
+
+	/*
+	 * Need to check to promote map_elems from MAP_OR_NULL to MAP so that we
+	 * fail to load below for the right reason.
+	 */
+	if (!map_elems)
+		return 0;
+	/* Fail to load: we don't prove our access is inside map_elems[] */
+	for (int i = 0; i < NR_MAP_ELEMS; i++)
+		map_elems[lookup_indexes[i]] = i;
+	return 0;
+}
+
+SEC("?tp/syscalls/sys_enter_nanosleep")
+int x_bad_bss_array_access(void *ctx)
+{
+	/* Fail to load: we don't prove our access is inside bss_elems[] */
+	for (int i = 0; i < NR_MAP_ELEMS; i++)
+		bss_elems[lookup_indexes[i]] = i;
+	return 0;
+}
diff --git a/tools/testing/selftests/bpf/progs/bpf_misc.h b/tools/testing/selftests/bpf/progs/bpf_misc.h
index 2fd59970c43a..002bab44cde2 100644
--- a/tools/testing/selftests/bpf/progs/bpf_misc.h
+++ b/tools/testing/selftests/bpf/progs/bpf_misc.h
@@ -135,4 +135,47 @@ 
 /* make it look to compiler like value is read and written */
 #define __sink(expr) asm volatile("" : "+g"(expr))
 
+/*
+ * Access an array element within a bound, such that the verifier knows the
+ * access is safe.
+ *
+ * This macro asm is the equivalent of:
+ *
+ *	if (!arr)
+ *		return NULL;
+ *	if (idx >= arr_sz)
+ *		return NULL;
+ *	return &arr[idx];
+ *
+ * The index (___idx below) needs to be a u64, at least for certain versions of
+ * the BPF ISA, since there aren't u32 conditional jumps.
+ */
+#define bpf_array_elem(arr, arr_sz, idx) ({				\
+	typeof(&(arr)[0]) ___arr = arr;					\
+	__u64 ___idx = idx;						\
+	if (___arr) {							\
+		asm volatile("if %[__idx] >= %[__bound] goto 1f;	\
+			      %[__idx] *= %[__size];		\
+			      %[__arr] += %[__idx];		\
+			      goto 2f;				\
+			      1:;				\
+			      %[__arr] = 0;			\
+			      2:				\
+			      "						\
+			     : [__arr]"+r"(___arr), [__idx]"+r"(___idx)	\
+			     : [__bound]"r"((arr_sz)),		        \
+			       [__size]"i"(sizeof(typeof((arr)[0])))	\
+			     : "cc");					\
+	}								\
+	___arr;								\
+})
+
+/*
+ * Convenience wrapper for bpf_array_elem(), where we compute the size of the
+ * array.  Be sure to use an actual array, and not a pointer, just like with the
+ * ARRAY_SIZE macro.
+ */
+#define bpf_array_sz_elem(arr, idx) \
+	bpf_array_elem(arr, sizeof(arr) / sizeof((arr)[0]), idx)
+
 #endif