netdev - Re: [PATCH v2 bpf-next 2/4] bpf: allow bpf_d_path in sleepable bpf

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: <CANA3-0d1oVEZh2_ypPWwO6DaOin38M--tipEPKZD4s5NANYfdA@mail.gmail.com>
Date:   Wed, 16 Dec 2020 19:31:47 +0100
From:   KP Singh <kpsingh@...nel.org>
To:     bpf <bpf@...r.kernel.org>, Networking <netdev@...r.kernel.org>
Cc:     KP Singh <kpsingh@...omium.org>
Subject: Re: [PATCH v2 bpf-next 2/4] bpf: allow bpf_d_path in sleepable
 bpf_iter program

On Wed, Dec 16, 2020 at 7:15 PM KP Singh <kpsingh@...nel.org> wrote:
>
> On Wed, Dec 16, 2020 at 1:06 AM Song Liu <songliubraving@...com> wrote:
> >
> > task_file and task_vma iter programs have access to file->f_path. Enable
> > bpf_d_path to print paths of these file.
> >
> > bpf_iter programs are generally called in sleepable context. However, it
> > is still necessary to diffientiate sleepable and non-sleepable bpf_iter
> > programs: sleepable programs have access to bpf_d_path; non-sleepable
> > programs have access to bpf_spin_lock.
> >
> > Signed-off-by: Song Liu <songliubraving@...com>
> > ---
> >  kernel/trace/bpf_trace.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> > index 4be771df5549a..9e5f9b968355f 100644
> > --- a/kernel/trace/bpf_trace.c
> > +++ b/kernel/trace/bpf_trace.c
> > @@ -1191,6 +1191,11 @@ BTF_SET_END(btf_allowlist_d_path)
> >
> >  static bool bpf_d_path_allowed(const struct bpf_prog *prog)
> >  {
> > +       if (prog->type == BPF_PROG_TYPE_TRACING &&
> > +           prog->expected_attach_type == BPF_TRACE_ITER &&
> > +           prog->aux->sleepable)
> > +               return true;
>

Another try to send it on the list.

> For the sleepable/non-sleepable we have been (until now) checking
> this in bpf_tracing_func_proto (or bpf_lsm_func_proto)
>
> eg.
>
> case BPF_FUNC_copy_from_user:
> return prog->aux->sleepable ? &bpf_copy_from_user_proto : NULL;
>
> But even beyond that, I don't think this is needed.
>
> We have originally exposed the helper to both sleepable and
> non-sleepable LSM and tracing programs with an allow list.
>
> For LSM the allow list is bpf_lsm_is_sleepable_hook) but
> that's just an initial allow list and thus causes some confusion
> w.r.t to sleep ability (maybe we should add a comment there).
>
> Based on the current logic, my understanding is that
> it's okay to use the helper in the allowed hooks in both
> "lsm.s/" and "lsm/" (and the same for
> BPF_PROG_TYPE_TRACING).
>
> We would have required sleepable only if this helper called "dput"
> (which can sleep).
>
> > +
> >         if (prog->type == BPF_PROG_TYPE_LSM)
> >                 return bpf_lsm_is_sleepable_hook(prog->aux->attach_btf_id);
> >
> > --
> > 2.24.1
> >