[PATCH 2/2] perf bpf: Check relocation target section

From: Wang Nan
Date: Fri Jan 22 2016 - 04:28:37 EST


Libbpf should check target section before doing relocation to ensure
the relocation is correct. If not, a bug in LLVM causes error. See [1].
Also, if an incorrect BPF script uses both global variable and
map, global variable whould be treated as map and be relocated
without error.

This patch saves id of map section into obj->efile and compare
target section of a relocation symbol against it during relocation.

Previous patch introduces a test case about this problem.
After this patch:

# ~/perf test BPF
37: Test BPF filter :
37.1: Test basic BPF filtering : Ok
37.2: Test BPF prologue generation : Ok
37.3: Test BPF relocation checker : Ok

# perf test -v BPF
...
37.3: Test BPF relocation checker :
...
libbpf: loading object '[bpf_relocation_test]' from buffer
libbpf: section .strtab, size 126, link 0, flags 0, type=3
libbpf: section .text, size 0, link 0, flags 6, type=1
libbpf: section .data, size 0, link 0, flags 3, type=1
libbpf: section .bss, size 0, link 0, flags 3, type=8
libbpf: section func=sys_write, size 104, link 0, flags 6, type=1
libbpf: found program func=sys_write
libbpf: section .relfunc=sys_write, size 16, link 10, flags 0, type=9
libbpf: section maps, size 16, link 0, flags 3, type=1
libbpf: maps in [bpf_relocation_test]: 16 bytes
libbpf: section license, size 4, link 0, flags 3, type=1
libbpf: license of [bpf_relocation_test] is GPL
libbpf: section version, size 4, link 0, flags 3, type=1
libbpf: kernel version of [bpf_relocation_test] is 40400
libbpf: section .symtab, size 144, link 1, flags 0, type=2
libbpf: map 0 is "my_table"
libbpf: collecting relocating info for: 'func=sys_write'
libbpf: Program 'func=sys_write' contains non-map related relo data pointing to section 65522
bpf: failed to load buffer
Compile BPF program failed.
test child finished with 0
---- end ----
Test BPF filter subtest 2: Ok

[1] https://llvm.org/bugs/show_bug.cgi?id=26243

Signed-off-by: Wang Nan <wangnan0@xxxxxxxxxx>
Cc: Alexei Starovoitov <ast@xxxxxxxxxx>
Cc: Arnaldo Carvalho de Melo <acme@xxxxxxxxxx>
Cc: Daniel Borkmann <daniel@xxxxxxxxxxxxx>
Cc: Li Zefan <lizefan@xxxxxxxxxx>
Cc: pi3orama@xxxxxxx
---
tools/lib/bpf/libbpf.c | 34 ++++++++++++++++++++++------------
1 file changed, 22 insertions(+), 12 deletions(-)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 8334a5a..8fa781f 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -201,6 +201,7 @@ struct bpf_object {
Elf_Data *data;
} *reloc;
int nr_reloc;
+ int maps_shndx;
} efile;
/*
* All loaded bpf_object is linked in a list, which is
@@ -350,6 +351,7 @@ static struct bpf_object *bpf_object__new(const char *path,
*/
obj->efile.obj_buf = obj_buf;
obj->efile.obj_buf_sz = obj_buf_sz;
+ obj->efile.maps_shndx = -1;

obj->loaded = false;

@@ -529,12 +531,12 @@ bpf_object__init_maps(struct bpf_object *obj, void *data,
}

static int
-bpf_object__init_maps_name(struct bpf_object *obj, int maps_shndx)
+bpf_object__init_maps_name(struct bpf_object *obj)
{
int i;
Elf_Data *symbols = obj->efile.symbols;

- if (!symbols || maps_shndx < 0)
+ if (!symbols || obj->efile.maps_shndx < 0)
return -EINVAL;

for (i = 0; i < symbols->d_size / sizeof(GElf_Sym); i++) {
@@ -544,7 +546,7 @@ bpf_object__init_maps_name(struct bpf_object *obj, int maps_shndx)

if (!gelf_getsym(symbols, i, &sym))
continue;
- if (sym.st_shndx != maps_shndx)
+ if (sym.st_shndx != obj->efile.maps_shndx)
continue;

map_name = elf_strptr(obj->efile.elf,
@@ -572,7 +574,7 @@ static int bpf_object__elf_collect(struct bpf_object *obj)
Elf *elf = obj->efile.elf;
GElf_Ehdr *ep = &obj->efile.ehdr;
Elf_Scn *scn = NULL;
- int idx = 0, err = 0, maps_shndx = -1;
+ int idx = 0, err = 0;

/* Elf is corrupted/truncated, avoid calling elf_strptr. */
if (!elf_rawdata(elf_getscn(elf, ep->e_shstrndx), NULL)) {
@@ -625,7 +627,7 @@ static int bpf_object__elf_collect(struct bpf_object *obj)
else if (strcmp(name, "maps") == 0) {
err = bpf_object__init_maps(obj, data->d_buf,
data->d_size);
- maps_shndx = idx;
+ obj->efile.maps_shndx = idx;
} else if (sh.sh_type == SHT_SYMTAB) {
if (obj->efile.symbols) {
pr_warning("bpf: multiple SYMTAB in %s\n",
@@ -674,8 +676,8 @@ static int bpf_object__elf_collect(struct bpf_object *obj)
pr_warning("Corrupted ELF file: index of strtab invalid\n");
return LIBBPF_ERRNO__FORMAT;
}
- if (maps_shndx >= 0)
- err = bpf_object__init_maps_name(obj, maps_shndx);
+ if (obj->efile.maps_shndx >= 0)
+ err = bpf_object__init_maps_name(obj);
out:
return err;
}
@@ -697,7 +699,8 @@ bpf_object__find_prog_by_idx(struct bpf_object *obj, int idx)
static int
bpf_program__collect_reloc(struct bpf_program *prog,
size_t nr_maps, GElf_Shdr *shdr,
- Elf_Data *data, Elf_Data *symbols)
+ Elf_Data *data, Elf_Data *symbols,
+ int maps_shndx)
{
int i, nrels;

@@ -724,9 +727,6 @@ bpf_program__collect_reloc(struct bpf_program *prog,
return -LIBBPF_ERRNO__FORMAT;
}

- insn_idx = rel.r_offset / sizeof(struct bpf_insn);
- pr_debug("relocation: insn_idx=%u\n", insn_idx);
-
if (!gelf_getsym(symbols,
GELF_R_SYM(rel.r_info),
&sym)) {
@@ -735,6 +735,15 @@ bpf_program__collect_reloc(struct bpf_program *prog,
return -LIBBPF_ERRNO__FORMAT;
}

+ if (sym.st_shndx != maps_shndx) {
+ pr_warning("Program '%s' contains non-map related relo data pointing to section %u\n",
+ prog->section_name, sym.st_shndx);
+ return -LIBBPF_ERRNO__RELOC;
+ }
+
+ insn_idx = rel.r_offset / sizeof(struct bpf_insn);
+ pr_debug("relocation: insn_idx=%u\n", insn_idx);
+
if (insns[insn_idx].code != (BPF_LD | BPF_IMM | BPF_DW)) {
pr_warning("bpf: relocation: invalid relo for insns[%d].code 0x%x\n",
insn_idx, insns[insn_idx].code);
@@ -863,7 +872,8 @@ static int bpf_object__collect_reloc(struct bpf_object *obj)

err = bpf_program__collect_reloc(prog, nr_maps,
shdr, data,
- obj->efile.symbols);
+ obj->efile.symbols,
+ obj->efile.maps_shndx);
if (err)
return err;
}
--
1.8.3.4