| Message ID | 20190115225447.245788-1-sdf@google.com |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | [bpf,1/3] bpf: don't assume build-id length is always 20 bytes | expand |
> 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 >
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 > > >
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) +
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(-)