All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Andrey Grodzovsky" <andrey.grodzovsky@crowdstrike.com>
Cc: bpf@vger.kernel.org
Subject: Re: [PATCH bpf-next v3 5/6] selftests/bpf: Cover BPF program load strategy transitions
Date: Thu, 17 Sep 2026 20:39:19 +0000	[thread overview]
Message-ID: <20260917203920.433F51F000FF@smtp.kernel.org> (raw)
In-Reply-To: <20260917203040.3150212-6-andrey.grodzovsky@crowdstrike.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] bpf: bpf_link leaked on selftest error path
--

commit 82f598b63abc74f2a315872de7cd207f7e441280
Author: Slava Imameev <slava.imameev@crowdstrike.com>

selftests/bpf: Cover BPF program load strategy transitions

This commit adds a load_type test to cover BPF program load strategy
transitions. It verifies transitions between DISABLED, AUTO, and MANUAL
strategies, as well as the behavior of autoattach.

> diff --git a/tools/testing/selftests/bpf/prog_tests/load_type.c b/tools/testing/selftests/bpf/prog_tests/load_type.c
> new file mode 100644
> index 0000000000000..55ca53685863c
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/load_type.c
> @@ -0,0 +1,157 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <test_progs.h>
> +#include <time.h>
> +#include "test_load_type.skel.h"
> +
> +void test_load_type(void)
> +{
> +	struct bpf_link *link;
> +	struct test_load_type *skel;

[ ... ]

> +	/* attach prog3 */
> +	link = bpf_program__attach(skel->progs.prog3);
> +	if (!ASSERT_OK_PTR(link, "attach"))
> +		goto cleanup;
> +
> +	usleep(1);
> +
> +	if (!ASSERT_TRUE(skel->bss->prog3_called, "prog3_called_again"))
> +		goto cleanup;

[Severity: Medium]
Will this leak the bpf_link if the assertion fails?

When the test assertion fails, it jumps to the cleanup block in
test_load_type(), skipping the explicit bpf_link__destroy() call. Because
test_load_type__destroy() doesn't clean up manually attached programs,
the link will be leaked.

Also, if you choose to fix this by adding bpf_link__destroy() in the
cleanup block, it looks like the link variable is uninitialized at declaration,
so it might be necessary to initialize it to NULL as well.

> +
> +	/* detach prog3 as test_load_type__destroy doesn't detach manually loaded programs */
> +	err = bpf_link__destroy(link);
> +	ASSERT_OK(err, "link_destroy");
> +
> +cleanup:
> +	test_load_type__destroy(skel);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260917203040.3150212-1-andrey.grodzovsky@crowdstrike.com?part=5

  reply	other threads:[~2026-09-17 20:39 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-17 20:30 [PATCH bpf-next v3 0/6] libbpf: BPF program manual loading Andrey Grodzovsky
2026-09-17 20:30 ` [PATCH bpf-next v3 1/6] libbpf: BPF program load strategy enum Andrey Grodzovsky
2026-09-17 21:16   ` bot+bpf-ci
2026-09-17 20:30 ` [PATCH bpf-next v3 2/6] libbpf: BPF programs manual loading and attaching Andrey Grodzovsky
2026-09-17 20:41   ` sashiko-bot
2026-09-17 21:34   ` bot+bpf-ci
2026-09-17 20:30 ` [PATCH bpf-next v3 3/6] libbpf: Support declarative manual load via SEC("!...") prefix Andrey Grodzovsky
2026-09-17 21:34   ` bot+bpf-ci
2026-09-17 20:30 ` [PATCH bpf-next v3 4/6] libbpf: Reject gen_loader for objects with already-manual programs Andrey Grodzovsky
2026-09-17 21:16   ` bot+bpf-ci
2026-09-17 20:30 ` [PATCH bpf-next v3 5/6] selftests/bpf: Cover BPF program load strategy transitions Andrey Grodzovsky
2026-09-17 20:39   ` sashiko-bot [this message]
2026-09-17 21:34   ` bot+bpf-ci
2026-09-17 20:30 ` [PATCH bpf-next v3 6/6] selftests/bpf: Cover BPF program manual loading Andrey Grodzovsky
2026-09-17 20:40   ` sashiko-bot
2026-09-17 21:34   ` bot+bpf-ci

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260917203920.433F51F000FF@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=andrey.grodzovsky@crowdstrike.com \
    --cc=bpf@vger.kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.