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

[Yonghong Song <yonghong.song@linux.dev>]

On 11/5/23 5:34 AM, Chuyi Zhou wrote:
> 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.
>
> This patch sets ctx_arg_info->reg_type is PTR_TO_BTF_ID_OR_NULL |
> PTR_TRUSTED in task_reg_info.

Actually we have a previous case like this. See

  https://lore.kernel.org/all/20230706133932.45883-3-aspsk@isovalent.com/

where PTR_TRUSTED is added to the arg flag for map_iter.

You could mention this case in your commit message.

>
> 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().
>
> 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,