linux-kernel - Re: [PATCH bpf-next 2/2] selftests/bpf: add inline assembly helpers to access array elements

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAEf4BzbKT3LbHQSFwpAfoJuhyGy2NpHk7A6ivkFiutN_jnKHYg@mail.gmail.com>
Date: Wed, 3 Jan 2024 11:51:13 -0800
From: Andrii Nakryiko <andrii.nakryiko@...il.com>
To: Barret Rhoden <brho@...gle.com>
Cc: Andrii Nakryiko <andrii@...nel.org>, Alexei Starovoitov <ast@...nel.org>, 
	Daniel Borkmann <daniel@...earbox.net>, Song Liu <song@...nel.org>, 
	Yonghong Song <yonghong.song@...ux.dev>, mattbobrowski@...gle.com, bpf@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH bpf-next 2/2] selftests/bpf: add inline assembly helpers
 to access array elements

On Wed, Jan 3, 2024 at 7:33 AM Barret Rhoden <brho@...gle.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@...gle.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);                                                      \
> +})
> +


[...]