| Message ID | 20200812123102.20032-1-jolsa@kernel.org |
|---|---|
| State | Accepted |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | [RFC,bpf-next] bpf: Iterate through all PT_NOTE sections when looking for build id | expand |
> On Aug 12, 2020, at 5:31 AM, Jiri Olsa <jolsa@kernel.org> wrote: > > Currently when we look for build id within bpf_get_stackid helper > call, we check the first NOTE section and we fail if build id is > not there. > > However on some system (Fedora) there can be multiple NOTE sections > in binaries and build id data is not always the first one, like: > > $ readelf -a /usr/bin/ls > ... > [ 2] .note.gnu.propert NOTE 0000000000000338 00000338 > 0000000000000020 0000000000000000 A 0 0 8358 > [ 3] .note.gnu.build-i NOTE 0000000000000358 00000358 > 0000000000000024 0000000000000000 A 0 0 437c > [ 4] .note.ABI-tag NOTE 000000000000037c 0000037c > ... > > So the stack_map_get_build_id function will fail on build id retrieval > and fallback to BPF_STACK_BUILD_ID_IP. > > This patch is changing the stack_map_get_build_id code to iterate > through all the NOTE sections and try to get build id data from > each of them. > > When tracing on sched_switch tracepoint that does bpf_get_stackid > helper call kernel build, I can see about 60% increase of successful > build id retrieval. The rest seems fails on -EFAULT. > > Signed-off-by: Jiri Olsa <jolsa@kernel.org> LGTM. Thanks for the fix! Acked-by: Song Liu <songliubraving@fb.com>
On Wed, Aug 12, 2020 at 8:27 AM Song Liu <songliubraving@fb.com> wrote: > > > > > On Aug 12, 2020, at 5:31 AM, Jiri Olsa <jolsa@kernel.org> wrote: > > > > Currently when we look for build id within bpf_get_stackid helper > > call, we check the first NOTE section and we fail if build id is > > not there. > > > > However on some system (Fedora) there can be multiple NOTE sections > > in binaries and build id data is not always the first one, like: > > > > $ readelf -a /usr/bin/ls > > ... > > [ 2] .note.gnu.propert NOTE 0000000000000338 00000338 > > 0000000000000020 0000000000000000 A 0 0 8358 > > [ 3] .note.gnu.build-i NOTE 0000000000000358 00000358 > > 0000000000000024 0000000000000000 A 0 0 437c > > [ 4] .note.ABI-tag NOTE 000000000000037c 0000037c > > ... > > > > So the stack_map_get_build_id function will fail on build id retrieval > > and fallback to BPF_STACK_BUILD_ID_IP. > > > > This patch is changing the stack_map_get_build_id code to iterate > > through all the NOTE sections and try to get build id data from > > each of them. > > > > When tracing on sched_switch tracepoint that does bpf_get_stackid > > helper call kernel build, I can see about 60% increase of successful > > build id retrieval. The rest seems fails on -EFAULT. > > > > Signed-off-by: Jiri Olsa <jolsa@kernel.org> > > LGTM. Thanks for the fix! > > Acked-by: Song Liu <songliubraving@fb.com> It's a good fix. Applied to bpf tree. Thanks
diff --git a/kernel/bpf/stackmap.c b/kernel/bpf/stackmap.c index 4fd830a62be2..cfed0ac44d38 100644 --- a/kernel/bpf/stackmap.c +++ b/kernel/bpf/stackmap.c @@ -213,11 +213,13 @@ static int stack_map_get_build_id_32(void *page_addr, phdr = (Elf32_Phdr *)(page_addr + sizeof(Elf32_Ehdr)); - for (i = 0; i < ehdr->e_phnum; ++i) - if (phdr[i].p_type == PT_NOTE) - return stack_map_parse_build_id(page_addr, build_id, - page_addr + phdr[i].p_offset, - phdr[i].p_filesz); + for (i = 0; i < ehdr->e_phnum; ++i) { + if (phdr[i].p_type == PT_NOTE && + !stack_map_parse_build_id(page_addr, build_id, + page_addr + phdr[i].p_offset, + phdr[i].p_filesz)) + return 0; + } return -EINVAL; } @@ -236,11 +238,13 @@ static int stack_map_get_build_id_64(void *page_addr, phdr = (Elf64_Phdr *)(page_addr + sizeof(Elf64_Ehdr)); - for (i = 0; i < ehdr->e_phnum; ++i) - if (phdr[i].p_type == PT_NOTE) - return stack_map_parse_build_id(page_addr, build_id, - page_addr + phdr[i].p_offset, - phdr[i].p_filesz); + for (i = 0; i < ehdr->e_phnum; ++i) { + if (phdr[i].p_type == PT_NOTE && + !stack_map_parse_build_id(page_addr, build_id, + page_addr + phdr[i].p_offset, + phdr[i].p_filesz)) + return 0; + } return -EINVAL; }
Currently when we look for build id within bpf_get_stackid helper call, we check the first NOTE section and we fail if build id is not there. However on some system (Fedora) there can be multiple NOTE sections in binaries and build id data is not always the first one, like: $ readelf -a /usr/bin/ls ... [ 2] .note.gnu.propert NOTE 0000000000000338 00000338 0000000000000020 0000000000000000 A 0 0 8358 [ 3] .note.gnu.build-i NOTE 0000000000000358 00000358 0000000000000024 0000000000000000 A 0 0 437c [ 4] .note.ABI-tag NOTE 000000000000037c 0000037c ... So the stack_map_get_build_id function will fail on build id retrieval and fallback to BPF_STACK_BUILD_ID_IP. This patch is changing the stack_map_get_build_id code to iterate through all the NOTE sections and try to get build id data from each of them. When tracing on sched_switch tracepoint that does bpf_get_stackid helper call kernel build, I can see about 60% increase of successful build id retrieval. The rest seems fails on -EFAULT. Signed-off-by: Jiri Olsa <jolsa@kernel.org> --- kernel/bpf/stackmap.c | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-)