* [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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.