Re: [PATCH bpf-next v1 10/14] selftests/bpf: Fix resource leaks caused by missing cleanups

[Eduard Zingerman]

On Wed, 2026-02-11 at 17:13 -0800, Ihor Solodrai wrote:
> ASAN reported a number of resource leaks:
>   - Add missing  *__destroy(skel) calls
>   - Replace bpf_link__detach() with bpf_link__destroy() where appropriate
>   - cgrp_local_storage: Add bpf_link__destroy() when bpf_iter_create fails
>   - dynptr: Add missing bpf_object__close()
> 
> Signed-off-by: Ihor Solodrai <ihor.solodrai@xxxxxxxxx>
> ---

Acked-by: Eduard Zingerman <eddyz87@xxxxxxxxx>

[...]

> diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
> b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
> index 256707e7d20d..ce010602a443 100644
> --- a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
> +++ b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c

[...]

> @@ -763,12 +763,15 @@ static void test_sockmap_unconnected_unix(void)
>  	map = bpf_map__fd(skel->maps.sock_map_rx);
>  
>  	stream = xsocket(AF_UNIX, SOCK_STREAM, 0);
> -	if (stream < 0)
> +	if (stream < 0) {
> +		test_sockmap_pass_prog__destroy(skel);
>  		return;
> +	}
>  
>  	dgram = xsocket(AF_UNIX, SOCK_DGRAM, 0);
>  	if (dgram < 0) {
>  		close(stream);
> +		test_sockmap_pass_prog__destroy(skel);
>  		return;
>  	}

Nit: initialize `stream` and `dgram` as -1 and add an exit label?

> @@ -780,6 +783,7 @@ static void test_sockmap_unconnected_unix(void)
>  
>  	close(stream);
>  	close(dgram);
> +	test_sockmap_pass_prog__destroy(skel);
>  }

[...]

> diff --git a/tools/testing/selftests/bpf/prog_tests/struct_ops_private_stack.c
> b/tools/testing/selftests/bpf/prog_tests/struct_ops_private_stack.c
> index 4006879ca3fe..c36aa2354d2f 100644
> --- a/tools/testing/selftests/bpf/prog_tests/struct_ops_private_stack.c
> +++ b/tools/testing/selftests/bpf/prog_tests/struct_ops_private_stack.c
> @@ -56,6 +56,7 @@ static void test_private_stack_fail(void)
>  	err = struct_ops_private_stack_fail__load(skel);
>  	if (!ASSERT_ERR(err, "struct_ops_private_stack_fail__load"))
>  		goto cleanup;
> +	struct_ops_private_stack_fail__destroy(skel);
>  	return;

Nit: just remove 'return' instead?
     `cleanup` already calls `destroy`.

>  
>  cleanup:
> diff --git a/tools/testing/selftests/bpf/prog_tests/tc_opts.c
> b/tools/testing/selftests/bpf/prog_tests/tc_opts.c
> index dd7a138d8c3d..2a4fd367da0a 100644
> --- a/tools/testing/selftests/bpf/prog_tests/tc_opts.c
> +++ b/tools/testing/selftests/bpf/prog_tests/tc_opts.c
> @@ -1363,6 +1363,7 @@ static void test_tc_opts_dev_cleanup_target(int target)
>  	ASSERT_OK(system("ip link del dev tcx_opts1"), "del veth");
>  	ASSERT_EQ(if_nametoindex("tcx_opts1"), 0, "dev1_removed");
>  	ASSERT_EQ(if_nametoindex("tcx_opts2"), 0, "dev2_removed");
> +	test_tc_link__destroy(skel);

Nit: this is exact copy of the `cleanup` label below.
     One can replace the three ASSERTs and the `destroy` call
     with `goto cleanup`.


[...]