netdev - Re: [PATCH v4 bpf-next 00/14] bpf: Add d

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]

Date:   Tue, 30 Jun 2020 15:55:38 +0200
From:   Jiri Olsa <jolsa@...hat.com>
To:     Andrii Nakryiko <andrii.nakryiko@...il.com>
Cc:     Jiri Olsa <jolsa@...nel.org>, Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Networking <netdev@...r.kernel.org>, bpf <bpf@...r.kernel.org>,
        Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>,
        Martin KaFai Lau <kafai@...com>,
        David Miller <davem@...hat.com>,
        John Fastabend <john.fastabend@...il.com>,
        Wenbo Zhang <ethercflow@...il.com>,
        KP Singh <kpsingh@...omium.org>,
        Andrii Nakryiko <andriin@...com>,
        Brendan Gregg <bgregg@...flix.com>,
        Florent Revest <revest@...omium.org>,
        Al Viro <viro@...iv.linux.org.uk>
Subject: Re: [PATCH v4 bpf-next 00/14] bpf: Add d_path helper

On Mon, Jun 29, 2020 at 06:54:30PM -0700, Andrii Nakryiko wrote:
> On Thu, Jun 25, 2020 at 4:47 PM Jiri Olsa <jolsa@...nel.org> wrote:
> >
> > hi,
> > adding d_path helper to return full path for 'path' object.
> >
> > In a preparation for that, this patchset also adds support for BTF ID
> > whitelists, because d_path can't be called from any probe due to its
> > locks usage. The whitelists allow verifier to check if the caller is
> > one of the functions from the whitelist.
> >
> > The whitelist is implemented in a generic way. This patchset introduces
> > macros that allow to define lists of BTF IDs, which are compiled in
> > the kernel image in a new .BTF.ids ELF section.
> >
> > The generic way of BTF ID lists allows us to use them in other places
> > in kernel (than just for whitelists), that could use static BTF ID
> > values compiled in and it's also implemented in this patchset.
> >
> > I originally added and used 'file_path' helper, which did the same,
> > but used 'struct file' object. Then realized that file_path is just
> > a wrapper for d_path, so we'd cover more calling sites if we add
> > d_path helper and allowed resolving BTF object within another object,
> > so we could call d_path also with file pointer, like:
> >
> >   bpf_d_path(&file->f_path, buf, size);
> >
> > This feature is mainly to be able to add dpath (filepath originally)
> > function to bpftrace:
> >
> >   # bpftrace -e 'kfunc:vfs_open { printf("%s\n", dpath(args->path)); }'
> >
> > v4 changes:
> >   - added ID sanity checks in btf_resolve_helper_id [Andrii]
> >   - resolve bpf_ctx_convert via BTF_ID [Andrii]
> >   - keep bpf_access_type in btf_struct_access [Andrii]
> >   - rename whitelist to se and use struct btf_id_set [Andrii]
> >   - several fixes for d_path prog/verifier tests [Andrii]
> >   - added union and typedefs types support [Andrii]
> >   - rename btfid to resolve_btfids [Andrii]
> >   - fix segfault in resolve_btfids [John]
> >   - rename section from .BTF_ids .BTF.ids (following .BTF.ext example)
> >   - add .BTF.ids section info into btf.rst [John]
> >   - updated over letter with more details [John]
> >
> > Also available at:
> >   https://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
> >   bpf/d_path
> >
> > thanks,
> > jirka
> >
> >
> > ---
> 
> Have you considered splitting this series into two? One with BTF ID
> resolution and corresponding patches. I'm pretty confident in that one
> and it seems ready (with some minor selftest changes). Then,
> separately, d_path and that sub-struct address logic. That one depends
> on the first one, but shouldn't really block BTF ID resolution from
> going in sooner.

ok, will split it and send the first part for now

jirka