* [PATCH] selftests/bpf:fix a resource leak
@ 2024-06-27 6:01 Zhu Jun
2024-06-28 22:23 ` Daniel Borkmann
0 siblings, 1 reply; 4+ messages in thread
From: Zhu Jun @ 2024-06-27 6:01 UTC (permalink / raw)
To: shuah
Cc: ast, daniel, andrii, martin.lau, eddyz87, song, yonghong.song,
john.fastabend, kpsingh, sdf, haoluo, jolsa, mykolal, bpf,
linux-kselftest, linux-kernel, Zhu Jun
The requested resources should be closed before return
in main(), otherwise resource leak will occur
Signed-off-by: Zhu Jun <zhujun2@cmss.chinamobile.com>
---
tools/testing/selftests/bpf/test_sockmap.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/bpf/test_sockmap.c b/tools/testing/selftests/bpf/test_sockmap.c
index a34e95040994..03d5dd617c4a 100644
--- a/tools/testing/selftests/bpf/test_sockmap.c
+++ b/tools/testing/selftests/bpf/test_sockmap.c
@@ -2092,7 +2092,7 @@ int main(int argc, char **argv)
if (err) {
fprintf(stderr, "populate program: (%s) %s\n",
bpf_file, strerror(errno));
- return 1;
+ goto out;
}
running = 1;
--
2.17.1
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] selftests/bpf:fix a resource leak
2024-06-27 6:01 [PATCH] selftests/bpf:fix a resource leak Zhu Jun
@ 2024-06-28 22:23 ` Daniel Borkmann
2024-07-01 2:13 ` Zhu Jun
0 siblings, 1 reply; 4+ messages in thread
From: Daniel Borkmann @ 2024-06-28 22:23 UTC (permalink / raw)
To: Zhu Jun, shuah
Cc: ast, andrii, martin.lau, eddyz87, song, yonghong.song,
john.fastabend, kpsingh, sdf, haoluo, jolsa, mykolal, bpf,
linux-kselftest, linux-kernel
On 6/27/24 8:01 AM, Zhu Jun wrote:
> The requested resources should be closed before return
> in main(), otherwise resource leak will occur
>
> Signed-off-by: Zhu Jun <zhujun2@cmss.chinamobile.com>
> ---
> tools/testing/selftests/bpf/test_sockmap.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/bpf/test_sockmap.c b/tools/testing/selftests/bpf/test_sockmap.c
> index a34e95040994..03d5dd617c4a 100644
> --- a/tools/testing/selftests/bpf/test_sockmap.c
> +++ b/tools/testing/selftests/bpf/test_sockmap.c
> @@ -2092,7 +2092,7 @@ int main(int argc, char **argv)
> if (err) {
> fprintf(stderr, "populate program: (%s) %s\n",
> bpf_file, strerror(errno));
> - return 1;
> + goto out;
> }
> running = 1;
There's one more when cgroup_setup_and_join() fails a bit higher up
where we end up leaking options.whitelist/options.blacklist. Maybe
could you reorder the free() towards the end, add a new label, set
err to cg_fd and jump there.
Thanks,
Daniel
^ permalink raw reply [flat|nested] 4+ messages in thread* [PATCH] selftests/bpf:fix a resource leak
2024-06-28 22:23 ` Daniel Borkmann
@ 2024-07-01 2:13 ` Zhu Jun
2024-07-01 9:27 ` Jiri Olsa
0 siblings, 1 reply; 4+ messages in thread
From: Zhu Jun @ 2024-07-01 2:13 UTC (permalink / raw)
To: daniel
Cc: andrii, ast, bpf, eddyz87, haoluo, john.fastabend, jolsa,
kpsingh, linux-kernel, linux-kselftest, martin.lau, mykolal, sdf,
shuah, song, yonghong.song, zhujun2
The requested resources should be closed before return
in main(), otherwise resource leak will occur
Signed-off-by: Zhu Jun <zhujun2@cmss.chinamobile.com>
---
tools/testing/selftests/bpf/test_sockmap.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/tools/testing/selftests/bpf/test_sockmap.c b/tools/testing/selftests/bpf/test_sockmap.c
index a34e95040994..d51af986d9d8 100644
--- a/tools/testing/selftests/bpf/test_sockmap.c
+++ b/tools/testing/selftests/bpf/test_sockmap.c
@@ -2075,8 +2075,10 @@ int main(int argc, char **argv)
if (!cg_fd) {
cg_fd = cgroup_setup_and_join(CG_PATH);
- if (cg_fd < 0)
- return cg_fd;
+ if (cg_fd < 0) {
+ err = cg_fd;
+ goto out1;
+ }
cg_created = 1;
}
@@ -2092,7 +2094,7 @@ int main(int argc, char **argv)
if (err) {
fprintf(stderr, "populate program: (%s) %s\n",
bpf_file, strerror(errno));
- return 1;
+ goto out;
}
running = 1;
@@ -2105,13 +2107,14 @@ int main(int argc, char **argv)
err = run_options(&options, cg_fd, test);
out:
+ close(cg_fd);
+ if (cg_created)
+ cleanup_cgroup_environment();
+out1:
if (options.whitelist)
free(options.whitelist);
if (options.blacklist)
free(options.blacklist);
- close(cg_fd);
- if (cg_created)
- cleanup_cgroup_environment();
return err;
}
--
2.17.1
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] selftests/bpf:fix a resource leak
2024-07-01 2:13 ` Zhu Jun
@ 2024-07-01 9:27 ` Jiri Olsa
0 siblings, 0 replies; 4+ messages in thread
From: Jiri Olsa @ 2024-07-01 9:27 UTC (permalink / raw)
To: Zhu Jun
Cc: daniel, andrii, ast, bpf, eddyz87, haoluo, john.fastabend,
kpsingh, linux-kernel, linux-kselftest, martin.lau, mykolal, sdf,
shuah, song, yonghong.song
On Sun, Jun 30, 2024 at 07:13:31PM -0700, Zhu Jun wrote:
> The requested resources should be closed before return
> in main(), otherwise resource leak will occur
>
> Signed-off-by: Zhu Jun <zhujun2@cmss.chinamobile.com>
could you please send this not as reply but as a separate patch with v2
> ---
> tools/testing/selftests/bpf/test_sockmap.c | 15 +++++++++------
> 1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/test_sockmap.c b/tools/testing/selftests/bpf/test_sockmap.c
> index a34e95040994..d51af986d9d8 100644
> --- a/tools/testing/selftests/bpf/test_sockmap.c
> +++ b/tools/testing/selftests/bpf/test_sockmap.c
> @@ -2075,8 +2075,10 @@ int main(int argc, char **argv)
>
> if (!cg_fd) {
> cg_fd = cgroup_setup_and_join(CG_PATH);
> - if (cg_fd < 0)
> - return cg_fd;
> + if (cg_fd < 0) {
> + err = cg_fd;
> + goto out1;
> + }
> cg_created = 1;
> }
>
> @@ -2092,7 +2094,7 @@ int main(int argc, char **argv)
> if (err) {
> fprintf(stderr, "populate program: (%s) %s\n",
> bpf_file, strerror(errno));
> - return 1;
> + goto out;
> }
> running = 1;
>
> @@ -2105,13 +2107,14 @@ int main(int argc, char **argv)
>
> err = run_options(&options, cg_fd, test);
> out:
could we check for cg_fd >= 0 and have just one out label? like:
out:
if (cg_fd >= 0)
close(cf_fd);
if (cg_created)
...
thanks,
jirka
> + close(cg_fd);
> + if (cg_created)
> + cleanup_cgroup_environment();
> +out1:
> if (options.whitelist)
> free(options.whitelist);
> if (options.blacklist)
> free(options.blacklist);
> - close(cg_fd);
> - if (cg_created)
> - cleanup_cgroup_environment();
> return err;
> }
>
> --
> 2.17.1
>
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-07-01 9:27 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-06-27 6:01 [PATCH] selftests/bpf:fix a resource leak Zhu Jun
2024-06-28 22:23 ` Daniel Borkmann
2024-07-01 2:13 ` Zhu Jun
2024-07-01 9:27 ` Jiri Olsa
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®