Re: [PATCH net-next 4/5] selftests/bpf: check XDP attach on a nested bond slave

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] selftests/bpf: false-positive test success due to ASSERT_GE(nest2, 0)
--

commit 342ea5a121a99c07f3e0bd7119d434dddb04cb6e
Author: Jakub Kicinski <kuba@kernel.org>

selftests/bpf: check XDP attach on a nested bond slave

This commit adds testing to verify that attaching a BPF program to a nested
slave device correctly fails when the master device already has a program
attached. It retrieves the interface index for "bond_nest2" and asserts that
attaching to it returns an error.

> diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_bonding.c b/tools/testing/selftests/bpf/prog_tests/xdp_bonding.c
> index c42488e445c28..486e0f9a2adac 100644
> --- a/tools/testing/selftests/bpf/prog_tests/xdp_bonding.c
> +++ b/tools/testing/selftests/bpf/prog_tests/xdp_bonding.c
[ ... ]
> @@ -488,10 +489,23 @@ static void test_xdp_bonding_nested(struct skeletons *skeletons)
>  	if (!ASSERT_OK(err, "set bond_nest2 master"))
>  		goto out;
>  
> +	nest2 = if_nametoindex("bond_nest2");
> +	if (!ASSERT_GE(nest2, 0, "if_nametoindex bond_nest2"))
> +		goto out;
> +

[Severity: Medium]
Will this assertion accept an invalid interface index if if_nametoindex()
fails?

If if_nametoindex("bond_nest2") fails, it returns 0. The ASSERT_GE check
accepts 0, which then propagates down to bpf_program__attach_xdp().

>  	link = bpf_program__attach_xdp(skeletons->xdp_dummy->progs.xdp_dummy_prog, bond);
> -	ASSERT_OK_PTR(link, "attach program to master");
> +	if (!ASSERT_OK_PTR(link, "attach program to master"))
> +		goto out;
> +
> +	/* Attaching to a nested slave is not allowed either. Only the direct
> +	 * upper is consulted, so this only holds if every device the program
> +	 * was propagated to records it.
> +	 */
> +	link2 = bpf_program__attach_xdp(skeletons->xdp_dummy->progs.xdp_dummy_prog, nest2);
> +	ASSERT_ERR_PTR(link2, "attach program to nested slave when master has program");

[Severity: Medium]
Could a failure to resolve the interface index result in a false-positive test
success here?

If nest2 is 0 from a previous if_nametoindex() failure,
bpf_program__attach_xdp() will predictably fail. Because the test expects
attachment to fail for nesting restrictions, the expected-failure assertion
is satisfied, which could silently mask test environment setup failures.

>  
>  out:
> +	bpf_link__destroy(link2);
>  	bpf_link__destroy(link);
>  	system("ip link del bond");
>  	system("ip link del bond_nest1");

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260928223648.2739371-1-kuba@kernel.org?part=4