On Thu, Feb 6, 2025 at 10:34 AM Blaise Boscaccy
<bboscaccy@xxxxxxxxxxxxxxxxxxx> wrote:
>
> Andrii Nakryiko <andrii.nakryiko@xxxxxxxxx> writes:
>
> > On Wed, Feb 5, 2025 at 11:09 AM Blaise Boscaccy
> > <bboscaccy@xxxxxxxxxxxxxxxxxxx> wrote:
> >>
> >> Add a flexible mechanism, using existing ELF constructs, to attach
> >> additional metadata to BPF programs for possible use by BPF
> >> gatekeepers and skeletons.
> >>
> >> During object file parsing, note sections are no longer skipped and
> >> now treated as read-only data. During libbpf-based loading or skeleton
> >> generation, those sections are then transformed into read-only maps
> >> which are subsequently passed into the kernel.
> >
> > We already have this mechanism, it's .rodata (and
> > .rodata.<customname>) section(s). Adding .note sections as BPF maps
> > make no sense to me. Just piggy-back on .rodata for storing any
> > necessary metadata.
> >
> > pw-bot: cr
> >
>
> The ELF specification clearly states:
> "Sometimes a vendor or system builder needs to mark an object file with
> special information that other programs will check for conformance,
> compatibility, etc. Sections of type SHT_NOTE and program header
> elements of type PT_NOTE can be used for this purpose."
Does ELF specification say anything about "and libbpf should create a
BPF map for those SHT_NOTE sections"?
>
> Which is exactly what we are trying to do. They make no mention of
> piggy-backing off of .rodata.
You are trying to redefine non-loadable SHT_NOTE data into loadable
data backed by a BPF map. I'm not sure how your arguments are
supporting the hack you are trying to do. We are not going to start
creating new BPF maps for any random SHT_NOTE section in the BPF
object file. Use .rodata if you want to get some read-only data into
the kernel.
>
> Further, one can generally strip away .note sections and be left with an
> object that's still functioning. The same cannot be said about .rodata.
>
>
>
> >>
> >> Signed-off-by: Blaise Boscaccy <bboscaccy@xxxxxxxxxxxxxxxxxxx>
> >> ---
> >> tools/bpf/bpftool/gen.c | 4 ++--
> >> tools/lib/bpf/libbpf.c | 6 ++++++
> >> 2 files changed, 8 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/tools/bpf/bpftool/gen.c b/tools/bpf/bpftool/gen.c
> >> index 5a4d3240689ed..311d6a3f1c4bb 100644
> >> --- a/tools/bpf/bpftool/gen.c
> >> +++ b/tools/bpf/bpftool/gen.c
> >> @@ -92,7 +92,7 @@ static void get_header_guard(char *guard, const char *obj_name, const char *suff
> >>
> >> static bool get_map_ident(const struct bpf_map *map, char *buf, size_t buf_sz)
> >> {
> >> - static const char *sfxs[] = { ".data", ".rodata", ".bss", ".kconfig" };
> >> + static const char *sfxs[] = { ".data", ".rodata", ".bss", ".kconfig", ".note" };
> >> const char *name = bpf_map__name(map);
> >> int i, n;
> >>
> >> @@ -117,7 +117,7 @@ static bool get_map_ident(const struct bpf_map *map, char *buf, size_t buf_sz)
> >>
> >> static bool get_datasec_ident(const char *sec_name, char *buf, size_t buf_sz)
> >> {
> >> - static const char *pfxs[] = { ".data", ".rodata", ".bss", ".kconfig" };
> >> + static const char *pfxs[] = { ".data", ".rodata", ".bss", ".kconfig", ".note" };
> >> int i, n;
> >>
> >> /* recognize hard coded LLVM section name */
> >> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> >> index 194809da51725..be6af0fece040 100644
> >> --- a/tools/lib/bpf/libbpf.c
> >> +++ b/tools/lib/bpf/libbpf.c
> >> @@ -523,6 +523,7 @@ struct bpf_struct_ops {
> >> #define STRUCT_OPS_SEC ".struct_ops"
> >> #define STRUCT_OPS_LINK_SEC ".struct_ops.link"
> >> #define ARENA_SEC ".addr_space.1"
> >> +#define NOTE_SEC ".note"
> >>
> >> enum libbpf_map_type {
> >> LIBBPF_MAP_UNSPEC,
> >> @@ -3977,6 +3978,11 @@ static int bpf_object__elf_collect(struct bpf_object *obj)
> >> sec_desc->sec_type = SEC_BSS;
> >> sec_desc->shdr = sh;
> >> sec_desc->data = data;
> >> + } else if (sh->sh_type == SHT_NOTE && (strcmp(name, NOTE_SEC) == 0 ||
> >> + str_has_pfx(name, NOTE_SEC "."))) {
> >> + sec_desc->sec_type = SEC_RODATA;
> >> + sec_desc->shdr = sh;
> >> + sec_desc->data = data;
> >> } else {
> >> pr_info("elf: skipping section(%d) %s (size %zu)\n", idx, name,
> >> (size_t)sh->sh_size);
> >> --
> >> 2.48.1
> >>
