> diff --git a/tools/testing/selftests/bpf/prog_tests/btf_module_names.c
> b/tools/testing/selftests/bpf/prog_tests/btf_module_names.c
> new file mode 100644
> index 000000000000..d5674cad0017
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/btf_module_names.c
> @@ -0,0 +1,93 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include <test_progs.h>
> +#include "btf_module_names.skel.h"
> +
> +static void btf_module_names_load(void)
> +{
> + struct btf_module_names *skel = NULL;
> + int ret;
> + static const char *mod_names[] = { "bpf_testmod" };
> +
> + LIBBPF_OPTS(bpf_object_open_opts, opts,
> + .btf_module_names = mod_names,
> + .nr_btf_module_names = 1,
> + );
> +
> + skel = btf_module_names__open_opts(&opts);
> + if (!ASSERT_OK_PTR(skel, "btf_module_names__open_opts"))
> + goto out;
> +
> + ret = btf_module_names__load(skel);
> + ASSERT_OK(ret, "btf_module_names__load");
The three subtests (btf_module_names_load, btf_module_names_skip, and
btf_module_names_empty) leave the input-validation paths from the
preceding commit completely unexercised.
That commit adds checks in bpf_object_init_btf_module_names() that are
documented in libbpf.h as part of the API contract, particularly:
"The list must not contain duplicate entries; otherwise -EINVAL is
returned."
Uncovered paths in bpf_object_init_btf_module_names():
- Empty or NULL entry detection (if (!names[i] || !names[i][0]))
- Duplicate name detection (the for loop with strcmp checking all
previous entries)
- Error cleanup path (err_out:) that calls
bpf_object_free_btf_module_names() after partial strdup() walk
Would it make sense to add subtests covering these validation paths? A
single extra __open_opts() call checked with ASSERT_ERR_PTR() or
ASSERT_NULL() would exercise each. The duplicate-detection loop is
non-trivial enough to deserve one.
Also, nothing in the series asserts the default btf_module_names == NULL
case (load everything, unchanged behaviour), so a regression that makes
the option mandatory would pass undetected.
> +out:
> + btf_module_names__destroy(skel);
> +}
> +
> +/*
> + * Verify that an unrequested module BTF is skipped. The BPF program
> + * requires the BTF of bpf_testmod, but bpf_testmod is not specified in
> + * btf_module_names, so its BTF is skipped and the BPF program fails to load.
> + */
> +static void btf_module_names_skip(void)
> +{
> + struct btf_module_names *skel = NULL;
> + int ret;
> + static const char *mod_names[] = { "module_nonexist" };
> +
> + LIBBPF_OPTS(bpf_object_open_opts, opts,
> + .btf_module_names = mod_names,
> + .nr_btf_module_names = 1,
> + );
> +
> + skel = btf_module_names__open_opts(&opts);
> + if (!ASSERT_OK_PTR(skel, "btf_module_names__open_opts"))
> + goto out;
> +
> + ret = btf_module_names__load(skel);
> + ASSERT_ERR(ret, "btf_module_names__load");
A subsystem pattern flags this as potentially concerning: both negative
subtests assert only that the load failed, not the specific reason why.
ASSERT_ERR expands to check ___res < 0, so btf_module_names_skip() and
btf_module_names_empty() pass for any negative return from
btf_module_names__load().
The intended error is specific: with bpf_testmod's BTF filtered out,
find_kernel_btf_id() walks obj->btf_modules and falls through to return
-ESRCH, which bpf_object_load() propagates to __load(). Asserting that
value (for example, ASSERT_EQ(ret, -ESRCH, ...)) would pin the
behaviour.
As written, several unrelated conditions make both subtests report
success while proving nothing about the new filter:
- CONFIG_DEBUG_INFO_BTF_MODULES=n: bpf_testmod loads (so env.has_testmod
is true and the test is not skipped) but has no BTF at all.
Attach-target resolution then fails regardless of btf_module_names, so
both negative subtests pass vacuously.
- Unprivileged/EPERM: load_module_btfs() returns 0 early without loading
any module BTF (pr_debug skipping message), again yielding -ESRCH from
an unrelated cause.
- Any future validation of the new opts that rejects the list at load
time rather than at open time.
Would pinning the expected -ESRCH value make these subtests more robust?
> +
> +out:
> + btf_module_names__destroy(skel);
> +}
[ ... ]
---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/32730521630
