On 12/15/22 5:59 PM, Kui-Feng Lee wrote:
According to a report, the system may crash when a task iterator
There is no context about this 'a report'. You can just remove it
and say:
When a task iterator traverses vma(s), it is possible task->mm
might become invalid in the middle of traversal and this may
cause kernel misbehave (e.g., crash).
travels vma(s). The investigation shows it takes place if the
visiting task dies during the visit. >
This test case creates iterators repeatedly and forks short-lived
processes in the background to detect this bug. The test will last
for 3 seconds to get the chance to trigger the issue.
The subject is not precise. The test is not about
"create new processes repeatedly in the background."
It is about
"Add a test for iter/task_vma with shortlived processes"
Signed-off-by: Kui-Feng Lee <kuifeng@xxxxxxxx>
Ack with a few nits.
Acked-by: Yonghong Song <yhs@xxxxxx>
---
.../selftests/bpf/prog_tests/bpf_iter.c | 79 +++++++++++++++++++
1 file changed, 79 insertions(+)
diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_iter.c b/tools/testing/selftests/bpf/prog_tests/bpf_iter.c
index 6f8ed61fc4b4..df13350d615a 100644
--- a/tools/testing/selftests/bpf/prog_tests/bpf_iter.c
+++ b/tools/testing/selftests/bpf/prog_tests/bpf_iter.c
@@ -1465,6 +1465,83 @@ static void test_task_vma_common(struct bpf_iter_attach_opts *opts)
bpf_iter_task_vma__destroy(skel);
}
+static void test_task_vma_dead_task(void)
+{
+ int err, iter_fd = -1;
+ struct bpf_iter_task_vma *skel;
+ int wstatus, child_pid = -1;
+ time_t start_tm, cur_tm;
+ int wait_sec = 3;
Since it is new code, maybe reverse Christmas tree coding style.
+
+ skel = bpf_iter_task_vma__open();
+ if (!ASSERT_OK_PTR(skel, "bpf_iter_task_vma__open"))
+ return;
+
+ skel->bss->pid = getpid();
+
+ err = bpf_iter_task_vma__load(skel);
+ if (!ASSERT_OK(err, "bpf_iter_task_vma__load"))
+ goto out;
+
+ skel->links.proc_maps = bpf_program__attach_iter(
+ skel->progs.proc_maps, NULL);
+
+ if (!ASSERT_OK_PTR(skel->links.proc_maps, "bpf_program__attach_iter")) {
+ skel->links.proc_maps = NULL;
+ goto out;
+ }
+
+ start_tm = time(NULL);
+ if (start_tm < 0)
+ goto out;
From the man page, start_tm should not fail. Note that you didn't put
an ASSERT* either. So I think you can remove it. The same for a few
instances below.
+ cur_tm = start_tm;
+
+ child_pid = fork();
+ if (child_pid == 0) {
+ /* Fork short-lived processes in the background. */
+ while (cur_tm < start_tm + wait_sec) {
+ system("echo > /dev/null");
+ cur_tm = time(NULL);
+ if (cur_tm < 0)
+ exit(1);
+ }
+ exit(0);
+ }
+
+ if (!ASSERT_GE(child_pid, 0, "fork_child"))
+ goto out;
+
+ while (cur_tm < start_tm + wait_sec) {
+ iter_fd = bpf_iter_create(bpf_link__fd(skel->links.proc_maps));
+ if (!ASSERT_GE(iter_fd, 0, "create_iter"))
+ goto out;
+
+ /* Drain all data from iter_fd. */
+ while (cur_tm < start_tm + wait_sec) {
+ err = read_fd_into_buffer(iter_fd, task_vma_output, CMP_BUFFER_SIZE);
+ if (!ASSERT_GE(err, 0, "read_iter_fd"))
+ goto out;
+
+ cur_tm = time(NULL);
+ if (cur_tm < 0)
+ goto out;
+
+ if (err == 0)
+ break;
+ }
+
+ close(iter_fd);
+ iter_fd = -1;
+ }
+
+ check_bpf_link_info(skel->progs.proc_maps);
+
+out:
+ waitpid(child_pid, &wstatus, 0);
+ close(iter_fd);
+ bpf_iter_task_vma__destroy(skel);
+}
+
void test_bpf_sockmap_map_iter_fd(void)
{
struct bpf_iter_sockmap *skel;
@@ -1586,6 +1663,8 @@ void test_bpf_iter(void)
test_task_file();
if (test__start_subtest("task_vma"))
test_task_vma();
+ if (test__start_subtest("task_vma_dead_task"))
+ test_task_vma_dead_task();
if (test__start_subtest("task_btf"))
test_task_btf();
if (test__start_subtest("tcp4"))
