* [PATCH bpf-next v4 1/3] bpf: Relax allowlist for css_task iter
2023-10-31 5:04 [PATCH bpf-next v4 0/3] Relax allowlist for open-coded css_task iter Chuyi Zhou
@ 2023-10-31 5:04 ` Chuyi Zhou
2023-10-31 5:04 ` [PATCH bpf-next v4 2/3] selftests/bpf: Add tests for css_task iter combining with cgroup iter Chuyi Zhou
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Chuyi Zhou @ 2023-10-31 5:04 UTC (permalink / raw)
To: bpf; +Cc: ast, daniel, andrii, martin.lau, Chuyi Zhou, Yonghong Song
The newly added open-coded css_task iter would try to hold the global
css_set_lock in bpf_iter_css_task_new, so the bpf side has to be careful in
where it allows to use this iter. The mainly concern is dead locking on
css_set_lock. check_css_task_iter_allowlist() in verifier enforced css_task
can only be used in bpf_lsm hooks and sleepable bpf_iter.
This patch relax the allowlist for css_task iter. Any lsm and any iter
(even non-sleepable) and any sleepable are safe since they would not hold
the css_set_lock before entering BPF progs context.
This patch also fixes the misused BPF_TRACE_ITER in
check_css_task_iter_allowlist which compared bpf_prog_type with
bpf_attach_type.
Fixes: 9c66dc94b62ae ("bpf: Introduce css_task open-coded iterator kfuncs")
Signed-off-by: Chuyi Zhou <zhouchuyi@bytedance.com>
Acked-by: Yonghong Song <yonghong.song@linux.dev>
---
kernel/bpf/verifier.c | 16 ++++++++++++----
.../selftests/bpf/progs/iters_task_failure.c | 4 ++--
2 files changed, 14 insertions(+), 6 deletions(-)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 857d76694517..b2ddecfdff9d 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -11394,6 +11394,12 @@ static int process_kf_arg_ptr_to_rbtree_node(struct bpf_verifier_env *env,
&meta->arg_rbtree_root.field);
}
+/*
+ * css_task iter allowlist is needed to avoid dead locking on css_set_lock.
+ * LSM hooks and iters (both sleepable and non-sleepable) are safe.
+ * Any sleepable progs are also safe since bpf_check_attach_target() enforce
+ * them can only be attached to some specific hook points.
+ */
static bool check_css_task_iter_allowlist(struct bpf_verifier_env *env)
{
enum bpf_prog_type prog_type = resolve_prog_type(env->prog);
@@ -11401,10 +11407,12 @@ static bool check_css_task_iter_allowlist(struct bpf_verifier_env *env)
switch (prog_type) {
case BPF_PROG_TYPE_LSM:
return true;
- case BPF_TRACE_ITER:
- return env->prog->aux->sleepable;
+ case BPF_PROG_TYPE_TRACING:
+ if (env->prog->expected_attach_type == BPF_TRACE_ITER)
+ return true;
+ fallthrough;
default:
- return false;
+ return env->prog->aux->sleepable;
}
}
@@ -11663,7 +11671,7 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_
case KF_ARG_PTR_TO_ITER:
if (meta->func_id == special_kfunc_list[KF_bpf_iter_css_task_new]) {
if (!check_css_task_iter_allowlist(env)) {
- verbose(env, "css_task_iter is only allowed in bpf_lsm and bpf iter-s\n");
+ verbose(env, "css_task_iter is only allowed in bpf_lsm, bpf_iter and sleepable progs\n");
return -EINVAL;
}
}
diff --git a/tools/testing/selftests/bpf/progs/iters_task_failure.c b/tools/testing/selftests/bpf/progs/iters_task_failure.c
index c3bf96a67dba..6b1588d70652 100644
--- a/tools/testing/selftests/bpf/progs/iters_task_failure.c
+++ b/tools/testing/selftests/bpf/progs/iters_task_failure.c
@@ -84,8 +84,8 @@ int BPF_PROG(iter_css_lock_and_unlock)
return 0;
}
-SEC("?fentry.s/" SYS_PREFIX "sys_getpgid")
-__failure __msg("css_task_iter is only allowed in bpf_lsm and bpf iter-s")
+SEC("?fentry/" SYS_PREFIX "sys_getpgid")
+__failure __msg("css_task_iter is only allowed in bpf_lsm, bpf_iter and sleepable progs")
int BPF_PROG(iter_css_task_for_each)
{
u64 cg_id = bpf_get_current_cgroup_id();
--
2.20.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH bpf-next v4 2/3] selftests/bpf: Add tests for css_task iter combining with cgroup iter
2023-10-31 5:04 [PATCH bpf-next v4 0/3] Relax allowlist for open-coded css_task iter Chuyi Zhou
2023-10-31 5:04 ` [PATCH bpf-next v4 1/3] bpf: Relax allowlist for " Chuyi Zhou
@ 2023-10-31 5:04 ` Chuyi Zhou
2023-10-31 5:04 ` [PATCH bpf-next v4 3/3] selftests/bpf: Add test for using css_task iter in sleepable progs Chuyi Zhou
2023-11-02 6:10 ` [PATCH bpf-next v4 0/3] Relax allowlist for open-coded css_task iter patchwork-bot+netdevbpf
3 siblings, 0 replies; 6+ messages in thread
From: Chuyi Zhou @ 2023-10-31 5:04 UTC (permalink / raw)
To: bpf; +Cc: ast, daniel, andrii, martin.lau, Chuyi Zhou, Yonghong Song
This patch adds a test which demonstrates how css_task iter can be combined
with cgroup iter and it won't cause deadlock, though cgroup iter is not
sleepable.
Signed-off-by: Chuyi Zhou <zhouchuyi@bytedance.com>
Acked-by: Yonghong Song <yonghong.song@linux.dev>
---
.../selftests/bpf/prog_tests/cgroup_iter.c | 33 ++++++++++++++
.../selftests/bpf/progs/iters_css_task.c | 44 +++++++++++++++++++
2 files changed, 77 insertions(+)
diff --git a/tools/testing/selftests/bpf/prog_tests/cgroup_iter.c b/tools/testing/selftests/bpf/prog_tests/cgroup_iter.c
index e02feb5fae97..574d9a0cdc8e 100644
--- a/tools/testing/selftests/bpf/prog_tests/cgroup_iter.c
+++ b/tools/testing/selftests/bpf/prog_tests/cgroup_iter.c
@@ -4,6 +4,7 @@
#include <test_progs.h>
#include <bpf/libbpf.h>
#include <bpf/btf.h>
+#include "iters_css_task.skel.h"
#include "cgroup_iter.skel.h"
#include "cgroup_helpers.h"
@@ -263,6 +264,35 @@ static void test_walk_dead_self_only(struct cgroup_iter *skel)
close(cgrp_fd);
}
+static void test_walk_self_only_css_task(void)
+{
+ struct iters_css_task *skel;
+ int err;
+
+ skel = iters_css_task__open();
+ if (!ASSERT_OK_PTR(skel, "skel_open"))
+ return;
+
+ bpf_program__set_autoload(skel->progs.cgroup_id_printer, true);
+
+ err = iters_css_task__load(skel);
+ if (!ASSERT_OK(err, "skel_load"))
+ goto cleanup;
+
+ err = join_cgroup(cg_path[CHILD2]);
+ if (!ASSERT_OK(err, "join_cgroup"))
+ goto cleanup;
+
+ skel->bss->target_pid = getpid();
+ snprintf(expected_output, sizeof(expected_output),
+ PROLOGUE "%8llu\n" EPILOGUE, cg_id[CHILD2]);
+ read_from_cgroup_iter(skel->progs.cgroup_id_printer, cg_fd[CHILD2],
+ BPF_CGROUP_ITER_SELF_ONLY, "test_walk_self_only_css_task");
+ ASSERT_EQ(skel->bss->css_task_cnt, 1, "css_task_cnt");
+cleanup:
+ iters_css_task__destroy(skel);
+}
+
void test_cgroup_iter(void)
{
struct cgroup_iter *skel = NULL;
@@ -293,6 +323,9 @@ void test_cgroup_iter(void)
test_walk_self_only(skel);
if (test__start_subtest("cgroup_iter__dead_self_only"))
test_walk_dead_self_only(skel);
+ if (test__start_subtest("cgroup_iter__self_only_css_task"))
+ test_walk_self_only_css_task();
+
out:
cgroup_iter__destroy(skel);
cleanup_cgroups();
diff --git a/tools/testing/selftests/bpf/progs/iters_css_task.c b/tools/testing/selftests/bpf/progs/iters_css_task.c
index 5089ce384a1c..384ff806990f 100644
--- a/tools/testing/selftests/bpf/progs/iters_css_task.c
+++ b/tools/testing/selftests/bpf/progs/iters_css_task.c
@@ -10,6 +10,7 @@
char _license[] SEC("license") = "GPL";
+struct cgroup *bpf_cgroup_acquire(struct cgroup *p) __ksym;
struct cgroup *bpf_cgroup_from_id(u64 cgid) __ksym;
void bpf_cgroup_release(struct cgroup *p) __ksym;
@@ -45,3 +46,46 @@ int BPF_PROG(iter_css_task_for_each, struct vm_area_struct *vma,
return -EPERM;
}
+
+static inline u64 cgroup_id(struct cgroup *cgrp)
+{
+ return cgrp->kn->id;
+}
+
+SEC("?iter/cgroup")
+int cgroup_id_printer(struct bpf_iter__cgroup *ctx)
+{
+ struct seq_file *seq = ctx->meta->seq;
+ struct cgroup *cgrp, *acquired;
+ struct cgroup_subsys_state *css;
+ struct task_struct *task;
+ u64 cgrp_id;
+
+ cgrp = ctx->cgroup;
+
+ /* epilogue */
+ if (cgrp == NULL) {
+ BPF_SEQ_PRINTF(seq, "epilogue\n");
+ return 0;
+ }
+
+ /* prologue */
+ if (ctx->meta->seq_num == 0)
+ BPF_SEQ_PRINTF(seq, "prologue\n");
+
+ cgrp_id = cgroup_id(cgrp);
+
+ BPF_SEQ_PRINTF(seq, "%8llu\n", cgrp_id);
+
+ acquired = bpf_cgroup_from_id(cgrp_id);
+ if (!acquired)
+ return 0;
+ css = &acquired->self;
+ css_task_cnt = 0;
+ bpf_for_each(css_task, task, css, CSS_TASK_ITER_PROCS) {
+ if (task->pid == target_pid)
+ css_task_cnt++;
+ }
+ bpf_cgroup_release(acquired);
+ return 0;
+}
--
2.20.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH bpf-next v4 3/3] selftests/bpf: Add test for using css_task iter in sleepable progs
2023-10-31 5:04 [PATCH bpf-next v4 0/3] Relax allowlist for open-coded css_task iter Chuyi Zhou
2023-10-31 5:04 ` [PATCH bpf-next v4 1/3] bpf: Relax allowlist for " Chuyi Zhou
2023-10-31 5:04 ` [PATCH bpf-next v4 2/3] selftests/bpf: Add tests for css_task iter combining with cgroup iter Chuyi Zhou
@ 2023-10-31 5:04 ` Chuyi Zhou
2023-11-02 2:52 ` Yonghong Song
2023-11-02 6:10 ` [PATCH bpf-next v4 0/3] Relax allowlist for open-coded css_task iter patchwork-bot+netdevbpf
3 siblings, 1 reply; 6+ messages in thread
From: Chuyi Zhou @ 2023-10-31 5:04 UTC (permalink / raw)
To: bpf; +Cc: ast, daniel, andrii, martin.lau, Chuyi Zhou
This Patch add a test to prove css_task iter can be used in normal
sleepable progs.
Signed-off-by: Chuyi Zhou <zhouchuyi@bytedance.com>
---
.../testing/selftests/bpf/prog_tests/iters.c | 1 +
.../selftests/bpf/progs/iters_css_task.c | 19 +++++++++++++++++++
2 files changed, 20 insertions(+)
diff --git a/tools/testing/selftests/bpf/prog_tests/iters.c b/tools/testing/selftests/bpf/prog_tests/iters.c
index c2425791c923..bf84d4a1d9ae 100644
--- a/tools/testing/selftests/bpf/prog_tests/iters.c
+++ b/tools/testing/selftests/bpf/prog_tests/iters.c
@@ -294,6 +294,7 @@ void test_iters(void)
RUN_TESTS(iters_state_safety);
RUN_TESTS(iters_looping);
RUN_TESTS(iters);
+ RUN_TESTS(iters_css_task);
if (env.has_testmod)
RUN_TESTS(iters_testmod_seq);
diff --git a/tools/testing/selftests/bpf/progs/iters_css_task.c b/tools/testing/selftests/bpf/progs/iters_css_task.c
index 384ff806990f..e180aa1b1d52 100644
--- a/tools/testing/selftests/bpf/progs/iters_css_task.c
+++ b/tools/testing/selftests/bpf/progs/iters_css_task.c
@@ -89,3 +89,22 @@ int cgroup_id_printer(struct bpf_iter__cgroup *ctx)
bpf_cgroup_release(acquired);
return 0;
}
+
+SEC("?fentry.s/" SYS_PREFIX "sys_getpgid")
+int BPF_PROG(iter_css_task_for_each_sleep)
+{
+ u64 cgrp_id = bpf_get_current_cgroup_id();
+ struct cgroup *cgrp = bpf_cgroup_from_id(cgrp_id);
+ struct cgroup_subsys_state *css;
+ struct task_struct *task;
+
+ if (cgrp == NULL)
+ return 0;
+ css = &cgrp->self;
+
+ bpf_for_each(css_task, task, css, CSS_TASK_ITER_PROCS) {
+
+ }
+ bpf_cgroup_release(cgrp);
+ return 0;
+}
--
2.20.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH bpf-next v4 3/3] selftests/bpf: Add test for using css_task iter in sleepable progs
2023-10-31 5:04 ` [PATCH bpf-next v4 3/3] selftests/bpf: Add test for using css_task iter in sleepable progs Chuyi Zhou
@ 2023-11-02 2:52 ` Yonghong Song
0 siblings, 0 replies; 6+ messages in thread
From: Yonghong Song @ 2023-11-02 2:52 UTC (permalink / raw)
To: Chuyi Zhou, bpf; +Cc: ast, daniel, andrii, martin.lau
On 10/30/23 10:04 PM, Chuyi Zhou wrote:
> This Patch add a test to prove css_task iter can be used in normal
> sleepable progs.
>
> Signed-off-by: Chuyi Zhou <zhouchuyi@bytedance.com>
Acked-by: Yonghong Song <yonghong.song@linux.dev>
> ---
> .../testing/selftests/bpf/prog_tests/iters.c | 1 +
> .../selftests/bpf/progs/iters_css_task.c | 19 +++++++++++++++++++
> 2 files changed, 20 insertions(+)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/iters.c b/tools/testing/selftests/bpf/prog_tests/iters.c
> index c2425791c923..bf84d4a1d9ae 100644
> --- a/tools/testing/selftests/bpf/prog_tests/iters.c
> +++ b/tools/testing/selftests/bpf/prog_tests/iters.c
> @@ -294,6 +294,7 @@ void test_iters(void)
> RUN_TESTS(iters_state_safety);
> RUN_TESTS(iters_looping);
> RUN_TESTS(iters);
> + RUN_TESTS(iters_css_task);
>
> if (env.has_testmod)
> RUN_TESTS(iters_testmod_seq);
> diff --git a/tools/testing/selftests/bpf/progs/iters_css_task.c b/tools/testing/selftests/bpf/progs/iters_css_task.c
> index 384ff806990f..e180aa1b1d52 100644
> --- a/tools/testing/selftests/bpf/progs/iters_css_task.c
> +++ b/tools/testing/selftests/bpf/progs/iters_css_task.c
> @@ -89,3 +89,22 @@ int cgroup_id_printer(struct bpf_iter__cgroup *ctx)
> bpf_cgroup_release(acquired);
> return 0;
> }
> +
> +SEC("?fentry.s/" SYS_PREFIX "sys_getpgid")
> +int BPF_PROG(iter_css_task_for_each_sleep)
> +{
> + u64 cgrp_id = bpf_get_current_cgroup_id();
> + struct cgroup *cgrp = bpf_cgroup_from_id(cgrp_id);
> + struct cgroup_subsys_state *css;
> + struct task_struct *task;
> +
> + if (cgrp == NULL)
> + return 0;
> + css = &cgrp->self;
> +
> + bpf_for_each(css_task, task, css, CSS_TASK_ITER_PROCS) {
> +
> + }
> + bpf_cgroup_release(cgrp);
> + return 0;
> +}
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next v4 0/3] Relax allowlist for open-coded css_task iter
2023-10-31 5:04 [PATCH bpf-next v4 0/3] Relax allowlist for open-coded css_task iter Chuyi Zhou
` (2 preceding siblings ...)
2023-10-31 5:04 ` [PATCH bpf-next v4 3/3] selftests/bpf: Add test for using css_task iter in sleepable progs Chuyi Zhou
@ 2023-11-02 6:10 ` patchwork-bot+netdevbpf
3 siblings, 0 replies; 6+ messages in thread
From: patchwork-bot+netdevbpf @ 2023-11-02 6:10 UTC (permalink / raw)
To: Chuyi Zhou; +Cc: bpf, ast, daniel, andrii, martin.lau
Hello:
This series was applied to bpf/bpf.git (master)
by Alexei Starovoitov <ast@kernel.org>:
On Tue, 31 Oct 2023 13:04:35 +0800 you wrote:
> Hi,
> The patchset aims to relax the allowlist for open-coded css_task iter
> suggested by Alexei[1].
>
> Please see individual patches for more details. And comments are always
> welcome.
>
> [...]
Here is the summary with links:
- [bpf-next,v4,1/3] bpf: Relax allowlist for css_task iter
https://git.kernel.org/bpf/bpf/c/3091b667498b
- [bpf-next,v4,2/3] selftests/bpf: Add tests for css_task iter combining with cgroup iter
https://git.kernel.org/bpf/bpf/c/f49843afde67
- [bpf-next,v4,3/3] selftests/bpf: Add test for using css_task iter in sleepable progs
https://git.kernel.org/bpf/bpf/c/d8234d47c4aa
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] 6+ messages in thread