* [PATCH bpf v2 1/3] bpf: don't assume build-id length is always 20 bytes
@ 2019-01-16 22:03 Stanislav Fomichev
2019-01-16 22:03 ` [PATCH bpf v2 2/3] bpf: zero out build_id for BPF_STACK_BUILD_ID_IP Stanislav Fomichev
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Stanislav Fomichev @ 2019-01-16 22:03 UTC (permalink / raw)
To: netdev; +Cc: davem, ast, daniel, songliubraving, Stanislav Fomichev
Build-id length is not fixed to 20, it can be (`man ld` /--build-id):
* 128-bit (uuid)
* 160-bit (sha1)
* any length specified in ld --build-id=0xhexstring
To fix the issue of missing BPF_STACK_BUILD_ID_VALID for shorter build-ids,
assume that build-id is somewhere in the range of 1 .. 20.
Set the remaining bytes to zero.
v2:
* don't introduce new "len = min(BPF_BUILD_ID_SIZE, nhdr->n_descsz)",
we already know that nhdr->n_descsz <= BPF_BUILD_ID_SIZE if we enter
this 'if' condition
Fixes: 615755a77b24 ("bpf: extend stackmap to save binary_build_id+offset instead of address")
Acked-by: Song Liu <songliubraving@fb.com>
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
kernel/bpf/stackmap.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/kernel/bpf/stackmap.c b/kernel/bpf/stackmap.c
index d9e2483669d0..f9df545e92f6 100644
--- a/kernel/bpf/stackmap.c
+++ b/kernel/bpf/stackmap.c
@@ -180,11 +180,14 @@ static inline int stack_map_parse_build_id(void *page_addr,
if (nhdr->n_type == BPF_BUILD_ID &&
nhdr->n_namesz == sizeof("GNU") &&
- nhdr->n_descsz == BPF_BUILD_ID_SIZE) {
+ nhdr->n_descsz > 0 &&
+ nhdr->n_descsz <= BPF_BUILD_ID_SIZE) {
memcpy(build_id,
note_start + note_offs +
ALIGN(sizeof("GNU"), 4) + sizeof(Elf32_Nhdr),
- BPF_BUILD_ID_SIZE);
+ nhdr->n_descsz);
+ memset(build_id + nhdr->n_descsz, 0,
+ BPF_BUILD_ID_SIZE - nhdr->n_descsz);
return 0;
}
new_offs = note_offs + sizeof(Elf32_Nhdr) +
--
2.20.1.97.g81188d93c3-goog
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH bpf v2 2/3] bpf: zero out build_id for BPF_STACK_BUILD_ID_IP
2019-01-16 22:03 [PATCH bpf v2 1/3] bpf: don't assume build-id length is always 20 bytes Stanislav Fomichev
@ 2019-01-16 22:03 ` Stanislav Fomichev
2019-01-16 22:03 ` [PATCH bpf v2 3/3] selftests/bpf: retry tests that expect build-id Stanislav Fomichev
2019-01-17 15:55 ` [PATCH bpf v2 1/3] bpf: don't assume build-id length is always 20 bytes Daniel Borkmann
2 siblings, 0 replies; 4+ messages in thread
From: Stanislav Fomichev @ 2019-01-16 22:03 UTC (permalink / raw)
To: netdev; +Cc: davem, ast, daniel, songliubraving, Stanislav Fomichev
When returning BPF_STACK_BUILD_ID_IP from stack_map_get_build_id_offset,
make sure that build_id field is empty. Since we are using percpu
free list, there is a possibility that we might reuse some previous
bpf_stack_build_id with non-zero build_id.
Fixes: 615755a77b24 ("bpf: extend stackmap to save binary_build_id+offset instead of address")
Acked-by: Song Liu <songliubraving@fb.com>
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
kernel/bpf/stackmap.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/kernel/bpf/stackmap.c b/kernel/bpf/stackmap.c
index f9df545e92f6..d43b14535827 100644
--- a/kernel/bpf/stackmap.c
+++ b/kernel/bpf/stackmap.c
@@ -314,6 +314,7 @@ static void stack_map_get_build_id_offset(struct bpf_stack_build_id *id_offs,
for (i = 0; i < trace_nr; i++) {
id_offs[i].status = BPF_STACK_BUILD_ID_IP;
id_offs[i].ip = ips[i];
+ memset(id_offs[i].build_id, 0, BPF_BUILD_ID_SIZE);
}
return;
}
@@ -324,6 +325,7 @@ static void stack_map_get_build_id_offset(struct bpf_stack_build_id *id_offs,
/* per entry fall back to ips */
id_offs[i].status = BPF_STACK_BUILD_ID_IP;
id_offs[i].ip = ips[i];
+ memset(id_offs[i].build_id, 0, BPF_BUILD_ID_SIZE);
continue;
}
id_offs[i].offset = (vma->vm_pgoff << PAGE_SHIFT) + ips[i]
--
2.20.1.97.g81188d93c3-goog
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH bpf v2 3/3] selftests/bpf: retry tests that expect build-id
2019-01-16 22:03 [PATCH bpf v2 1/3] bpf: don't assume build-id length is always 20 bytes Stanislav Fomichev
2019-01-16 22:03 ` [PATCH bpf v2 2/3] bpf: zero out build_id for BPF_STACK_BUILD_ID_IP Stanislav Fomichev
@ 2019-01-16 22:03 ` Stanislav Fomichev
2019-01-17 15:55 ` [PATCH bpf v2 1/3] bpf: don't assume build-id length is always 20 bytes Daniel Borkmann
2 siblings, 0 replies; 4+ messages in thread
From: Stanislav Fomichev @ 2019-01-16 22:03 UTC (permalink / raw)
To: netdev; +Cc: davem, ast, daniel, songliubraving, Stanislav Fomichev
While running test_progs in a loop I found out that I'm sometimes hitting
"Didn't find expected build ID from the map" error.
Looking at stack_map_get_build_id_offset() it seems that it is racy (by
design) and can sometimes return BPF_STACK_BUILD_ID_IP (i.e. can't trylock
current->mm->mmap_sem).
Let's retry this test a single time.
Fixes: 13790d1cc72c ("bpf: add selftest for stackmap with build_id in NMI context")
Acked-by: Song Liu <songliubraving@fb.com>
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
tools/testing/selftests/bpf/test_progs.c | 30 ++++++++++++++++++++++++
1 file changed, 30 insertions(+)
diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
index 126fc624290d..25f0083a9b2e 100644
--- a/tools/testing/selftests/bpf/test_progs.c
+++ b/tools/testing/selftests/bpf/test_progs.c
@@ -1188,7 +1188,9 @@ static void test_stacktrace_build_id(void)
int i, j;
struct bpf_stack_build_id id_offs[PERF_MAX_STACK_DEPTH];
int build_id_matches = 0;
+ int retry = 1;
+retry:
err = bpf_prog_load(file, BPF_PROG_TYPE_TRACEPOINT, &obj, &prog_fd);
if (CHECK(err, "prog_load", "err %d errno %d\n", err, errno))
goto out;
@@ -1301,6 +1303,19 @@ static void test_stacktrace_build_id(void)
previous_key = key;
} while (bpf_map_get_next_key(stackmap_fd, &previous_key, &key) == 0);
+ /* stack_map_get_build_id_offset() is racy and sometimes can return
+ * BPF_STACK_BUILD_ID_IP instead of BPF_STACK_BUILD_ID_VALID;
+ * try it one more time.
+ */
+ if (build_id_matches < 1 && retry--) {
+ ioctl(pmu_fd, PERF_EVENT_IOC_DISABLE);
+ close(pmu_fd);
+ bpf_object__close(obj);
+ printf("%s:WARN:Didn't find expected build ID from the map, retrying\n",
+ __func__);
+ goto retry;
+ }
+
if (CHECK(build_id_matches < 1, "build id match",
"Didn't find expected build ID from the map\n"))
goto disable_pmu;
@@ -1341,7 +1356,9 @@ static void test_stacktrace_build_id_nmi(void)
int i, j;
struct bpf_stack_build_id id_offs[PERF_MAX_STACK_DEPTH];
int build_id_matches = 0;
+ int retry = 1;
+retry:
err = bpf_prog_load(file, BPF_PROG_TYPE_PERF_EVENT, &obj, &prog_fd);
if (CHECK(err, "prog_load", "err %d errno %d\n", err, errno))
return;
@@ -1436,6 +1453,19 @@ static void test_stacktrace_build_id_nmi(void)
previous_key = key;
} while (bpf_map_get_next_key(stackmap_fd, &previous_key, &key) == 0);
+ /* stack_map_get_build_id_offset() is racy and sometimes can return
+ * BPF_STACK_BUILD_ID_IP instead of BPF_STACK_BUILD_ID_VALID;
+ * try it one more time.
+ */
+ if (build_id_matches < 1 && retry--) {
+ ioctl(pmu_fd, PERF_EVENT_IOC_DISABLE);
+ close(pmu_fd);
+ bpf_object__close(obj);
+ printf("%s:WARN:Didn't find expected build ID from the map, retrying\n",
+ __func__);
+ goto retry;
+ }
+
if (CHECK(build_id_matches < 1, "build id match",
"Didn't find expected build ID from the map\n"))
goto disable_pmu;
--
2.20.1.97.g81188d93c3-goog
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH bpf v2 1/3] bpf: don't assume build-id length is always 20 bytes
2019-01-16 22:03 [PATCH bpf v2 1/3] bpf: don't assume build-id length is always 20 bytes Stanislav Fomichev
2019-01-16 22:03 ` [PATCH bpf v2 2/3] bpf: zero out build_id for BPF_STACK_BUILD_ID_IP Stanislav Fomichev
2019-01-16 22:03 ` [PATCH bpf v2 3/3] selftests/bpf: retry tests that expect build-id Stanislav Fomichev
@ 2019-01-17 15:55 ` Daniel Borkmann
2 siblings, 0 replies; 4+ messages in thread
From: Daniel Borkmann @ 2019-01-17 15:55 UTC (permalink / raw)
To: Stanislav Fomichev, netdev; +Cc: davem, ast, songliubraving
On 01/16/2019 11:03 PM, Stanislav Fomichev wrote:
> Build-id length is not fixed to 20, it can be (`man ld` /--build-id):
> * 128-bit (uuid)
> * 160-bit (sha1)
> * any length specified in ld --build-id=0xhexstring
>
> To fix the issue of missing BPF_STACK_BUILD_ID_VALID for shorter build-ids,
> assume that build-id is somewhere in the range of 1 .. 20.
> Set the remaining bytes to zero.
>
> v2:
> * don't introduce new "len = min(BPF_BUILD_ID_SIZE, nhdr->n_descsz)",
> we already know that nhdr->n_descsz <= BPF_BUILD_ID_SIZE if we enter
> this 'if' condition
>
> Fixes: 615755a77b24 ("bpf: extend stackmap to save binary_build_id+offset instead of address")
> Acked-by: Song Liu <songliubraving@fb.com>
> Signed-off-by: Stanislav Fomichev <sdf@google.com>
Series applied, thanks!
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-01-17 15:56 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-01-16 22:03 [PATCH bpf v2 1/3] bpf: don't assume build-id length is always 20 bytes Stanislav Fomichev
2019-01-16 22:03 ` [PATCH bpf v2 2/3] bpf: zero out build_id for BPF_STACK_BUILD_ID_IP Stanislav Fomichev
2019-01-16 22:03 ` [PATCH bpf v2 3/3] selftests/bpf: retry tests that expect build-id Stanislav Fomichev
2019-01-17 15:55 ` [PATCH bpf v2 1/3] bpf: don't assume build-id length is always 20 bytes Daniel Borkmann
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).