linux-kernel - Re: [PATCH bpf-next v5 6/8] bpf: Let bpf_iter_task

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAEf4BzbWQPxm3in9LBeKq50xT1rNLaZ_8X931FqrK=PEatxAfA@mail.gmail.com>
Date:   Fri, 13 Oct 2023 14:27:26 -0700
From:   Andrii Nakryiko <andrii.nakryiko@...il.com>
To:     Chuyi Zhou <zhouchuyi@...edance.com>
Cc:     bpf@...r.kernel.org, ast@...nel.org, daniel@...earbox.net,
        andrii@...nel.org, martin.lau@...nel.org, tj@...nel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH bpf-next v5 6/8] bpf: Let bpf_iter_task_new accept null
 task ptr

On Wed, Oct 11, 2023 at 5:09 AM Chuyi Zhou <zhouchuyi@...edance.com> wrote:
>
> When using task_iter to iterate all threads of a specific task, we enforce
> that the user must pass a valid task pointer to ensure safety. However,
> when iterating all threads/process in the system, BPF verifier still
> require a valid ptr instead of "nullable" pointer, even though it's
> pointless, which is a kind of surprising from usability standpoint. It
> would be nice if we could let that kfunc accept a explicit null pointer
> when we are using BPF_TASK_ITER_ALL_{PROCS, THREADS} and a valid pointer
> when using BPF_TASK_ITER_THREAD.
>
> Given a trival kfunc:
>         __bpf_kfunc void FN(struct TYPE_A *obj);
>
> BPF Prog would reject a nullptr for obj. The error info is:
> "arg#x pointer type xx xx must point to scalar, or struct with scalar"
> reported by get_kfunc_ptr_arg_type(). The reg->type is SCALAR_VALUE and
> the btf type of ref_t is not scalar or scalar_struct which leads to the
> rejection of get_kfunc_ptr_arg_type.
>
> This patch add "__nullable" annotation:
>         __bpf_kfunc void FN(struct TYPE_A *obj__nullable);
> Here __nullable indicates obj can be optional, user can pass a explicit
> nullptr or a normal TYPE_A pointer. In get_kfunc_ptr_arg_type(), we will
> detect whether the current arg is optional and register is null, If so,
> return a new kfunc_ptr_arg_type KF_ARG_PTR_TO_NULL and skip to the next
> arg in check_kfunc_args().
>
> Signed-off-by: Chuyi Zhou <zhouchuyi@...edance.com>
> ---
>  kernel/bpf/task_iter.c |  7 +++++--
>  kernel/bpf/verifier.c  | 13 ++++++++++++-
>  2 files changed, 17 insertions(+), 3 deletions(-)
>

Looks good to me, but someone better versed in kfunc internals should
double-check.

Acked-by: Andrii Nakryiko <andrii@...nel.org>

> diff --git a/kernel/bpf/task_iter.c b/kernel/bpf/task_iter.c
> index caeddad3d2f1..0772545568f1 100644
> --- a/kernel/bpf/task_iter.c
> +++ b/kernel/bpf/task_iter.c
> @@ -873,7 +873,7 @@ enum {
>  };
>

[...]