netdev - Re: [PATCH bpf-next] selftests/bpf: use getpagesize() to initialize ring buffer size

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: <CAEf4BzYHggCfbSGb8autEDcHhZXabK-n36rggyjJeL0uLEr+DQ@mail.gmail.com>
Date:   Mon, 31 Jan 2022 16:02:00 -0800
From:   Andrii Nakryiko <andrii.nakryiko@...il.com>
To:     Hou Tao <houtao1@...wei.com>
Cc:     Alexei Starovoitov <ast@...nel.org>,
        Martin KaFai Lau <kafai@...com>, Yonghong Song <yhs@...com>,
        Daniel Borkmann <daniel@...earbox.net>,
        Andrii Nakryiko <andrii@...nel.org>,
        "David S . Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Networking <netdev@...r.kernel.org>, bpf <bpf@...r.kernel.org>
Subject: Re: [PATCH bpf-next] selftests/bpf: use getpagesize() to initialize
 ring buffer size

On Wed, Jan 26, 2022 at 6:34 PM Hou Tao <houtao1@...wei.com> wrote:
>
> 4096 is OK for x86-64, but for other archs with greater than 4KB
> page size (e.g. 64KB under arm64), test_verifier for test case
> "check valid spill/fill, ptr to mem" will fail, so just use
> getpagesize() to initialize the ring buffer size. Do this for
> test_progs as well.
>
> Signed-off-by: Hou Tao <houtao1@...wei.com>
> ---
>  tools/testing/selftests/bpf/prog_tests/d_path.c | 14 ++++++++++++--
>  .../testing/selftests/bpf/prog_tests/test_ima.c | 17 +++++++++++++----
>  tools/testing/selftests/bpf/progs/ima.c         |  1 -
>  .../bpf/progs/test_d_path_check_types.c         |  1 -
>  tools/testing/selftests/bpf/test_verifier.c     |  2 +-
>  5 files changed, 26 insertions(+), 9 deletions(-)
>

[...]

> @@ -86,5 +94,6 @@ void test_test_ima(void)
>         CHECK(err, "failed to run command", "%s, errno = %d\n", cmd, errno);
>  close_prog:
>         ring_buffer__free(ringbuf);
> +destroy_skel:
>         ima__destroy(skel);
>  }
> diff --git a/tools/testing/selftests/bpf/progs/ima.c b/tools/testing/selftests/bpf/progs/ima.c
> index 96060ff4ffc6..e192a9f16aea 100644
> --- a/tools/testing/selftests/bpf/progs/ima.c
> +++ b/tools/testing/selftests/bpf/progs/ima.c
> @@ -13,7 +13,6 @@ u32 monitored_pid = 0;
>
>  struct {
>         __uint(type, BPF_MAP_TYPE_RINGBUF);
> -       __uint(max_entries, 1 << 12);

Should we just bump it to 64/128/256KB instead? It's quite annoying to
do a split open and then load just due to this...

I'm also wondering if we should either teach kernel to round up to
closes power-of-2 of page_size internally, or teach libbpf to do this
for RINGBUF maps. Thoughts?


>  } ringbuf SEC(".maps");
>
>  char _license[] SEC("license") = "GPL";
> diff --git a/tools/testing/selftests/bpf/progs/test_d_path_check_types.c b/tools/testing/selftests/bpf/progs/test_d_path_check_types.c
> index 7e02b7361307..1b68d4a65abb 100644
> --- a/tools/testing/selftests/bpf/progs/test_d_path_check_types.c
> +++ b/tools/testing/selftests/bpf/progs/test_d_path_check_types.c
> @@ -8,7 +8,6 @@ extern const int bpf_prog_active __ksym;
>
>  struct {
>         __uint(type, BPF_MAP_TYPE_RINGBUF);
> -       __uint(max_entries, 1 << 12);
>  } ringbuf SEC(".maps");
>
>  SEC("fentry/security_inode_getattr")
> diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
> index 29bbaa58233c..6acb5e747715 100644
> --- a/tools/testing/selftests/bpf/test_verifier.c
> +++ b/tools/testing/selftests/bpf/test_verifier.c
> @@ -931,7 +931,7 @@ static void do_test_fixup(struct bpf_test *test, enum bpf_prog_type prog_type,
>         }
>         if (*fixup_map_ringbuf) {
>                 map_fds[20] = create_map(BPF_MAP_TYPE_RINGBUF, 0,
> -                                          0, 4096);
> +                                          0, getpagesize());
>                 do {
>                         prog[*fixup_map_ringbuf].imm = map_fds[20];
>                         fixup_map_ringbuf++;
> --
> 2.29.2
>