Re: [PATCH bpf] libbpf: fix global variable relocation
From: Yonghong Song <hidden>
Date: 2019-11-27 18:19:59
Also in:
bpf
On 11/26/19 10:01 PM, Andrii Nakryiko wrote:
Similarly to a0d7da26ce86 ("libbpf: Fix call relocation offset calculation
bug"), relocations against global variables need to take into account
referenced symbol's st_value, which holds offset into a corresponding data
section (and, subsequently, offset into internal backing map). For static
variables this offset is always zero and data offset is completely described
by respective instruction's imm field.
Convert a bunch of selftests to global variables. Previously they were relying
on `static volatile` trick to ensure Clang doesn't inline static variables,
which with global variables is not necessary anymore.
Fixes: 393cdfbee809 ("libbpf: Support initialized global variables")
Signed-off-by: Andrii Nakryiko <redacted>LGTM with a few nits below. Acked-by: Yonghong Song <redacted>
quoted hunk ↗ jump to hunk
--- tools/lib/bpf/libbpf.c | 40 +++++++++---------- .../testing/selftests/bpf/progs/fentry_test.c | 12 +++--- .../selftests/bpf/progs/fexit_bpf2bpf.c | 6 +-- .../testing/selftests/bpf/progs/fexit_test.c | 12 +++--- tools/testing/selftests/bpf/progs/test_mmap.c | 4 +- 5 files changed, 35 insertions(+), 39 deletions(-)diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c index b20f82e58989..4209b5a23a53 100644 --- a/tools/lib/bpf/libbpf.c +++ b/tools/lib/bpf/libbpf.c@@ -171,10 +171,8 @@ struct bpf_program { RELO_DATA, } type; int insn_idx; - union { - int map_idx; - int text_off; - }; + int map_idx; + int sym_off; } *reloc_desc; int nr_reloc; int log_level;
[...]
quoted hunk ↗ jump to hunk
@@ -3622,31 +3622,27 @@ bpf_program__relocate(struct bpf_program *prog, struct bpf_object *obj) return 0; for (i = 0; i < prog->nr_reloc; i++) { + struct reloc_desc *relo = &prog->reloc_desc[i]; + if (prog->reloc_desc[i].type == RELO_LD64 || prog->reloc_desc[i].type == RELO_DATA) {
Using relo->type == RELO_LD64 or RELO_DATA?
- bool relo_data = prog->reloc_desc[i].type == RELO_DATA;
- struct bpf_insn *insns = prog->insns;
- int insn_idx, map_idx;
-
- insn_idx = prog->reloc_desc[i].insn_idx;
- map_idx = prog->reloc_desc[i].map_idx;
+ struct bpf_insn *insn = &prog->insns[relo->insn_idx];
- if (insn_idx + 1 >= (int)prog->insns_cnt) {
+ if (relo->insn_idx + 1 >= (int)prog->insns_cnt) {
pr_warn("relocation out of range: '%s'\n",
prog->section_name);
return -LIBBPF_ERRNO__RELOC;
}
- if (!relo_data) {
- insns[insn_idx].src_reg = BPF_PSEUDO_MAP_FD;
+ if (relo->type != RELO_DATA) {
+ insn[0].src_reg = BPF_PSEUDO_MAP_FD;
} else {
- insns[insn_idx].src_reg = BPF_PSEUDO_MAP_VALUE;
- insns[insn_idx + 1].imm = insns[insn_idx].imm;
+ insn[0].src_reg = BPF_PSEUDO_MAP_VALUE;
+ insn[1].imm = insn[0].imm + relo->sym_off;
}
- insns[insn_idx].imm = obj->maps[map_idx].fd;
+ insn[0].imm = obj->maps[relo->map_idx].fd;
} else if (prog->reloc_desc[i].type == RELO_CALL) {Using relo->type == RELO_CALL?
quoted hunk ↗ jump to hunk
- err = bpf_program__reloc_text(prog, obj, - &prog->reloc_desc[i]); + err = bpf_program__reloc_text(prog, obj, relo); if (err) return err; }diff --git a/tools/testing/selftests/bpf/progs/fentry_test.c b/tools/testing/selftests/bpf/progs/fentry_test.c index d2af9f039df5..615f7c6bca77 100644 --- a/tools/testing/selftests/bpf/progs/fentry_test.c +++ b/tools/testing/selftests/bpf/progs/fentry_test.c@@ -6,28 +6,28 @@
[...]