* [PATCH bpf 1/3] bpf: don't assume build-id length is always 20 bytes
@ 2019-01-15 22:54 Stanislav Fomichev
2019-01-15 22:54 ` [PATCH bpf 2/3] bpf: zero out build_id for BPF_STACK_BUILD_ID_IP Stanislav Fomichev
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Stanislav Fomichev @ 2019-01-15 22:54 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.
Fixes: 615755a77b24 ("bpf: extend stackmap to save binary_build_id+offset instead of address")
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
kernel/bpf/stackmap.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/kernel/bpf/stackmap.c b/kernel/bpf/stackmap.c
index d9e2483669d0..8975d1768dcb 100644
--- a/kernel/bpf/stackmap.c
+++ b/kernel/bpf/stackmap.c
@@ -180,11 +180,15 @@ 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) {
+ __u32 len = min_t(__u32,
+ BPF_BUILD_ID_SIZE, nhdr->n_descsz);
memcpy(build_id,
note_start + note_offs +
ALIGN(sizeof("GNU"), 4) + sizeof(Elf32_Nhdr),
- BPF_BUILD_ID_SIZE);
+ len);
+ memset(build_id + len, 0, BPF_BUILD_ID_SIZE - len);
return 0;
}
new_offs = note_offs + sizeof(Elf32_Nhdr) +
--
2.20.1.97.g81188d93c3-goog
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH bpf 2/3] bpf: zero out build_id for BPF_STACK_BUILD_ID_IP 2019-01-15 22:54 [PATCH bpf 1/3] bpf: don't assume build-id length is always 20 bytes Stanislav Fomichev @ 2019-01-15 22:54 ` Stanislav Fomichev 2019-01-16 17:48 ` Song Liu 2019-01-15 22:54 ` [PATCH bpf 3/3] selftests/bpf: retry tests that expect build-id Stanislav Fomichev 2019-01-16 17:45 ` [PATCH bpf 1/3] bpf: don't assume build-id length is always 20 bytes Song Liu 2 siblings, 1 reply; 11+ messages in thread From: Stanislav Fomichev @ 2019-01-15 22:54 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") 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 8975d1768dcb..f4b57c68c45f 100644 --- a/kernel/bpf/stackmap.c +++ b/kernel/bpf/stackmap.c @@ -315,6 +315,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; } @@ -325,6 +326,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] 11+ messages in thread
* Re: [PATCH bpf 2/3] bpf: zero out build_id for BPF_STACK_BUILD_ID_IP 2019-01-15 22:54 ` [PATCH bpf 2/3] bpf: zero out build_id for BPF_STACK_BUILD_ID_IP Stanislav Fomichev @ 2019-01-16 17:48 ` Song Liu 0 siblings, 0 replies; 11+ messages in thread From: Song Liu @ 2019-01-16 17:48 UTC (permalink / raw) To: Stanislav Fomichev Cc: Netdev, davem@davemloft.net, ast@kernel.org, daniel@iogearbox.net > On Jan 15, 2019, at 2:54 PM, Stanislav Fomichev <sdf@google.com> wrote: > > 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") > Signed-off-by: Stanislav Fomichev <sdf@google.com> Acked-by: Song Liu <songliubraving@fb.com> > --- > kernel/bpf/stackmap.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/kernel/bpf/stackmap.c b/kernel/bpf/stackmap.c > index 8975d1768dcb..f4b57c68c45f 100644 > --- a/kernel/bpf/stackmap.c > +++ b/kernel/bpf/stackmap.c > @@ -315,6 +315,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; > } > @@ -325,6 +326,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 [flat|nested] 11+ messages in thread
* [PATCH bpf 3/3] selftests/bpf: retry tests that expect build-id 2019-01-15 22:54 [PATCH bpf 1/3] bpf: don't assume build-id length is always 20 bytes Stanislav Fomichev 2019-01-15 22:54 ` [PATCH bpf 2/3] bpf: zero out build_id for BPF_STACK_BUILD_ID_IP Stanislav Fomichev @ 2019-01-15 22:54 ` Stanislav Fomichev 2019-01-16 17:49 ` Song Liu 2019-01-16 17:45 ` [PATCH bpf 1/3] bpf: don't assume build-id length is always 20 bytes Song Liu 2 siblings, 1 reply; 11+ messages in thread From: Stanislav Fomichev @ 2019-01-15 22:54 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") 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] 11+ messages in thread
* Re: [PATCH bpf 3/3] selftests/bpf: retry tests that expect build-id 2019-01-15 22:54 ` [PATCH bpf 3/3] selftests/bpf: retry tests that expect build-id Stanislav Fomichev @ 2019-01-16 17:49 ` Song Liu 0 siblings, 0 replies; 11+ messages in thread From: Song Liu @ 2019-01-16 17:49 UTC (permalink / raw) To: Stanislav Fomichev Cc: Netdev, davem@davemloft.net, ast@kernel.org, daniel@iogearbox.net > On Jan 15, 2019, at 2:54 PM, Stanislav Fomichev <sdf@google.com> wrote: > > 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") > Signed-off-by: Stanislav Fomichev <sdf@google.com> Acked-by: Song Liu <songliubraving@fb.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 [flat|nested] 11+ messages in thread
* Re: [PATCH bpf 1/3] bpf: don't assume build-id length is always 20 bytes 2019-01-15 22:54 [PATCH bpf 1/3] bpf: don't assume build-id length is always 20 bytes Stanislav Fomichev 2019-01-15 22:54 ` [PATCH bpf 2/3] bpf: zero out build_id for BPF_STACK_BUILD_ID_IP Stanislav Fomichev 2019-01-15 22:54 ` [PATCH bpf 3/3] selftests/bpf: retry tests that expect build-id Stanislav Fomichev @ 2019-01-16 17:45 ` Song Liu 2019-01-16 17:50 ` Stanislav Fomichev 2019-01-16 18:11 ` [PATCH bpf v2 " Stanislav Fomichev 2 siblings, 2 replies; 11+ messages in thread From: Song Liu @ 2019-01-16 17:45 UTC (permalink / raw) To: Stanislav Fomichev Cc: Netdev, davem@davemloft.net, ast@kernel.org, daniel@iogearbox.net > On Jan 15, 2019, at 2:54 PM, Stanislav Fomichev <sdf@google.com> 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. > > Fixes: 615755a77b24 ("bpf: extend stackmap to save binary_build_id+offset instead of address") > Signed-off-by: Stanislav Fomichev <sdf@google.com> > --- > kernel/bpf/stackmap.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/kernel/bpf/stackmap.c b/kernel/bpf/stackmap.c > index d9e2483669d0..8975d1768dcb 100644 > --- a/kernel/bpf/stackmap.c > +++ b/kernel/bpf/stackmap.c > @@ -180,11 +180,15 @@ 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) { > + __u32 len = min_t(__u32, > + BPF_BUILD_ID_SIZE, nhdr->n_descsz); Given the check above, we only need len = nhdr->n_descsz, right? Other than this, Acked-by: Song Liu <songliubraving@fb.com> Thanks for the fix! > memcpy(build_id, > note_start + note_offs + > ALIGN(sizeof("GNU"), 4) + sizeof(Elf32_Nhdr), > - BPF_BUILD_ID_SIZE); > + len); > + memset(build_id + len, 0, BPF_BUILD_ID_SIZE - len); > return 0; > } > new_offs = note_offs + sizeof(Elf32_Nhdr) + > -- > 2.20.1.97.g81188d93c3-goog > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH bpf 1/3] bpf: don't assume build-id length is always 20 bytes 2019-01-16 17:45 ` [PATCH bpf 1/3] bpf: don't assume build-id length is always 20 bytes Song Liu @ 2019-01-16 17:50 ` Stanislav Fomichev 2019-01-16 18:11 ` [PATCH bpf v2 " Stanislav Fomichev 1 sibling, 0 replies; 11+ messages in thread From: Stanislav Fomichev @ 2019-01-16 17:50 UTC (permalink / raw) To: Song Liu Cc: Netdev, davem@davemloft.net, ast@kernel.org, daniel@iogearbox.net On Wed, Jan 16, 2019 at 9:45 AM Song Liu <songliubraving@fb.com> wrote: > > > > > On Jan 15, 2019, at 2:54 PM, Stanislav Fomichev <sdf@google.com> 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. > > > > Fixes: 615755a77b24 ("bpf: extend stackmap to save binary_build_id+offset instead of address") > > Signed-off-by: Stanislav Fomichev <sdf@google.com> > > --- > > kernel/bpf/stackmap.c | 8 ++++++-- > > 1 file changed, 6 insertions(+), 2 deletions(-) > > > > diff --git a/kernel/bpf/stackmap.c b/kernel/bpf/stackmap.c > > index d9e2483669d0..8975d1768dcb 100644 > > --- a/kernel/bpf/stackmap.c > > +++ b/kernel/bpf/stackmap.c > > @@ -180,11 +180,15 @@ 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) { > > + __u32 len = min_t(__u32, > > + BPF_BUILD_ID_SIZE, nhdr->n_descsz); > > Given the check above, we only need len = nhdr->n_descsz, right? Ah, correct, I'll fix in v2. I initially had without `if (nhdr->n_descsz <= BPF_BUILD_ID_SIZE)` and clamped it here, but then decided that clamping is probably bad as well. > > Other than this, > > Acked-by: Song Liu <songliubraving@fb.com> > > Thanks for the fix! > > > memcpy(build_id, > > note_start + note_offs + > > ALIGN(sizeof("GNU"), 4) + sizeof(Elf32_Nhdr), > > - BPF_BUILD_ID_SIZE); > > + len); > > + memset(build_id + len, 0, BPF_BUILD_ID_SIZE - len); > > return 0; > > } > > new_offs = note_offs + sizeof(Elf32_Nhdr) + > > -- > > 2.20.1.97.g81188d93c3-goog > > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH bpf v2 1/3] bpf: don't assume build-id length is always 20 bytes 2019-01-16 17:45 ` [PATCH bpf 1/3] bpf: don't assume build-id length is always 20 bytes Song Liu 2019-01-16 17:50 ` Stanislav Fomichev @ 2019-01-16 18:11 ` Stanislav Fomichev 2019-01-16 18:20 ` Song Liu 2019-01-16 21:59 ` Daniel Borkmann 1 sibling, 2 replies; 11+ messages in thread From: Stanislav Fomichev @ 2019-01-16 18:11 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") 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] 11+ messages in thread
* Re: [PATCH bpf v2 1/3] bpf: don't assume build-id length is always 20 bytes 2019-01-16 18:11 ` [PATCH bpf v2 " Stanislav Fomichev @ 2019-01-16 18:20 ` Song Liu 2019-01-16 21:59 ` Daniel Borkmann 1 sibling, 0 replies; 11+ messages in thread From: Song Liu @ 2019-01-16 18:20 UTC (permalink / raw) To: Stanislav Fomichev Cc: Netdev, davem@davemloft.net, ast@kernel.org, daniel@iogearbox.net > On Jan 16, 2019, at 10:11 AM, Stanislav Fomichev <sdf@google.com> 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") > Signed-off-by: Stanislav Fomichev <sdf@google.com> Acked-by: Song Liu <songliubraving@fb.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 [flat|nested] 11+ messages in thread
* Re: [PATCH bpf v2 1/3] bpf: don't assume build-id length is always 20 bytes 2019-01-16 18:11 ` [PATCH bpf v2 " Stanislav Fomichev 2019-01-16 18:20 ` Song Liu @ 2019-01-16 21:59 ` Daniel Borkmann 2019-01-16 22:01 ` Stanislav Fomichev 1 sibling, 1 reply; 11+ messages in thread From: Daniel Borkmann @ 2019-01-16 21:59 UTC (permalink / raw) To: Stanislav Fomichev, netdev; +Cc: davem, ast, songliubraving On 01/16/2019 07:11 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") > Signed-off-by: Stanislav Fomichev <sdf@google.com> Hmm, looks like rest of the v2 series didn't make it to the list. Please double check; just in case simply resend the full v2 set so it properly ends up in patchwork. Thanks, Daniel ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH bpf v2 1/3] bpf: don't assume build-id length is always 20 bytes 2019-01-16 21:59 ` Daniel Borkmann @ 2019-01-16 22:01 ` Stanislav Fomichev 0 siblings, 0 replies; 11+ messages in thread From: Stanislav Fomichev @ 2019-01-16 22:01 UTC (permalink / raw) To: Daniel Borkmann; +Cc: Stanislav Fomichev, netdev, davem, ast, songliubraving On 01/16, Daniel Borkmann wrote: > On 01/16/2019 07:11 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") > > Signed-off-by: Stanislav Fomichev <sdf@google.com> > > Hmm, looks like rest of the v2 series didn't make it to the list. > Please double check; just in case simply resend the full v2 set so > it properly ends up in patchwork. Oh, I didn't send the first two patches because I didn't change them, I'll resend full v2 series in a moment. > > Thanks, > Daniel ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2019-01-16 22:01 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-01-15 22:54 [PATCH bpf 1/3] bpf: don't assume build-id length is always 20 bytes Stanislav Fomichev 2019-01-15 22:54 ` [PATCH bpf 2/3] bpf: zero out build_id for BPF_STACK_BUILD_ID_IP Stanislav Fomichev 2019-01-16 17:48 ` Song Liu 2019-01-15 22:54 ` [PATCH bpf 3/3] selftests/bpf: retry tests that expect build-id Stanislav Fomichev 2019-01-16 17:49 ` Song Liu 2019-01-16 17:45 ` [PATCH bpf 1/3] bpf: don't assume build-id length is always 20 bytes Song Liu 2019-01-16 17:50 ` Stanislav Fomichev 2019-01-16 18:11 ` [PATCH bpf v2 " Stanislav Fomichev 2019-01-16 18:20 ` Song Liu 2019-01-16 21:59 ` Daniel Borkmann 2019-01-16 22:01 ` Stanislav Fomichev
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).