Re: [PATCH bpf-next v2 2/4] selftests/bpf: Add assertions for bpf_program__{add,clear}_flags()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Missing early exit check on BPF skeleton load and flag manipulation operations allows execution to proceed in an invalid state.
--

commit 1694db036efd5301fa9d3722ffe0c0854fa291b9
Author: Toke Høiland-Jørgensen <toke@redhat.com>

selftests/bpf: Add assertions for bpf_program__{add,clear}_flags()
    
Add assertions that round-tripping through bpf_program__add_flags() and
bpf_program__clear_flags() ends up with the original flags value.
Arbitrarily add these to the kernel_flag test prog since that's where
we're already checking for the bpf_program__flags() value (and the test
name contains "flag").

> diff --git a/tools/testing/selftests/bpf/prog_tests/kernel_flag.c b/tools/testing/selftests/bpf/prog_tests/kernel_flag.c
> index 25eb59f460abe..72c0ef8da409e 100644
> --- a/tools/testing/selftests/bpf/prog_tests/kernel_flag.c
> +++ b/tools/testing/selftests/bpf/prog_tests/kernel_flag.c
> @@ -10,15 +10,31 @@ void test_kernel_flag(void)

[ ... ]

> +	ret = bpf_program__add_flags(prog, BPF_F_ANY_ALIGNMENT);
> +	ASSERT_OK(ret, "bpf_program__add_flags ret");

[Severity: Low]
Should this ASSERT_OK() call be wrapped in an if (!...) goto close_prog;
check?  If it fails, the test will continue executing instead of cleanly
aborting.

> +	ASSERT_EQ(bpf_program__flags(prog), flags | BPF_F_ANY_ALIGNMENT,
> +		  "bpf_program__add_flags value");
> +
> +	ret = bpf_program__clear_flags(prog, BPF_F_ANY_ALIGNMENT);
> +	ASSERT_OK(ret, "bpf_program__clear_flags ret");
> +	ASSERT_EQ(bpf_program__flags(prog), flags, "bpf_program__clear_flags value");
> +
> +	ret = test_kernel_flag__load(lsm_skel);
> +	ASSERT_OK(ret, "test_kernel_flag__load");

[Severity: Low]
Is it intentional to allow execution to proceed if the skeleton fails to
load?  Wrapping this in if (!ASSERT_OK(ret, ...)) goto close_prog; would
abort the test cleanly and avoid cascading test failures.

> +
>  	lsm_skel->bss->monitored_tid = sys_gettid();
>  
>  	ret = test_kernel_flag__attach(lsm_skel);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260910183222.307208-1-toke@redhat.com?part=2