On Mon, Mar 4, 2024 at 2:52 PM Eduard Zingerman <eddyz87@xxxxxxxxx> wrote:
>
> When loading struct_ops programs kernel requires BTF id of the
> struct_ops type and member index for attachment point inside that
> type. This makes it not possible to have same BPF program used in
> struct_ops maps that have different struct_ops type.
> Check if libbpf rejects such BPF objects files.
>
> Signed-off-by: Eduard Zingerman <eddyz87@xxxxxxxxx>
> ---
> .../selftests/bpf/bpf_testmod/bpf_testmod.c | 24 +++++++++++++
> .../selftests/bpf/bpf_testmod/bpf_testmod.h | 4 +++
> .../selftests/bpf/prog_tests/bad_struct_ops.c | 35 +++++++++++++++++++
> .../selftests/bpf/progs/bad_struct_ops.c | 25 +++++++++++++
> 4 files changed, 88 insertions(+)
> create mode 100644 tools/testing/selftests/bpf/prog_tests/bad_struct_ops.c
> create mode 100644 tools/testing/selftests/bpf/progs/bad_struct_ops.c
>
CI reports kernel crashes, so please check that, but overall things look good:
Acked-by: Andrii Nakryiko <andrii@xxxxxxxxxx>
> diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
> index b9ff88e3d463..2de7e80dbb4b 100644
> --- a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
> +++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
> @@ -610,6 +610,29 @@ struct bpf_struct_ops bpf_bpf_testmod_ops = {
> .owner = THIS_MODULE,
> };
>
[...]
> diff --git a/tools/testing/selftests/bpf/prog_tests/bad_struct_ops.c b/tools/testing/selftests/bpf/prog_tests/bad_struct_ops.c
> new file mode 100644
> index 000000000000..9f5dbefa0dd9
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/bad_struct_ops.c
> @@ -0,0 +1,35 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <test_progs.h>
> +#include "bad_struct_ops.skel.h"
> +
> +static void invalid_prog_reuse(void)
> +{
> + struct bad_struct_ops *skel;
> + char *log = NULL;
> + int err;
> +
> + skel = bad_struct_ops__open();
> + if (!ASSERT_OK_PTR(skel, "bad_struct_ops__open"))
> + return;
> +
> + if (start_libbpf_log_capture())
> + goto cleanup;
> +
> + err = bad_struct_ops__load(skel);
> + log = stop_libbpf_log_capture();
> + ASSERT_ERR(err, "bad_struct_ops__load should fail");
second argument isn't actually a test description, it's meant to be
used as a (relatively) short identifier of *what* is being checked
(see how ASSERT_xxx() macro are using them)
> + ASSERT_HAS_SUBSTR(log,
> + "struct_ops init_kern testmod_2 func ptr test_1: invalid reuse of prog test_1",
> + "expected init_kern message");
> +
> +cleanup:
> + free(log);
> + bad_struct_ops__destroy(skel);
> +}
> +
> +void test_bad_struct_ops(void)
> +{
> + if (test__start_subtest("invalid_prog_reuse"))
> + invalid_prog_reuse();
> +}
[...]
