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

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20180206160057.jxf7qc64jq2gmrf2@ast-mbp.dhcp.thefacebook.com>
Date:   Tue, 6 Feb 2018 08:00:59 -0800
From:   Alexei Starovoitov <alexei.starovoitov@...il.com>
To:     Jesper Dangaard Brouer <brouer@...hat.com>
Cc:     netdev@...r.kernel.org, Daniel Borkmann <borkmann@...earbox.net>,
        wangnan0@...wei.com, jakub.kicinski@...ronome.com, joe@....org,
        acme@...hat.com, eric@...it.org, yhs@...com
Subject: Re: [bpf-next V2 PATCH 5/5] tools/libbpf: handle issues with bpf ELF
 objects containing .eh_frames

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 ?
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?

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

> Thus, my solution is simply to match on the name of the relocation
> section, to skip that too.
> 
> Note, for samples/bpf/ the '-target bpf' parameter to clang cannot be used
> due to incompatibility with asm embedded headers, that some of the samples
> include. This is explained in more details by Yonghong Song in bpf_devel_QA.
> 
> Signed-off-by: Jesper Dangaard Brouer <brouer@...hat.com>
> ---
>  tools/lib/bpf/libbpf.c |    7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index b4eeaa3ebff5..84e8bbe07347 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -822,6 +822,13 @@ static int bpf_object__elf_collect(struct bpf_object *obj)
>  			void *reloc = obj->efile.reloc;
>  			int nr_reloc = obj->efile.nr_reloc + 1;
>  
> +			/* Skip decoding of "eh" exception frames */
> +			if (strcmp(name, ".rel.eh_frame") == 0) {
> +				pr_debug("skip relo section %s(%d) for section(%d)\n",
> +					 name, idx, sh.sh_info);
> +				continue;
> +			}
> +
>  			reloc = realloc(reloc,
>  					sizeof(*obj->efile.reloc) * nr_reloc);
>  			if (!reloc) {
>