在 2023/10/30 22:37, Chuyi Zhou 写道:
Hello,
在 2023/10/24 01:14, Alexei Starovoitov 写道:
On Mon, Oct 23, 2023 at 6:50 AM Chuyi Zhou <zhouchuyi@xxxxxxxxxxxxx>
wrote:
R1_w=ptr_task_struct(off=0,imm=0) R7_w=ptr_task_struct(off=0,imm=0)
18: (85) call bpf_task_acquire#26990
R1 must be a rcu pointer
I will try to figure out it.
Thanks. That would be great.
So far it looks like a regression.
I'm guessing __bpf_md_ptr wrapping is confusing the verifier.
Since it's more complicated than I thought, please respin
the current set with fixes to patch 1 and leave the patch 2 as-is.
That can be another follow up.
After adding this change:
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 15d71d2986d3..4565e5457754 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -6071,6 +6071,8 @@ bool btf_ctx_access(int off, int size, enum
bpf_access_type type,
info->reg_type = ctx_arg_info->reg_type;
info->btf = btf_vmlinux;
info->btf_id = ctx_arg_info->btf_id;
+ if (prog_args_trusted(prog))
+ info->reg_type |= PTR_TRUSTED;
return true;
}
}
the task pointer would be recognized as 'trusted_ptr_or_null_task_struct'.
The VERIFIER LOG:
=============
reg type unsupported for arg#0 function dump_task#7
0: R1=ctx(off=0,imm=0) R10=fp0
; struct task_struct *task = ctx->task;
0: (79) r1 = *(u64 *)(r1 +8) ;
R1_w=trusted_ptr_or_null_task_struct(id=1,off=0,imm=0)
The following BPF Prog works well.
SEC("iter/task")
int dump_task(struct bpf_iter__task *ctx)
{
struct task_struct *task = ctx->task;
struct task_struct *acq;
if (task == NULL)
return 0;
acq = bpf_task_acquire(task);
if (!acq)
return 0;
bpf_task_release(acq);
return 0;
}
bpf_iter__task->meta would be recognized as a trusted ptr.
In btf_ctx_access(), we would add PTR_TRUSTED flag if
prog_args_trusted(prog) return true.
It seems for BPF_TRACE_ITER, ctx access is always 'trusted'.
But I noticed that in task_iter.c, we actually explicitly declare that
the type of bpf_iter__task->task is PTR_TO_BTF_ID_OR_NULL.
static struct bpf_iter_reg task_reg_info = {
.target = "task",
.attach_target = bpf_iter_attach_task,
.feature = BPF_ITER_RESCHED,
.ctx_arg_info_size = 1,
.ctx_arg_info = {
{ offsetof(struct bpf_iter__task, task),
PTR_TO_BTF_ID_OR_NULL },
},
.seq_info = &task_seq_info,
.fill_link_info = bpf_iter_fill_link_info,
.show_fdinfo = bpf_iter_task_show_fdinfo,
};
btf_ctx_access() would enforce the reg_type is
ctx_arg_info->reg_type(PTR_TO_BTF_ID_OR_NULL) for bpf_iter__task->task:
bool btf_ctx_access()
....
/* this is a pointer to another type */
for (i = 0; i < prog->aux->ctx_arg_info_size; i++) {
const struct bpf_ctx_arg_aux *ctx_arg_info = &prog->aux->ctx_arg_info[i];
if (ctx_arg_info->offset == off) {
...
info->reg_type = ctx_arg_info->reg_type;
info->btf = btf_vmlinux;
info->btf_id = ctx_arg_info->btf_id;
return true;
}
}
So, maybe another possible solution is:
diff --git a/kernel/bpf/cgroup_iter.c b/kernel/bpf/cgroup_iter.c
index 209e5135f9fb..72a6778e3fba 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 | MEM_RCU },
},
.seq_info = &cgroup_iter_seq_info,
};
diff --git a/kernel/bpf/task_iter.c b/kernel/bpf/task_iter.c
index 59e747938bdb..4fd3f734dffd 100644
--- a/kernel/bpf/task_iter.c
+++ b/kernel/bpf/task_iter.c
@@ -706,7 +706,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,
Just some thoughts.
Thanks.
