netdev - Re: [RFC PATCH bpf-next 05/16] bpf: create file or anonymous dumpers

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: <20200411231125.haqk5by4p34wudn7@ast-mbp>
Date:   Sat, 11 Apr 2020 16:11:25 -0700
From:   Alexei Starovoitov <alexei.starovoitov@...il.com>
To:     Yonghong Song <yhs@...com>
Cc:     Andrii Nakryiko <andrii.nakryiko@...il.com>,
        Andrii Nakryiko <andriin@...com>, bpf <bpf@...r.kernel.org>,
        Martin KaFai Lau <kafai@...com>,
        Networking <netdev@...r.kernel.org>,
        Alexei Starovoitov <ast@...com>,
        Daniel Borkmann <daniel@...earbox.net>,
        Kernel Team <kernel-team@...com>
Subject: Re: [RFC PATCH bpf-next 05/16] bpf: create file or anonymous dumpers

On Fri, Apr 10, 2020 at 04:47:36PM -0700, Yonghong Song wrote:
> > 
> > Instead of special-casing dumper_name, can we require specifying full
> > path, and then check whether it is in BPF FS vs BPFDUMP FS? If the
> > latter, additionally check that it is in the right sub-directory
> > matching its intended target type.
> 
> We could. I just think specifying full path for bpfdump is not necessary
> since it is a single user mount...
> 
> > 
> > But honestly, just doing everything within BPF FS starts to seem
> > cleaner at this point...
> 
> bpffs is multi mount, which is not a perfect fit for bpfdump,
> considering mounting inside namespace, etc, all dumpers are gone.

As Yonghong pointed out reusing bpffs for dumpers doesn't look possible
from implementation perspective.
Even if it was possible the files in such mix-and-match file system
would be of different kinds with different semantics. I think that
will lead to mediocre user experience when file 'foo' is cat-able
with nice human output, but file 'bar' isn't cat-able at all because
it's just a pinned map. imo having all dumpers in one fixed location
in /sys/kernel/bpfdump makes it easy to discover for folks who might
not even know what bpf is.
For example when I'm trying to learn some new area of the kernel I might go
poke around /proc and /sys directory looking for a file name that could be
interesting to 'cat'. This is how I discovered /sys/kernel/slab/ :)
I think keeping all dumpers in /sys/kernel/bpfdump/ will make them
similarly discoverable.

re: f_dump flag...
May be it's a sign that pinning is not the right name for such operation?
If kernel cannot distinguish pinning dumper prog into bpffs as a vanilla
pinning operation vs pinning into bpfdumpfs to make it cat-able then something
isn't right about api. Either it needs to be a new bpf syscall command (like
install_dumper_in_dumpfs) or reuse pinning command, but make libbpf specify the
full path. From bpf prog point of view it may still specify only the final
name, but libbpf can prepend the /sys/kernel/bpfdump/.../. May be there is a
third option. Extra flag for pinning just doesn't look right. What if we do
another specialized file system later? It would need yet another flag to pin
there?