Re: [bpf-next V2 PATCH 5/5] tools/libbpf: handle issues with bpf ELF objects containing .eh_frames

[Jesper Dangaard Brouer <brouer@redhat.com>]

On Tue, 6 Feb 2018 08:00:59 -0800 Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:

> On Tue, Feb 06, 2018 at 03:54:28PM +0100, Jesper Dangaard Brouer wrote:
> > If clang >= 4.0.1 is missing the option '-target bpf', it will cause
> > llc/llvm to create two ELF sections for "Exception Frames", with
> > section names '.eh_frame' and '.rel.eh_frame'.
> > 
> > The BPF ELF loader library libbpf fails when loading files with these
> > sections.  The other in-kernel BPF ELF loader in samples/bpf/bpf_load.c,
> > handle this gracefully. And iproute2 loader also seems to work with these
> > "eh" sections.
> > 
> > The issue in libbpf is caused by bpf_object__elf_collect() skip the
> > '.eh_frame' and thus doesn't create an internal data structure
> > pointing to this ELF section index.  Later when the relocation section
> > '.rel.eh_frame' is processed, it tries to find the '.eh_frame' via the
> > ELF section idx, which is that fails (in bpf_object__collect_reloc).
> > 
> > I couldn't find a way to see that the '.rel.eh_frame' was irrelevant
> > (that is only determined by looking at the section it reference, which
> > we no longer have info available on).  
> 
> but does this approach work for all extra sections and relocations emitted
> when source is compiled with -g ?

No, but I plan to follow up and do a more complete solution later. This
is a workaround to get the Suricata use-case working and also that
samples/bpf/ can be loaded.

> To address this case bpf_load.c does:
>   if (shdr.sh_type == SHT_REL) {
>           struct bpf_insn *insns;
> 
>           /* locate prog sec that need map fixup (relocations) */
>           if (get_sec(elf, shdr.sh_info, &ehdr, &shname_prog,
>                       &shdr_prog, &data_prog))
>                   continue;
> 
>           if (shdr_prog.sh_type != SHT_PROGBITS ||
>               !(shdr_prog.sh_flags & SHF_EXECINSTR))
>                   continue;
> 
> why the same approach is not applicable here?

As described above bpf_object__elf_collect() skip the "real" section
that the relo-section want to lookup (based on the same kind of
check), but libbpf is now missing the section idx in its internal
structures... and thus the relo lookup of the idx fails. (bpf_load.c
does the lookup in the ELF obj directly, thus it does not have this
problem).


> I guess we can apply this workaround as-is but it looks incomplete.

Yes, it is a workaround to move forward... it requires a larger change
to libbpf, so it stores idx'es of skipped sections.

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer