[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <20191127060144.3066500-1-andriin@fb.com>
Date: Tue, 26 Nov 2019 22:01:44 -0800
From: Andrii Nakryiko <andriin@...com>
To: <bpf@...r.kernel.org>, <netdev@...r.kernel.org>, <ast@...com>,
<daniel@...earbox.net>
CC: <andrii.nakryiko@...il.com>, <kernel-team@...com>,
Andrii Nakryiko <andriin@...com>
Subject: [PATCH bpf] libbpf: fix global variable relocation
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 <andriin@...com>
---
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;
@@ -1824,7 +1822,7 @@ static int bpf_program__record_reloc(struct bpf_program *prog,
}
reloc_desc->type = RELO_CALL;
reloc_desc->insn_idx = insn_idx;
- reloc_desc->text_off = sym->st_value / 8;
+ reloc_desc->sym_off = sym->st_value;
obj->has_pseudo_calls = true;
return 0;
}
@@ -1868,6 +1866,7 @@ static int bpf_program__record_reloc(struct bpf_program *prog,
reloc_desc->type = RELO_LD64;
reloc_desc->insn_idx = insn_idx;
reloc_desc->map_idx = map_idx;
+ reloc_desc->sym_off = 0; /* sym->st_value determines map_idx */
return 0;
}
@@ -1899,6 +1898,7 @@ static int bpf_program__record_reloc(struct bpf_program *prog,
reloc_desc->type = RELO_DATA;
reloc_desc->insn_idx = insn_idx;
reloc_desc->map_idx = map_idx;
+ reloc_desc->sym_off = sym->st_value;
return 0;
}
@@ -3563,8 +3563,8 @@ bpf_program__reloc_text(struct bpf_program *prog, struct bpf_object *obj,
return -LIBBPF_ERRNO__RELOC;
if (prog->idx == obj->efile.text_shndx) {
- pr_warn("relo in .text insn %d into off %d\n",
- relo->insn_idx, relo->text_off);
+ pr_warn("relo in .text insn %d into off %d (insn #%d)\n",
+ relo->insn_idx, relo->sym_off, relo->sym_off / 8);
return -LIBBPF_ERRNO__RELOC;
}
@@ -3599,7 +3599,7 @@ bpf_program__reloc_text(struct bpf_program *prog, struct bpf_object *obj,
prog->section_name);
}
insn = &prog->insns[relo->insn_idx];
- insn->imm += relo->text_off + prog->main_prog_cnt - relo->insn_idx;
+ insn->imm += relo->sym_off / 8 + prog->main_prog_cnt - relo->insn_idx;
return 0;
}
@@ -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) {
- 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) {
- 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 @@
char _license[] SEC("license") = "GPL";
-static volatile __u64 test1_result;
+__u64 test1_result = 0;
BPF_TRACE_1("fentry/bpf_fentry_test1", test1, int, a)
{
test1_result = a == 1;
return 0;
}
-static volatile __u64 test2_result;
+__u64 test2_result = 0;
BPF_TRACE_2("fentry/bpf_fentry_test2", test2, int, a, __u64, b)
{
test2_result = a == 2 && b == 3;
return 0;
}
-static volatile __u64 test3_result;
+__u64 test3_result = 0;
BPF_TRACE_3("fentry/bpf_fentry_test3", test3, char, a, int, b, __u64, c)
{
test3_result = a == 4 && b == 5 && c == 6;
return 0;
}
-static volatile __u64 test4_result;
+__u64 test4_result = 0;
BPF_TRACE_4("fentry/bpf_fentry_test4", test4,
void *, a, char, b, int, c, __u64, d)
{
@@ -35,7 +35,7 @@ BPF_TRACE_4("fentry/bpf_fentry_test4", test4,
return 0;
}
-static volatile __u64 test5_result;
+__u64 test5_result = 0;
BPF_TRACE_5("fentry/bpf_fentry_test5", test5,
__u64, a, void *, b, short, c, int, d, __u64, e)
{
@@ -44,7 +44,7 @@ BPF_TRACE_5("fentry/bpf_fentry_test5", test5,
return 0;
}
-static volatile __u64 test6_result;
+__u64 test6_result = 0;
BPF_TRACE_6("fentry/bpf_fentry_test6", test6,
__u64, a, void *, b, short, c, int, d, void *, e, __u64, f)
{
diff --git a/tools/testing/selftests/bpf/progs/fexit_bpf2bpf.c b/tools/testing/selftests/bpf/progs/fexit_bpf2bpf.c
index 525d47d7b589..2d211ee98a1c 100644
--- a/tools/testing/selftests/bpf/progs/fexit_bpf2bpf.c
+++ b/tools/testing/selftests/bpf/progs/fexit_bpf2bpf.c
@@ -8,7 +8,7 @@ struct sk_buff {
unsigned int len;
};
-static volatile __u64 test_result;
+__u64 test_result = 0;
BPF_TRACE_2("fexit/test_pkt_access", test_main,
struct sk_buff *, skb, int, ret)
{
@@ -23,7 +23,7 @@ BPF_TRACE_2("fexit/test_pkt_access", test_main,
return 0;
}
-static volatile __u64 test_result_subprog1;
+__u64 test_result_subprog1 = 0;
BPF_TRACE_2("fexit/test_pkt_access_subprog1", test_subprog1,
struct sk_buff *, skb, int, ret)
{
@@ -56,7 +56,7 @@ struct args_subprog2 {
__u64 args[5];
__u64 ret;
};
-static volatile __u64 test_result_subprog2;
+__u64 test_result_subprog2 = 0;
SEC("fexit/test_pkt_access_subprog2")
int test_subprog2(struct args_subprog2 *ctx)
{
diff --git a/tools/testing/selftests/bpf/progs/fexit_test.c b/tools/testing/selftests/bpf/progs/fexit_test.c
index 2487e98edb34..86db0d60fb6e 100644
--- a/tools/testing/selftests/bpf/progs/fexit_test.c
+++ b/tools/testing/selftests/bpf/progs/fexit_test.c
@@ -6,28 +6,28 @@
char _license[] SEC("license") = "GPL";
-static volatile __u64 test1_result;
+__u64 test1_result = 0;
BPF_TRACE_2("fexit/bpf_fentry_test1", test1, int, a, int, ret)
{
test1_result = a == 1 && ret == 2;
return 0;
}
-static volatile __u64 test2_result;
+__u64 test2_result = 0;
BPF_TRACE_3("fexit/bpf_fentry_test2", test2, int, a, __u64, b, int, ret)
{
test2_result = a == 2 && b == 3 && ret == 5;
return 0;
}
-static volatile __u64 test3_result;
+__u64 test3_result = 0;
BPF_TRACE_4("fexit/bpf_fentry_test3", test3, char, a, int, b, __u64, c, int, ret)
{
test3_result = a == 4 && b == 5 && c == 6 && ret == 15;
return 0;
}
-static volatile __u64 test4_result;
+__u64 test4_result = 0;
BPF_TRACE_5("fexit/bpf_fentry_test4", test4,
void *, a, char, b, int, c, __u64, d, int, ret)
{
@@ -37,7 +37,7 @@ BPF_TRACE_5("fexit/bpf_fentry_test4", test4,
return 0;
}
-static volatile __u64 test5_result;
+__u64 test5_result = 0;
BPF_TRACE_6("fexit/bpf_fentry_test5", test5,
__u64, a, void *, b, short, c, int, d, __u64, e, int, ret)
{
@@ -46,7 +46,7 @@ BPF_TRACE_6("fexit/bpf_fentry_test5", test5,
return 0;
}
-static volatile __u64 test6_result;
+__u64 test6_result = 0;
BPF_TRACE_7("fexit/bpf_fentry_test6", test6,
__u64, a, void *, b, short, c, int, d, void *, e, __u64, f,
int, ret)
diff --git a/tools/testing/selftests/bpf/progs/test_mmap.c b/tools/testing/selftests/bpf/progs/test_mmap.c
index 0d2ec9fbcf61..e808791b7047 100644
--- a/tools/testing/selftests/bpf/progs/test_mmap.c
+++ b/tools/testing/selftests/bpf/progs/test_mmap.c
@@ -15,8 +15,8 @@ struct {
__type(value, __u64);
} data_map SEC(".maps");
-static volatile __u64 in_val;
-static volatile __u64 out_val;
+__u64 in_val = 0;
+__u64 out_val = 0;
SEC("raw_tracepoint/sys_enter")
int test_mmap(void *ctx)
--
2.17.1
Powered by blists - more mailing lists