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  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]
Date:   Thu, 10 Sep 2020 12:59:33 -0700
From:   Andrii Nakryiko <andrii.nakryiko@...il.com>
To:     Stanislav Fomichev <sdf@...gle.com>
Cc:     Networking <netdev@...r.kernel.org>, bpf <bpf@...r.kernel.org>,
        "David S. Miller" <davem@...emloft.net>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        YiFei Zhu <zhuyifei@...gle.com>,
        YiFei Zhu <zhuyifei1999@...il.com>
Subject: Re: [PATCH bpf-next v4 5/5] selftests/bpf: Test load and dump
 metadata with btftool and skel

On Wed, Sep 9, 2020 at 11:25 AM Stanislav Fomichev <sdf@...gle.com> wrote:
>
> From: YiFei Zhu <zhuyifei@...gle.com>
>
> This is a simple test to check that loading and dumping metadata
> in btftool works, whether or not metadata contents are used by the
> program.
>
> A C test is also added to make sure the skeleton code can read the
> metadata values.
>
> Cc: YiFei Zhu <zhuyifei1999@...il.com>
> Signed-off-by: YiFei Zhu <zhuyifei@...gle.com>
> Signed-off-by: Stanislav Fomichev <sdf@...gle.com>
> ---

It would be good to test that libbpf does bind .rodata even if BPF
program doesn't use it. So for metadata_unused you can get
bpf_prog_info and check that it does contain the id of .rodata?

>  tools/testing/selftests/bpf/Makefile          |  3 +-
>  .../selftests/bpf/prog_tests/metadata.c       | 81 ++++++++++++++++++
>  .../selftests/bpf/progs/metadata_unused.c     | 15 ++++
>  .../selftests/bpf/progs/metadata_used.c       | 15 ++++
>  .../selftests/bpf/test_bpftool_metadata.sh    | 82 +++++++++++++++++++
>  5 files changed, 195 insertions(+), 1 deletion(-)
>  create mode 100644 tools/testing/selftests/bpf/prog_tests/metadata.c
>  create mode 100644 tools/testing/selftests/bpf/progs/metadata_unused.c
>  create mode 100644 tools/testing/selftests/bpf/progs/metadata_used.c
>  create mode 100755 tools/testing/selftests/bpf/test_bpftool_metadata.sh
>
> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> index 65d3d9aaeb31..3c92db8a189a 100644
> --- a/tools/testing/selftests/bpf/Makefile
> +++ b/tools/testing/selftests/bpf/Makefile
> @@ -68,7 +68,8 @@ TEST_PROGS := test_kmod.sh \
>         test_tc_edt.sh \
>         test_xdping.sh \
>         test_bpftool_build.sh \
> -       test_bpftool.sh
> +       test_bpftool.sh \
> +       test_bpftool_metadata.sh \
>
>  TEST_PROGS_EXTENDED := with_addr.sh \
>         with_tunnels.sh \
> diff --git a/tools/testing/selftests/bpf/prog_tests/metadata.c b/tools/testing/selftests/bpf/prog_tests/metadata.c
> new file mode 100644
> index 000000000000..dea8fa86b5fb
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/metadata.c
> @@ -0,0 +1,81 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +/*
> + * Copyright 2020 Google LLC.
> + */
> +
> +#include <test_progs.h>
> +#include <cgroup_helpers.h>
> +#include <network_helpers.h>
> +
> +#include "metadata_unused.skel.h"
> +#include "metadata_used.skel.h"
> +
> +static int duration;
> +
> +static void test_metadata_unused(void)
> +{
> +       struct metadata_unused *obj;
> +       int err;
> +
> +       obj = metadata_unused__open_and_load();
> +       if (CHECK(!obj, "skel-load", "errno %d", errno))
> +               return;
> +
> +       /* Assert that we can access the metadata in skel and the values are
> +        * what we expect.
> +        */
> +       if (CHECK(strncmp(obj->rodata->bpf_metadata_a, "foo",
> +                         sizeof(obj->rodata->bpf_metadata_a)),
> +                 "bpf_metadata_a", "expected \"foo\", value differ"))
> +               goto close_bpf_object;
> +       if (CHECK(obj->rodata->bpf_metadata_b != 1, "bpf_metadata_b",
> +                 "expected 1, got %d", obj->rodata->bpf_metadata_b))
> +               goto close_bpf_object;
> +
> +       /* Assert that binding metadata map to prog again succeeds. */
> +       err = bpf_prog_bind_map(bpf_program__fd(obj->progs.prog),
> +                               bpf_map__fd(obj->maps.rodata), NULL);
> +       CHECK(err, "rebind_map", "errno %d, expected 0", errno);
> +
> +close_bpf_object:
> +       metadata_unused__destroy(obj);
> +}
> +
> +static void test_metadata_used(void)
> +{
> +       struct metadata_used *obj;
> +       int err;
> +
> +       obj = metadata_used__open_and_load();
> +       if (CHECK(!obj, "skel-load", "errno %d", errno))
> +               return;
> +
> +       /* Assert that we can access the metadata in skel and the values are
> +        * what we expect.
> +        */
> +       if (CHECK(strncmp(obj->rodata->bpf_metadata_a, "bar",
> +                         sizeof(obj->rodata->bpf_metadata_a)),
> +                 "metadata_a", "expected \"bar\", value differ"))
> +               goto close_bpf_object;
> +       if (CHECK(obj->rodata->bpf_metadata_b != 2, "metadata_b",
> +                 "expected 2, got %d", obj->rodata->bpf_metadata_b))
> +               goto close_bpf_object;
> +
> +       /* Assert that binding metadata map to prog again succeeds. */
> +       err = bpf_prog_bind_map(bpf_program__fd(obj->progs.prog),
> +                               bpf_map__fd(obj->maps.rodata), NULL);
> +       CHECK(err, "rebind_map", "errno %d, expected 0", errno);
> +
> +close_bpf_object:
> +       metadata_used__destroy(obj);
> +}
> +
> +void test_metadata(void)
> +{
> +       if (test__start_subtest("unused"))
> +               test_metadata_unused();
> +
> +       if (test__start_subtest("used"))
> +               test_metadata_used();
> +}
> diff --git a/tools/testing/selftests/bpf/progs/metadata_unused.c b/tools/testing/selftests/bpf/progs/metadata_unused.c
> new file mode 100644
> index 000000000000..db5b804f6f4c
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/metadata_unused.c
> @@ -0,0 +1,15 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +#include <linux/bpf.h>
> +#include <bpf/bpf_helpers.h>
> +
> +const char bpf_metadata_a[] SEC(".rodata") = "foo";
> +const int bpf_metadata_b SEC(".rodata") = 1;

please add volatile to ensure these are not optimized away

> +
> +SEC("cgroup_skb/egress")
> +int prog(struct xdp_md *ctx)
> +{
> +       return 0;
> +}
> +
> +char _license[] SEC("license") = "GPL";
> diff --git a/tools/testing/selftests/bpf/progs/metadata_used.c b/tools/testing/selftests/bpf/progs/metadata_used.c
> new file mode 100644
> index 000000000000..0dcb1ba2f0ae
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/metadata_used.c
> @@ -0,0 +1,15 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +#include <linux/bpf.h>
> +#include <bpf/bpf_helpers.h>
> +
> +const char bpf_metadata_a[] SEC(".rodata") = "bar";
> +const int bpf_metadata_b SEC(".rodata") = 2;

same about volatile

> +
> +SEC("cgroup_skb/egress")
> +int prog(struct xdp_md *ctx)
> +{
> +       return bpf_metadata_b ? 1 : 0;
> +}
> +
> +char _license[] SEC("license") = "GPL";

[...]

Powered by blists - more mailing lists