netdev - Re: [PATCH bpf-next v3 3/8] libbpf: Add BPF_PROG_BIND_MAP syscall and use it on .metadata section

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, 8 Sep 2020 08:19:59 -0700
From:   Stanislav Fomichev <sdf@...gle.com>
To:     Toke Høiland-Jørgensen <toke@...hat.com>
Cc:     Andrii Nakryiko <andrii.nakryiko@...il.com>,
        Alexei Starovoitov <alexei.starovoitov@...il.com>,
        Networking <netdev@...r.kernel.org>, bpf <bpf@...r.kernel.org>,
        "David S. Miller" <davem@...emloft.net>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        YiFei Zhu <zhuyifei@...gle.com>,
        YiFei Zhu <zhuyifei1999@...il.com>,
        Andrey Ignatov <rdna@...com>
Subject: Re: [PATCH bpf-next v3 3/8] libbpf: Add BPF_PROG_BIND_MAP syscall and
 use it on .metadata section

On Mon, Sep 7, 2020 at 1:49 AM Toke Høiland-Jørgensen <toke@...hat.com> wrote:
>
> Andrii Nakryiko <andrii.nakryiko@...il.com> writes:
>
> >> May be we should talk about problem statement and goals.
> >> Do we actually need metadata per program or metadata per single .o
> >> or metadata per final .o with multiple .o linked together?
> >> What is this metadata?
> >
> > Yep, that's a very valid question. I've also CC'ed Andrey.
>
> For the libxdp use case, I need metadata per program. But I'm already
> sticking that in a single section and disambiguating by struct name
> (just prefixing the function name with a _ ), so I think it's fine to
> have this kind of "concatenated metadata" per elf file and parse out the
> per-program information from that. This is similar to the BTF-encoded
> "metadata" we can do today.
We've come full circle :-)
I think we discussed that approach originally - to stick everything
into existing global .data/.rodata and use some variable prefix for
the metadata. I'm fine with that approach. The only thing I don't
understand is - why bother with the additional .rodata.metadata
section and merging?
Can we unconditionally do BPF_PROG_BIND_MAP(.rodata) from libbpf (and
ignore the error) and be done?

Sticking to the original question: for our use-case, the metadata is
per .o file. I'm not sure how it would work in the 'multiple .o linked
together' use case. Ideally, we'd need to preserve all metadata?

> >> If it's just unreferenced by program read only data then no special names or
> >> prefixes are needed. We can introduce BPF_PROG_BIND_MAP to bind any map to any
> >> program and it would be up to tooling to decide the meaning of the data in the
> >> map. For example, bpftool can choose to print all variables from all read only
> >> maps that match "bpf_metadata_" prefix, but it will be bpftool convention only
> >> and not hard coded in libbpf.
> >
> > Agree as well. It feels a bit odd for libbpf to handle ".metadata"
> > specially, given libbpf itself doesn't care about its contents at all.
> >
> > So thanks for bringing this up, I think this is an important
> > discussion to have.
>
> I'm fine with having this be part of .rodata. One drawback, though, is
> that if any metadata is defined, it becomes a bit more complicated to
> use bpf_map__set_initial_value() because that now also has to include
> the metadata. Any way we can improve upon that?
Right. One additional thing we wanted this metadata to have is the
comm of the process who loaded this bpf program (to be filled/added by
libbpf).
I suppose .rodata.metadata section can help with that?