netdev - Re: [PATCH bpf-next v2 20/20] tools/bpf: selftests: add bpf

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: <CAEf4BzZ7Jx9ZkHbRpj4Nzy1nJLhLaUoX6MTiTKvrLO2zPKFrBg@mail.gmail.com>
Date:   Tue, 5 May 2020 23:39:44 -0700
From:   Andrii Nakryiko <andrii.nakryiko@...il.com>
To:     Yonghong Song <yhs@...com>
Cc:     Andrii Nakryiko <andriin@...com>, bpf <bpf@...r.kernel.org>,
        Martin KaFai Lau <kafai@...com>,
        Networking <netdev@...r.kernel.org>,
        Alexei Starovoitov <ast@...com>,
        Daniel Borkmann <daniel@...earbox.net>,
        Kernel Team <kernel-team@...com>
Subject: Re: [PATCH bpf-next v2 20/20] tools/bpf: selftests: add bpf_iter selftests

On Sun, May 3, 2020 at 11:26 PM Yonghong Song <yhs@...com> wrote:
>
> The added test includes the following subtests:
>   - test verifier change for btf_id_or_null
>   - test load/create_iter/read for
>     ipv6_route/netlink/bpf_map/task/task_file
>   - test anon bpf iterator
>   - test anon bpf iterator reading one char at a time
>   - test file bpf iterator
>   - test overflow (single bpf program output not overflow)
>   - test overflow (single bpf program output overflows)
>
> Th ipv6_route tests the following verifier change
>   - access fields in the variable length array of the structure.
>
> The netlink load tests th following verifier change
>   - put a btf_id ptr value in a stack and accessible to
>     tracing/iter programs.
>
>   $ test_progs -n 2
>   #2/1 btf_id_or_null:OK
>   #2/2 ipv6_route:OK
>   #2/3 netlink:OK
>   #2/4 bpf_map:OK
>   #2/5 task:OK
>   #2/6 task_file:OK
>   #2/7 anon:OK
>   #2/8 anon-read-one-char:OK
>   #2/9 file:OK
>   #2/10 overflow:OK
>   #2/11 overflow-e2big:OK
>   #2 bpf_iter:OK
>   Summary: 1/11 PASSED, 0 SKIPPED, 0 FAILED
>
> Signed-off-by: Yonghong Song <yhs@...com>
> ---

Looks good overall. bpf_link__disconnect() is wrong, though, please
remove it. With that:

Acked-by: Andrii Nakryiko <andriin@...com>

>  .../selftests/bpf/prog_tests/bpf_iter.c       | 390 ++++++++++++++++++
>  .../selftests/bpf/progs/bpf_iter_test_kern1.c |   4 +
>  .../selftests/bpf/progs/bpf_iter_test_kern2.c |   4 +
>  .../selftests/bpf/progs/bpf_iter_test_kern3.c |  18 +
>  .../selftests/bpf/progs/bpf_iter_test_kern4.c |  48 +++
>  .../bpf/progs/bpf_iter_test_kern_common.h     |  22 +
>  6 files changed, 486 insertions(+)
>  create mode 100644 tools/testing/selftests/bpf/prog_tests/bpf_iter.c
>  create mode 100644 tools/testing/selftests/bpf/progs/bpf_iter_test_kern1.c
>  create mode 100644 tools/testing/selftests/bpf/progs/bpf_iter_test_kern2.c
>  create mode 100644 tools/testing/selftests/bpf/progs/bpf_iter_test_kern3.c
>  create mode 100644 tools/testing/selftests/bpf/progs/bpf_iter_test_kern4.c
>  create mode 100644 tools/testing/selftests/bpf/progs/bpf_iter_test_kern_common.h
>

[...]

> +
> +free_link:
> +       bpf_link__disconnect(link);

bpf_link__disconnect() actually will make destroy() below not close
link. So no need for it. Same below in few places.

> +       bpf_link__destroy(link);
> +}
> +

[...]