[PATCH bpf v2 0/2] Let BPF verifier consider {task,cgroup} is trusted in bpf_iter_reg

[PATCH bpf v2 0/2] Let BPF verifier consider {task,cgroup} is trusted in bpf_iter_reg

[Chuyi Zhou]

Hi,
The patchset aims to let the BPF verivier consider
bpf_iter__cgroup->cgroup and bpf_iter__task->task is trused suggested by
Alexei[1].

Please see individual patches for more details. And comments are always
welcome.

Link[1]:https://lore.kernel.org/bpf/20231022154527.229117-1-zhouchuyi@bytedance.com/T/#mb57725edc8ccdd50a1b165765c7619b4d65ed1b0

v2->v1:
 * Patch #1: Add Yonghong's ack and add description of similar case in
   log.
 * Patch #2: Add Yonghong's ack

Chuyi Zhou (2):
  bpf: Let verifier consider {task,cgroup} is trusted in bpf_iter_reg
  selftests/bpf: get trusted cgrp from bpf_iter__cgroup directly

 kernel/bpf/cgroup_iter.c                         |  2 +-
 kernel/bpf/task_iter.c                           |  2 +-
 .../testing/selftests/bpf/progs/iters_css_task.c | 16 ++++------------
 3 files changed, 6 insertions(+), 14 deletions(-)

-- 
2.20.1

[PATCH bpf v2 1/2] bpf: Let verifier consider {task,cgroup} is trusted in bpf_iter_reg

[Chuyi Zhou]

BTF_TYPE_SAFE_TRUSTED(struct bpf_iter__task) in verifier.c wanted to
teach BPF verifier that bpf_iter__task -> task is a trusted ptr. But it
doesn't work well.

The reason is, bpf_iter__task -> task would go through btf_ctx_access()
which enforces the reg_type of 'task' is ctx_arg_info->reg_type, and in
task_iter.c, we actually explicitly declare that the
ctx_arg_info->reg_type is PTR_TO_BTF_ID_OR_NULL.

Actually we have a previous case like this[1] where PTR_TRUSTED is added to
the arg flag for map_iter.

This patch sets ctx_arg_info->reg_type is PTR_TO_BTF_ID_OR_NULL |
PTR_TRUSTED in task_reg_info.

Similarly, bpf_cgroup_reg_info -> cgroup is also PTR_TRUSTED since we are
under the protection of cgroup_mutex and we would check cgroup_is_dead()
in __cgroup_iter_seq_show().

Link[1]:https://lore.kernel.org/all/20230706133932.45883-3-aspsk@isovalent.com/

Signed-off-by: Chuyi Zhou <zhouchuyi@bytedance.com>
Acked-by: Yonghong Song <yonghong.song@linux.dev>
---
 kernel/bpf/cgroup_iter.c | 2 +-
 kernel/bpf/task_iter.c   | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/bpf/cgroup_iter.c b/kernel/bpf/cgroup_iter.c
index d1b5c5618..f04a468cf 100644
--- a/kernel/bpf/cgroup_iter.c
+++ b/kernel/bpf/cgroup_iter.c
@@ -282,7 +282,7 @@ static struct bpf_iter_reg bpf_cgroup_reg_info = {
 	.ctx_arg_info_size	= 1,
 	.ctx_arg_info		= {
 		{ offsetof(struct bpf_iter__cgroup, cgroup),
-		  PTR_TO_BTF_ID_OR_NULL },
+		  PTR_TO_BTF_ID_OR_NULL | PTR_TRUSTED },
 	},
 	.seq_info		= &cgroup_iter_seq_info,
 };
diff --git a/kernel/bpf/task_iter.c b/kernel/bpf/task_iter.c
index 4e156dca4..26082b978 100644
--- a/kernel/bpf/task_iter.c
+++ b/kernel/bpf/task_iter.c
@@ -704,7 +704,7 @@ static struct bpf_iter_reg task_reg_info = {
 	.ctx_arg_info_size	= 1,
 	.ctx_arg_info		= {
 		{ offsetof(struct bpf_iter__task, task),
-		  PTR_TO_BTF_ID_OR_NULL },
+		  PTR_TO_BTF_ID_OR_NULL | PTR_TRUSTED },
 	},
 	.seq_info		= &task_seq_info,
 	.fill_link_info		= bpf_iter_fill_link_info,
-- 
2.20.1

[PATCH bpf v2 2/2] selftests/bpf: get trusted cgrp from bpf_iter__cgroup directly

[Chuyi Zhou]

Commit f49843afde (selftests/bpf: Add tests for css_task iter combining
with cgroup iter) added a test which demonstrates how css_task iter can be
combined with cgroup iter. That test used bpf_cgroup_from_id() to convert
bpf_iter__cgroup->cgroup to a trusted ptr which is pointless now, since
with the previous fix, we can get a trusted cgroup directly from
bpf_iter__cgroup.

Signed-off-by: Chuyi Zhou <zhouchuyi@bytedance.com>
Acked-by: Yonghong Song <yonghong.song@linux.dev>
---
 .../testing/selftests/bpf/progs/iters_css_task.c | 16 ++++------------
 1 file changed, 4 insertions(+), 12 deletions(-)

diff --git a/tools/testing/selftests/bpf/progs/iters_css_task.c b/tools/testing/selftests/bpf/progs/iters_css_task.c
index e180aa1b1..9ac758649 100644
--- a/tools/testing/selftests/bpf/progs/iters_css_task.c
+++ b/tools/testing/selftests/bpf/progs/iters_css_task.c
@@ -56,12 +56,9 @@ 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 *cgrp = ctx->cgroup;
 	struct cgroup_subsys_state *css;
 	struct task_struct *task;
-	u64 cgrp_id;
-
-	cgrp = ctx->cgroup;
 
 	/* epilogue */
 	if (cgrp == NULL) {
@@ -73,20 +70,15 @@ int cgroup_id_printer(struct bpf_iter__cgroup *ctx)
 	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);
+	BPF_SEQ_PRINTF(seq, "%8llu\n", cgroup_id(cgrp));
 
-	acquired = bpf_cgroup_from_id(cgrp_id);
-	if (!acquired)
-		return 0;
-	css = &acquired->self;
+	css = &cgrp->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

Re: [PATCH bpf v2 0/2] Let BPF verifier consider {task,cgroup} is trusted in bpf_iter_reg

[patchwork-bot+netdevbpf]

Hello:

This series was applied to bpf/bpf.git (master)
by Martin KaFai Lau <martin.lau@kernel.org>:

On Tue,  7 Nov 2023 21:22:02 +0800 you wrote:
> Hi,
> The patchset aims to let the BPF verivier consider
> bpf_iter__cgroup->cgroup and bpf_iter__task->task is trused suggested by
> Alexei[1].
> 
> Please see individual patches for more details. And comments are always
> welcome.
> 
> [...]

Here is the summary with links:
  - [bpf,v2,1/2] bpf: Let verifier consider {task,cgroup} is trusted in bpf_iter_reg
    https://git.kernel.org/bpf/bpf/c/0de4f50de25a
  - [bpf,v2,2/2] selftests/bpf: get trusted cgrp from bpf_iter__cgroup directly
    https://git.kernel.org/bpf/bpf/c/3c5864ba9cf9

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html