* [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; 13+ 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] 13+ 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; 13+ 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] 13+ 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; 13+ 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] 13+ 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; 13+ 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] 13+ 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; 13+ 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] 13+ 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; 13+ 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] 13+ 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; 13+ 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] 13+ 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; 13+ 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] 13+ 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; 13+ 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] 13+ 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; 13+ 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] 13+ 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; 13+ 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] 13+ messages in thread
* [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-17 15:55 ` Daniel Borkmann
0 siblings, 1 reply; 13+ 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] 13+ 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 Stanislav Fomichev
@ 2019-01-17 15:55 ` Daniel Borkmann
0 siblings, 0 replies; 13+ 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] 13+ messages in thread
end of thread, other threads:[~2019-01-17 15:56 UTC | newest]
Thread overview: 13+ 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
-- strict thread matches above, loose matches on Subject: below --
2019-01-16 22:03 Stanislav Fomichev
2019-01-17 15:55 ` 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).