* [PATCH bpf v2 1/2] bpf: Fix iter/task tid filtering
@ 2024-10-16 21:00 Jordan Rome
2024-10-16 21:00 ` [PATCH bpf v2 2/2] bpf: properly test " Jordan Rome
2024-10-17 18:10 ` [PATCH bpf v2 1/2] bpf: Fix " patchwork-bot+netdevbpf
0 siblings, 2 replies; 3+ messages in thread
From: Jordan Rome @ 2024-10-16 21:00 UTC (permalink / raw)
To: bpf
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Kernel Team
In userspace, you can add a tid filter by setting
the "task.tid" field for "bpf_iter_link_info".
However, `get_pid_task` when called for the
`BPF_TASK_ITER_TID` type should have been using
`PIDTYPE_PID` (tid) instead of `PIDTYPE_TGID` (pid).
Fixes: f0d74c4da1f0 ("bpf: Parameterize task iterators.")
Signed-off-by: Jordan Rome <linux@jordanrome.com>
---
kernel/bpf/task_iter.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/bpf/task_iter.c b/kernel/bpf/task_iter.c
index 02aa9db8d796..5af9e130e500 100644
--- a/kernel/bpf/task_iter.c
+++ b/kernel/bpf/task_iter.c
@@ -99,7 +99,7 @@ static struct task_struct *task_seq_get_next(struct bpf_iter_seq_task_common *co
rcu_read_lock();
pid = find_pid_ns(common->pid, common->ns);
if (pid) {
- task = get_pid_task(pid, PIDTYPE_TGID);
+ task = get_pid_task(pid, PIDTYPE_PID);
*tid = common->pid;
}
rcu_read_unlock();
--
2.43.5
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [PATCH bpf v2 2/2] bpf: properly test iter/task tid filtering
2024-10-16 21:00 [PATCH bpf v2 1/2] bpf: Fix iter/task tid filtering Jordan Rome
@ 2024-10-16 21:00 ` Jordan Rome
2024-10-17 18:10 ` [PATCH bpf v2 1/2] bpf: Fix " patchwork-bot+netdevbpf
1 sibling, 0 replies; 3+ messages in thread
From: Jordan Rome @ 2024-10-16 21:00 UTC (permalink / raw)
To: bpf
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Kernel Team
Previously test_task_tid was setting `linfo.task.tid`
to `getpid()` which is the same as `gettid()` for the
parent process. Instead create a new child thread
and set `linfo.task.tid` to `gettid()` to make sure
the tid filtering logic is working as expected.
Signed-off-by: Jordan Rome <linux@jordanrome.com>
---
.../selftests/bpf/prog_tests/bpf_iter.c | 27 +++++++++++++++----
1 file changed, 22 insertions(+), 5 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_iter.c b/tools/testing/selftests/bpf/prog_tests/bpf_iter.c
index 52e6f7570475..f0a3a9c18e9e 100644
--- a/tools/testing/selftests/bpf/prog_tests/bpf_iter.c
+++ b/tools/testing/selftests/bpf/prog_tests/bpf_iter.c
@@ -226,7 +226,7 @@ static void test_task_common_nocheck(struct bpf_iter_attach_opts *opts,
ASSERT_OK(pthread_create(&thread_id, NULL, &do_nothing_wait, NULL),
"pthread_create");
- skel->bss->tid = getpid();
+ skel->bss->tid = gettid();
do_dummy_read_opts(skel->progs.dump_task, opts);
@@ -249,25 +249,42 @@ static void test_task_common(struct bpf_iter_attach_opts *opts, int num_unknown,
ASSERT_EQ(num_known_tid, num_known, "check_num_known_tid");
}
-static void test_task_tid(void)
+static void *run_test_task_tid(void *arg)
{
LIBBPF_OPTS(bpf_iter_attach_opts, opts);
union bpf_iter_link_info linfo;
int num_unknown_tid, num_known_tid;
+ ASSERT_NEQ(getpid(), gettid(), "check_new_thread_id");
+
memset(&linfo, 0, sizeof(linfo));
- linfo.task.tid = getpid();
+ linfo.task.tid = gettid();
opts.link_info = &linfo;
opts.link_info_len = sizeof(linfo);
test_task_common(&opts, 0, 1);
linfo.task.tid = 0;
linfo.task.pid = getpid();
- test_task_common(&opts, 1, 1);
+ /* This includes the parent thread, this thread,
+ * and the do_nothing_wait thread
+ */
+ test_task_common(&opts, 2, 1);
test_task_common_nocheck(NULL, &num_unknown_tid, &num_known_tid);
- ASSERT_GT(num_unknown_tid, 1, "check_num_unknown_tid");
+ ASSERT_GT(num_unknown_tid, 2, "check_num_unknown_tid");
ASSERT_EQ(num_known_tid, 1, "check_num_known_tid");
+
+ return NULL;
+}
+
+static void test_task_tid(void)
+{
+ pthread_t thread_id;
+
+ /* Create a new thread so pid and tid aren't the same */
+ ASSERT_OK(pthread_create(&thread_id, NULL, &run_test_task_tid, NULL),
+ "pthread_create");
+ ASSERT_FALSE(pthread_join(thread_id, NULL), "pthread_join");
}
static void test_task_pid(void)
--
2.43.5
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH bpf v2 1/2] bpf: Fix iter/task tid filtering
2024-10-16 21:00 [PATCH bpf v2 1/2] bpf: Fix iter/task tid filtering Jordan Rome
2024-10-16 21:00 ` [PATCH bpf v2 2/2] bpf: properly test " Jordan Rome
@ 2024-10-17 18:10 ` patchwork-bot+netdevbpf
1 sibling, 0 replies; 3+ messages in thread
From: patchwork-bot+netdevbpf @ 2024-10-17 18:10 UTC (permalink / raw)
To: Jordan Rome; +Cc: bpf, ast, daniel, andrii, martin.lau, kernel-team
Hello:
This series was applied to bpf/bpf.git (master)
by Andrii Nakryiko <andrii@kernel.org>:
On Wed, 16 Oct 2024 14:00:47 -0700 you wrote:
> In userspace, you can add a tid filter by setting
> the "task.tid" field for "bpf_iter_link_info".
> However, `get_pid_task` when called for the
> `BPF_TASK_ITER_TID` type should have been using
> `PIDTYPE_PID` (tid) instead of `PIDTYPE_TGID` (pid).
>
> Fixes: f0d74c4da1f0 ("bpf: Parameterize task iterators.")
> Signed-off-by: Jordan Rome <linux@jordanrome.com>
>
> [...]
Here is the summary with links:
- [bpf,v2,1/2] bpf: Fix iter/task tid filtering
https://git.kernel.org/bpf/bpf/c/9495a5b731fc
- [bpf,v2,2/2] bpf: properly test iter/task tid filtering
https://git.kernel.org/bpf/bpf/c/ee8c7c6c3f8c
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-10-17 18:10 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-16 21:00 [PATCH bpf v2 1/2] bpf: Fix iter/task tid filtering Jordan Rome
2024-10-16 21:00 ` [PATCH bpf v2 2/2] bpf: properly test " Jordan Rome
2024-10-17 18:10 ` [PATCH bpf v2 1/2] bpf: Fix " patchwork-bot+netdevbpf
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox