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  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date:   Wed, 9 Sep 2020 09:34:30 -0700
From:   Andrii Nakryiko <andrii.nakryiko@...il.com>
To:     Toke Høiland-Jørgensen <toke@...hat.com>
Cc:     Alexei Starovoitov <alexei.starovoitov@...il.com>,
        Stanislav Fomichev <sdf@...gle.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 Wed, Sep 9, 2020 at 3:58 AM Toke Høiland-Jørgensen <toke@...hat.com> wrote:
>
> Andrii Nakryiko <andrii.nakryiko@...il.com> writes:
>
> > 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.
> >>
> >> >> 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?
> >
> > I know that skeleton is not an answer for you, so you'll have to find
> > DATASEC and corresponding variable offset and size (libbpf provides
> > APIs for all those operations, but you'll need to combine them
> > together). Then mmap() map and then you can do partial updates. There
> > is no other way to update only portions of an ARRAY map, except
> > through memory-mapping.
>
> Well, I wouldn't mind having to go digging through the section. But is
> it really possible to pick out and modify parts of it my mmap() before
> the object is loaded (and the map frozen)? How? I seem to recall we
> added bpf_map__set_initial_value() because this was *not* possible with
> the public API?
>

Ah, right, .rodata is frozen on load, forgot we are talking about .rodata here.


> Also, for this, a bpf_map__get_initial_value() could be a simple way to
> allow partial modifications. The caller could just get the whole map
> value, modify it, and set it again afterwards with
> __set_initial_value(). Any objections to adding that?

Yeah, I think having an API for getting initial map value makes sense.
But please follow the naming convention for getters and call it
bpf_map__initial_value().

>
> -Toke
>

Powered by blists - more mailing lists