[PATCH] selftests/bpf: improve file descriptor closure handling

Ma Ke posted 1 patch 1 year, 5 months ago
tools/testing/selftests/bpf/prog_tests/fexit_stress.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
[PATCH] selftests/bpf: improve file descriptor closure handling
Posted by Ma Ke 1 year, 5 months ago
serial_test_fexit_stress() has a non-robust handling of file descriptor
closure. If an error occurs, the function may exit without closing open
file descriptors, potentially causing resource leaks.

Fix the issue by closing file descriptors in reverse order and starting
from the last opened. Ensure proper closure even if an error occurs early.

Fixes: 8fb9fb2f1728 ("selftests/bpf: Query BPF_MAX_TRAMP_LINKS using BTF")
Signed-off-by: Ma Ke <make24@iscas.ac.cn>
---
 tools/testing/selftests/bpf/prog_tests/fexit_stress.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/fexit_stress.c b/tools/testing/selftests/bpf/prog_tests/fexit_stress.c
index 596536def43d..b1980bd61583 100644
--- a/tools/testing/selftests/bpf/prog_tests/fexit_stress.c
+++ b/tools/testing/selftests/bpf/prog_tests/fexit_stress.c
@@ -49,11 +49,14 @@ void serial_test_fexit_stress(void)
 	ASSERT_OK(err, "bpf_prog_test_run_opts");
 
 out:
-	for (i = 0; i < bpf_max_tramp_links; i++) {
+	if (i >= bpf_max_tramp_links)
+		i = bpf_max_tramp_links - 1;
+	while (i >= 0) {
 		if (link_fd[i])
 			close(link_fd[i]);
 		if (fexit_fd[i])
 			close(fexit_fd[i]);
+		i--;
 	}
 	free(fd);
 }
-- 
2.25.1
Re: [PATCH] selftests/bpf: improve file descriptor closure handling
Posted by Andrii Nakryiko 1 year, 5 months ago
On Wed, Jun 26, 2024 at 5:19 AM Ma Ke <make24@iscas.ac.cn> wrote:
>
> serial_test_fexit_stress() has a non-robust handling of file descriptor
> closure. If an error occurs, the function may exit without closing open
> file descriptors, potentially causing resource leaks.
>
> Fix the issue by closing file descriptors in reverse order and starting
> from the last opened. Ensure proper closure even if an error occurs early.
>
> Fixes: 8fb9fb2f1728 ("selftests/bpf: Query BPF_MAX_TRAMP_LINKS using BTF")
> Signed-off-by: Ma Ke <make24@iscas.ac.cn>
> ---
>  tools/testing/selftests/bpf/prog_tests/fexit_stress.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>

there is no need for this change, I've applied your previous patch but
adjusted > 0 checks, see my comment there. That should be enough.

> diff --git a/tools/testing/selftests/bpf/prog_tests/fexit_stress.c b/tools/testing/selftests/bpf/prog_tests/fexit_stress.c
> index 596536def43d..b1980bd61583 100644
> --- a/tools/testing/selftests/bpf/prog_tests/fexit_stress.c
> +++ b/tools/testing/selftests/bpf/prog_tests/fexit_stress.c
> @@ -49,11 +49,14 @@ void serial_test_fexit_stress(void)
>         ASSERT_OK(err, "bpf_prog_test_run_opts");
>
>  out:
> -       for (i = 0; i < bpf_max_tramp_links; i++) {
> +       if (i >= bpf_max_tramp_links)
> +               i = bpf_max_tramp_links - 1;
> +       while (i >= 0) {
>                 if (link_fd[i])
>                         close(link_fd[i]);
>                 if (fexit_fd[i])
>                         close(fexit_fd[i]);
> +               i--;
>         }
>         free(fd);
>  }
> --
> 2.25.1
>