Separate tests that need error injection

[kernel-patches-daemon-bpf]

Pull request for series with
subject: Separate tests that need error injection
version: 2
url: https://patchwork.kernel.org/project/netdevbpf/list/?series=1052739

[kernel-patches-daemon-bpf]

Upstream branch: db975de
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1052739
version: 2

[kernel-patches-review-bot]

commit 4da14b5f0577225f677d0a563ace14698dae9dd5
Author: Viktor Malik <vmalik@redhat.com>

selftests/bpf: Split module_attach into subtests

This commit refactors the module_attach test to split it into individual
subtests, allowing selective test skipping when certain hooks are not
supported on a kernel.

> diff --git a/tools/testing/selftests/bpf/prog_tests/module_attach.c b/tools/testing/selftests/bpf/prog_tests/module_attach.c
> index 70fa7ae93173..8668c26c202f 100644
> --- a/tools/testing/selftests/bpf/prog_tests/module_attach.c
> +++ b/tools/testing/selftests/bpf/prog_tests/module_attach.c

[ ... ]

> +static void test_module_attach_prog(const char *prog_name, int sz,
> +				    const char *attach_target, int ret)
> +{
> +	struct test_module_attach *skel;
> +	struct bpf_program *prog;
> +	int err;
> +
> +	skel = test_module_attach__open();
> +	if (CHECK(!skel, "skel_open", "failed to open skeleton\n"))
> +		return;
> +
> +	prog = bpf_object__find_program_by_name(skel->obj, prog_name);
> +	if (!ASSERT_OK_PTR(prog, "find program"))
> +		goto cleanup;
> +	bpf_program__set_autoload(prog, true);
> +
> +	if (attach_target) {
> +		err = bpf_program__set_attach_target(prog, 0, attach_target);
> +		ASSERT_OK(err, attach_target);
> +	}
> +
> +	err = test_module_attach__load(skel);
> +	if (CHECK(err, "skel_load", "failed to load skeleton\n"))
> +		return;

Shouldn't this be "goto cleanup" instead of "return"?  The skeleton was
successfully opened and will leak if load fails.

[ ... ]

> +static void test_module_attach_writable(void)
> +{
> +	struct test_module_attach__bss *bss;
> +	struct test_module_attach *skel;
> +	struct bpf_program *prog;
> +	int writable_val = 0;
> +	int err;
> +
> +	skel = test_module_attach__open();
> +	if (CHECK(!skel, "skel_open", "failed to open skeleton\n"))
> +		return;
> +
> +	prog = bpf_object__find_program_by_name(skel->obj,
> +						"handle_raw_tp_writable_bare");
> +	if (!ASSERT_OK_PTR(prog, "find program"))
> +		goto cleanup;
> +	bpf_program__set_autoload(prog, true);
> +
> +	err = test_module_attach__load(skel);
> +	if (CHECK(err, "skel_load", "failed to load skeleton\n"))
> +		return;

Same issue here - should be "goto cleanup" to avoid leaking the skeleton.

[ ... ]

---

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

In-Reply-To-Subject: selftests/bpf: Split module_attach into subtests
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/21868797729

AI-authorship-score: low
AI-authorship-explanation: The code follows standard BPF selftest patterns with no signs of AI generation; the inconsistent error handling suggests human oversight rather than AI template output.
issues-found: 2
issue-severity-score: low
issue-severity-explanation: Skeleton memory leak in test code on error paths; affects only test cleanup, not production kernel code.

[kernel-patches-daemon-bpf]

Forwarding comment 3878063025 via email
In-Reply-To: 699a95dfb6493cfd588524f388214db1250518f3.1770732776.git.vmalik@redhat.com
Patch: https://patchwork.kernel.org/project/netdevbpf/patch/699a95dfb6493cfd588524f388214db1250518f3.1770732776.git.vmalik@redhat.com/