netdev - Re: [PATCH bpf-next] libbpf: Add bpf_object_

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: <87eetem1dm.fsf@toke.dk>
Date:   Fri, 27 Mar 2020 13:24:37 +0100
From:   Toke Høiland-Jørgensen <toke@...hat.com>
To:     Andrii Nakryiko <andrii.nakryiko@...il.com>
Cc:     Daniel Borkmann <daniel@...earbox.net>,
        Alexei Starovoitov <ast@...com>, bpf <bpf@...r.kernel.org>,
        Networking <netdev@...r.kernel.org>
Subject: Re: [PATCH bpf-next] libbpf: Add bpf_object__rodata getter function

Andrii Nakryiko <andrii.nakryiko@...il.com> writes:

> On Thu, Mar 26, 2020 at 8:18 AM Toke Høiland-Jørgensen <toke@...hat.com> wrote:
>>
>> This adds a new getter function to libbpf to get the rodata area of a bpf
>> object. This is useful if a program wants to modify the rodata before
>> loading the object. Any such modification needs to be done before loading,
>> since libbpf freezes the backing map after populating it (to allow the
>> kernel to do dead code elimination based on its contents).
>>
>> Signed-off-by: Toke Høiland-Jørgensen <toke@...hat.com>
>> ---
>>  tools/lib/bpf/libbpf.c   | 13 +++++++++++++
>>  tools/lib/bpf/libbpf.h   |  1 +
>>  tools/lib/bpf/libbpf.map |  1 +
>>  3 files changed, 15 insertions(+)
>>
>> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
>> index 085e41f9b68e..d3e3bbe12f78 100644
>> --- a/tools/lib/bpf/libbpf.c
>> +++ b/tools/lib/bpf/libbpf.c
>> @@ -1352,6 +1352,19 @@ bpf_object__init_internal_map(struct bpf_object *obj, enum libbpf_map_type type,
>>         return 0;
>>  }
>>
>> +void *bpf_object__rodata(const struct bpf_object *obj, size_t *size)
>
> We probably don't want to expose this API. It just doesn't scale,
> especially if/when we add support for custom sections names for global
> variables.

Right. I was not aware of any such plans, but OK.

> Also checking for map->mmaped is too restrictive. See how BPF skeleton
> solves this problem and still allows .rodata initialization even on
> kernels that don't support memory-mapping global variables.

Not sure what you mean here? As far as I can tell, the map->mmaped
pointer has nothing to do with the kernel support for mmaping the map
contents. It's just what libbpf does to store the data of any
internal_maps?

I mean, bpf_object__open_skeleton() just does this:

		if (mmaped && (*map)->libbpf_type != LIBBPF_MAP_KCONFIG)
			*mmaped = (*map)->mmaped;

which amounts to the same as I'm doing in this patch?

> But basically, why can't you use BPF skeleton?

Couple of reasons:

- I don't need any of the other features of the skeleton
- I don't want to depend on bpftool in the build process
- I don't want to embed the BPF bytecode into the C object

> Also, application can already find that map by looking at name.

Yes, it can find the map, but it can't access the data. But I guess I
could just add a getter for that. Just figured this was easier to
consume; but I can see why it might impose restrictions on future
changes, so I'll send a v2 with such a map-level getter instead.

-Toke