bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH bpf-next 0/2] Fix resource leaks in test_maps
@ 2022-09-21  2:58 Hou Tao
  2022-09-21  2:58 ` [PATCH bpf-next 1/2] selftests/bpf: Destroy the skeleton when CONFIG_PREEMPT is off Hou Tao
  2022-09-21  2:58 ` [PATCH bpf-next 2/2] selftests/bpf: Free the allocated resources after test case succeeds Hou Tao
  0 siblings, 2 replies; 3+ messages in thread
From: Hou Tao @ 2022-09-21  2:58 UTC (permalink / raw)
  To: bpf
  Cc: Martin KaFai Lau, Andrii Nakryiko, Song Liu, Hao Luo,
	Yonghong Song, Alexei Starovoitov, Daniel Borkmann, KP Singh,
	David S . Miller, Jakub Kicinski, Stanislav Fomichev, Jiri Olsa,
	John Fastabend, Lorenz Bauer, houtao1

From: Hou Tao <houtao1@huawei.com>

Hi,

It is just a tiny patch set aims to fix the resource leaks in test_maps
after test case succeeds or is skipped. And these leaks are spotted by
using address sanitizer and checking the content of /proc/$pid/fd.

Please see indiviual patch for more details.

Hou Tao (2):
  selftests/bpf: Destroy the skeleton when CONFIG_PREEMPT is off
  selftests/bpf: Free the allocated resources after test case succeeds

 .../bpf/map_tests/array_map_batch_ops.c       |  1 +
 .../bpf/map_tests/htab_map_batch_ops.c        |  1 +
 .../bpf/map_tests/lpm_trie_map_batch_ops.c    |  1 +
 .../bpf/map_tests/task_storage_map.c          |  1 +
 tools/testing/selftests/bpf/test_maps.c       | 24 ++++++++++++-------
 5 files changed, 19 insertions(+), 9 deletions(-)

-- 
2.29.2


^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PATCH bpf-next 1/2] selftests/bpf: Destroy the skeleton when CONFIG_PREEMPT is off
  2022-09-21  2:58 [PATCH bpf-next 0/2] Fix resource leaks in test_maps Hou Tao
@ 2022-09-21  2:58 ` Hou Tao
  2022-09-21  2:58 ` [PATCH bpf-next 2/2] selftests/bpf: Free the allocated resources after test case succeeds Hou Tao
  1 sibling, 0 replies; 3+ messages in thread
From: Hou Tao @ 2022-09-21  2:58 UTC (permalink / raw)
  To: bpf
  Cc: Martin KaFai Lau, Andrii Nakryiko, Song Liu, Hao Luo,
	Yonghong Song, Alexei Starovoitov, Daniel Borkmann, KP Singh,
	David S . Miller, Jakub Kicinski, Stanislav Fomichev, Jiri Olsa,
	John Fastabend, Lorenz Bauer, houtao1

From: Hou Tao <houtao1@huawei.com>

Destroy the created skeleton when CONFIG_PREEMPT is off, else will be
resource leak.

Fixes: 73b97bc78b32 ("selftests/bpf: Test concurrent updates on bpf_task_storage_busy")
Signed-off-by: Hou Tao <houtao1@huawei.com>
---
 tools/testing/selftests/bpf/map_tests/task_storage_map.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/testing/selftests/bpf/map_tests/task_storage_map.c b/tools/testing/selftests/bpf/map_tests/task_storage_map.c
index aac08c85240b..7d050364efca 100644
--- a/tools/testing/selftests/bpf/map_tests/task_storage_map.c
+++ b/tools/testing/selftests/bpf/map_tests/task_storage_map.c
@@ -79,6 +79,7 @@ void test_task_storage_map_stress_lookup(void)
 	/* Only for a fully preemptible kernel */
 	if (!skel->kconfig->CONFIG_PREEMPT) {
 		printf("%s SKIP (no CONFIG_PREEMPT)\n", __func__);
+		read_bpf_task_storage_busy__destroy(skel);
 		skips++;
 		return;
 	}
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* [PATCH bpf-next 2/2] selftests/bpf: Free the allocated resources after test case succeeds
  2022-09-21  2:58 [PATCH bpf-next 0/2] Fix resource leaks in test_maps Hou Tao
  2022-09-21  2:58 ` [PATCH bpf-next 1/2] selftests/bpf: Destroy the skeleton when CONFIG_PREEMPT is off Hou Tao
@ 2022-09-21  2:58 ` Hou Tao
  1 sibling, 0 replies; 3+ messages in thread
From: Hou Tao @ 2022-09-21  2:58 UTC (permalink / raw)
  To: bpf
  Cc: Martin KaFai Lau, Andrii Nakryiko, Song Liu, Hao Luo,
	Yonghong Song, Alexei Starovoitov, Daniel Borkmann, KP Singh,
	David S . Miller, Jakub Kicinski, Stanislav Fomichev, Jiri Olsa,
	John Fastabend, Lorenz Bauer, houtao1

From: Hou Tao <houtao1@huawei.com>

Free the created fd or allocated bpf_object after test case succeeds,
else there will be resource leaks.

Spotted by using address sanitizer and checking the content of
/proc/$pid/fd directory.

Signed-off-by: Hou Tao <houtao1@huawei.com>
---
 .../bpf/map_tests/array_map_batch_ops.c       |  1 +
 .../bpf/map_tests/htab_map_batch_ops.c        |  1 +
 .../bpf/map_tests/lpm_trie_map_batch_ops.c    |  1 +
 tools/testing/selftests/bpf/test_maps.c       | 24 ++++++++++++-------
 4 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/tools/testing/selftests/bpf/map_tests/array_map_batch_ops.c b/tools/testing/selftests/bpf/map_tests/array_map_batch_ops.c
index 78c76496b14a..cc95aafbd721 100644
--- a/tools/testing/selftests/bpf/map_tests/array_map_batch_ops.c
+++ b/tools/testing/selftests/bpf/map_tests/array_map_batch_ops.c
@@ -137,6 +137,7 @@ static void __test_map_lookup_and_update_batch(bool is_pcpu)
 	free(keys);
 	free(values);
 	free(visited);
+	close(map_fd);
 }
 
 static void array_map_batch_ops(void)
diff --git a/tools/testing/selftests/bpf/map_tests/htab_map_batch_ops.c b/tools/testing/selftests/bpf/map_tests/htab_map_batch_ops.c
index f807d53fd8dd..dbd680100e50 100644
--- a/tools/testing/selftests/bpf/map_tests/htab_map_batch_ops.c
+++ b/tools/testing/selftests/bpf/map_tests/htab_map_batch_ops.c
@@ -255,6 +255,7 @@ void __test_map_lookup_and_delete_batch(bool is_pcpu)
 	free(visited);
 	if (!is_pcpu)
 		free(values);
+	close(map_fd);
 }
 
 void htab_map_batch_ops(void)
diff --git a/tools/testing/selftests/bpf/map_tests/lpm_trie_map_batch_ops.c b/tools/testing/selftests/bpf/map_tests/lpm_trie_map_batch_ops.c
index 87d07b596e17..874688c6d221 100644
--- a/tools/testing/selftests/bpf/map_tests/lpm_trie_map_batch_ops.c
+++ b/tools/testing/selftests/bpf/map_tests/lpm_trie_map_batch_ops.c
@@ -150,4 +150,5 @@ void test_lpm_trie_map_batch_ops(void)
 	free(keys);
 	free(values);
 	free(visited);
+	close(map_fd);
 }
diff --git a/tools/testing/selftests/bpf/test_maps.c b/tools/testing/selftests/bpf/test_maps.c
index f0b830f3d7b8..ba7b2e0f9bf4 100644
--- a/tools/testing/selftests/bpf/test_maps.c
+++ b/tools/testing/selftests/bpf/test_maps.c
@@ -659,13 +659,13 @@ static void test_sockmap(unsigned int tasks, void *data)
 {
 	struct bpf_map *bpf_map_rx, *bpf_map_tx, *bpf_map_msg, *bpf_map_break;
 	int map_fd_msg = 0, map_fd_rx = 0, map_fd_tx = 0, map_fd_break;
+	struct bpf_object *parse_obj, *verdict_obj, *msg_obj;
 	int ports[] = {50200, 50201, 50202, 50204};
 	int err, i, fd, udp, sfd[6] = {0xdeadbeef};
 	u8 buf[20] = {0x0, 0x5, 0x3, 0x2, 0x1, 0x0};
 	int parse_prog, verdict_prog, msg_prog;
 	struct sockaddr_in addr;
 	int one = 1, s, sc, rc;
-	struct bpf_object *obj;
 	struct timeval to;
 	__u32 key, value;
 	pid_t pid[tasks];
@@ -761,6 +761,7 @@ static void test_sockmap(unsigned int tasks, void *data)
 		       i, udp);
 		goto out_sockmap;
 	}
+	close(udp);
 
 	/* Test update without programs */
 	for (i = 0; i < 6; i++) {
@@ -823,27 +824,27 @@ static void test_sockmap(unsigned int tasks, void *data)
 
 	/* Load SK_SKB program and Attach */
 	err = bpf_prog_test_load(SOCKMAP_PARSE_PROG,
-			    BPF_PROG_TYPE_SK_SKB, &obj, &parse_prog);
+			    BPF_PROG_TYPE_SK_SKB, &parse_obj, &parse_prog);
 	if (err) {
 		printf("Failed to load SK_SKB parse prog\n");
 		goto out_sockmap;
 	}
 
 	err = bpf_prog_test_load(SOCKMAP_TCP_MSG_PROG,
-			    BPF_PROG_TYPE_SK_MSG, &obj, &msg_prog);
+			    BPF_PROG_TYPE_SK_MSG, &msg_obj, &msg_prog);
 	if (err) {
 		printf("Failed to load SK_SKB msg prog\n");
 		goto out_sockmap;
 	}
 
 	err = bpf_prog_test_load(SOCKMAP_VERDICT_PROG,
-			    BPF_PROG_TYPE_SK_SKB, &obj, &verdict_prog);
+			    BPF_PROG_TYPE_SK_SKB, &verdict_obj, &verdict_prog);
 	if (err) {
 		printf("Failed to load SK_SKB verdict prog\n");
 		goto out_sockmap;
 	}
 
-	bpf_map_rx = bpf_object__find_map_by_name(obj, "sock_map_rx");
+	bpf_map_rx = bpf_object__find_map_by_name(verdict_obj, "sock_map_rx");
 	if (!bpf_map_rx) {
 		printf("Failed to load map rx from verdict prog\n");
 		goto out_sockmap;
@@ -855,7 +856,7 @@ static void test_sockmap(unsigned int tasks, void *data)
 		goto out_sockmap;
 	}
 
-	bpf_map_tx = bpf_object__find_map_by_name(obj, "sock_map_tx");
+	bpf_map_tx = bpf_object__find_map_by_name(verdict_obj, "sock_map_tx");
 	if (!bpf_map_tx) {
 		printf("Failed to load map tx from verdict prog\n");
 		goto out_sockmap;
@@ -867,7 +868,7 @@ static void test_sockmap(unsigned int tasks, void *data)
 		goto out_sockmap;
 	}
 
-	bpf_map_msg = bpf_object__find_map_by_name(obj, "sock_map_msg");
+	bpf_map_msg = bpf_object__find_map_by_name(verdict_obj, "sock_map_msg");
 	if (!bpf_map_msg) {
 		printf("Failed to load map msg from msg_verdict prog\n");
 		goto out_sockmap;
@@ -879,7 +880,7 @@ static void test_sockmap(unsigned int tasks, void *data)
 		goto out_sockmap;
 	}
 
-	bpf_map_break = bpf_object__find_map_by_name(obj, "sock_map_break");
+	bpf_map_break = bpf_object__find_map_by_name(verdict_obj, "sock_map_break");
 	if (!bpf_map_break) {
 		printf("Failed to load map tx from verdict prog\n");
 		goto out_sockmap;
@@ -1125,7 +1126,9 @@ static void test_sockmap(unsigned int tasks, void *data)
 	}
 	close(fd);
 	close(map_fd_rx);
-	bpf_object__close(obj);
+	bpf_object__close(parse_obj);
+	bpf_object__close(msg_obj);
+	bpf_object__close(verdict_obj);
 	return;
 out:
 	for (i = 0; i < 6; i++)
@@ -1283,8 +1286,11 @@ static void test_map_in_map(void)
 			printf("Inner map mim.inner was not destroyed\n");
 			goto out_map_in_map;
 		}
+
+		close(fd);
 	}
 
+	bpf_object__close(obj);
 	return;
 
 out_map_in_map:
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2022-09-21  2:41 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-21  2:58 [PATCH bpf-next 0/2] Fix resource leaks in test_maps Hou Tao
2022-09-21  2:58 ` [PATCH bpf-next 1/2] selftests/bpf: Destroy the skeleton when CONFIG_PREEMPT is off Hou Tao
2022-09-21  2:58 ` [PATCH bpf-next 2/2] selftests/bpf: Free the allocated resources after test case succeeds Hou Tao

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).