| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAP-5=fX_fT7e9tqDBKXh_1CQ8w80iOXCGz2kJT_nHpY6wYWqmQ@mail.gmail.com>
Date: Mon, 11 Jul 2022 09:09:37 -0700
From: Ian Rogers <irogers@...gle.com>
To: Leo Yan <leo.yan@...aro.org>, Fangrui Song <maskray@...gle.com>
Cc: Arnaldo Carvalho de Melo <acme@...nel.org>,
Peter Zijlstra <peterz@...radead.org>,
Namhyung Kim <namhyung@...nel.org>,
Ingo Molnar <mingo@...hat.com>,
Mark Rutland <mark.rutland@....com>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
Jiri Olsa <jolsa@...nel.org>, linux-perf-users@...r.kernel.org,
linux-kernel@...r.kernel.org, Chang Rui <changruinj@...il.com>
Subject: Re: [RFC PATCH v1] perf symbol: Correct address for bss symbols
On Sat, Jul 9, 2022 at 6:22 PM Leo Yan <leo.yan@...aro.org> wrote:
>
> When using 'perf mem' and 'perf c2c', an issue is observed that tool
> reports the wrong offset for global data symbols. This is a common
> issue on both x86 and Arm64 platforms.
>
> Let's see an example, for a test program, below is the disassembly for
> its .bss section which is dumped with objdump:
>
> ...
>
> Disassembly of section .data:
>
> 0000000000004000 <__data_start>:
> ...
>
> 0000000000004008 <__dso_handle>:
> 4008: 08 40 00 or %al,0x0(%rax)
> 400b: 00 00 add %al,(%rax)
> 400d: 00 00 add %al,(%rax)
> ...
>
> 0000000000004010 <wait_to_begin>:
> 4010: 01 00 add %eax,(%rax)
> 4012: 00 00 add %al,(%rax)
> 4014: 00 00 add %al,(%rax)
> ...
>
> 0000000000004018 <lock_thd_name>:
> 4018: 08 20 or %ah,(%rax)
> 401a: 00 00 add %al,(%rax)
> 401c: 00 00 add %al,(%rax)
> ...
>
> 0000000000004020 <reader_thd_name>:
> 4020: 10 20 adc %ah,(%rax)
> 4022: 00 00 add %al,(%rax)
> 4024: 00 00 add %al,(%rax)
> ...
>
> Disassembly of section .bss:
>
> 0000000000004040 <completed.0>:
> ...
>
> 0000000000004080 <buf1>:
> ...
>
> 00000000000040c0 <buf2>:
> ...
>
> 0000000000004100 <thread>:
> ...
>
> First we used 'perf mem record' to run the test program and then used
> 'perf --debug verbose=4 mem report' to observe what's the symbol info
> for 'buf1' and 'buf2' structures.
>
> # ./perf mem record -e ldlat-loads,ldlat-stores -- false_sharing.exe 8
> # ./perf --debug verbose=4 mem report
> ...
> dso__load_sym_internal: adjusting symbol: st_value: 0x40c0 sh_addr: 0x4040 sh_offset: 0x3028
> symbol__new: buf2 0x30a8-0x30e8
> ...
> dso__load_sym_internal: adjusting symbol: st_value: 0x4080 sh_addr: 0x4040 sh_offset: 0x3028
> symbol__new: buf1 0x3068-0x30a8
> ...
>
> Perf tool relies on libelf to parse symbols, here 'st_value' is the
> address from executable file, 'sh_addr' is the belonged section's linked
> start address, and 'sh_offset' is the dynamic loaded address for this
> section, then perf tool uses below formula to adjust symbol address:
>
> adjusted_address = st_value - sh_addr + sh_offset
>
> So we can see the final adjusted address ranges for buf1 and buf2 are
> [0x30a8-0x30e8) and [0x3068-0x30a8) respectively, apparently this is
> incorrect, in the code, the structure for 'buf1' and 'buf2' specifies
> compiler attribute with 64-byte alignment.
>
> The problem happens for 'sh_offset', libelf returns it as 0x3028 which
> is not 64-byte aligned, on the other hand, we can see both 'st_value'
> and 'sh_addr' are 64-byte aligned. Combining with disassembly, it's
> likely libelf uses the .data section end address as .bss section
> start address, therefore, it doesn't respect the alignment attribute for
> structures in .bss section.
>
> Since .data and .bss sections are in the continuous virtual address
> space, and .data section info returned by libelf is reliable, to fix
> this issue, if detects it's a bss symbol, it rolls back to use .data
> section info to adjust symbol's virtual address.
>
> Essentially, we need to fix libelf to return correct offsets for
> sections, on the other hand, we live commonly with existed versions of
> libelf. So we also need this change in perf tool.
>
> Fixes: f17e04afaff8 ("perf report: Fix ELF symbol parsing")
> Reported-by: Chang Rui <changruinj@...il.com>
> Signed-off-by: Leo Yan <leo.yan@...aro.org>
This looks good to me, I'm happy to add my:
Acked-by: Ian Rogers <irogers@...gle.com>
I've added Fangrui Song who is more knowledge-able on ELF, libelf,
etc. than me and may have additional thoughts.
Thanks,
Ian
> ---
> tools/perf/util/dso.h | 1 +
> tools/perf/util/symbol-elf.c | 26 ++++++++++++++++++++++++++
> 2 files changed, 27 insertions(+)
>
> diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h
> index 3a9fd4d389b5..00f57f4ac6bc 100644
> --- a/tools/perf/util/dso.h
> +++ b/tools/perf/util/dso.h
> @@ -180,6 +180,7 @@ struct dso {
> u8 rel;
> struct build_id bid;
> u64 text_offset;
> + int data_sec_index;
> const char *short_name;
> const char *long_name;
> u16 long_name_len;
> diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
> index ecd377938eea..ed65dd26d58e 100644
> --- a/tools/perf/util/symbol-elf.c
> +++ b/tools/perf/util/symbol-elf.c
> @@ -1095,6 +1095,7 @@ dso__load_sym_internal(struct dso *dso, struct map *map, struct symsrc *syms_ss,
> Elf *elf;
> int nr = 0;
> bool remap_kernel = false, adjust_kernel_syms = false;
> + size_t sec_index;
>
> if (kmap && !kmaps)
> return -1;
> @@ -1113,6 +1114,10 @@ dso__load_sym_internal(struct dso *dso, struct map *map, struct symsrc *syms_ss,
> ".text", NULL))
> dso->text_offset = tshdr.sh_addr - tshdr.sh_offset;
>
> + if (elf_section_by_name(runtime_ss->elf, &runtime_ss->ehdr, &tshdr,
> + ".data", &sec_index))
> + dso->data_sec_index = sec_index;
> +
> if (runtime_ss->opdsec)
> opddata = elf_rawdata(runtime_ss->opdsec, NULL);
>
> @@ -1227,6 +1232,27 @@ dso__load_sym_internal(struct dso *dso, struct map *map, struct symsrc *syms_ss,
>
> gelf_getshdr(sec, &shdr);
>
> + /*
> + * When the first data structure in .bss section is attributed
> + * with alignment (e.g. 64-byte aligned), libelf doesn't reflect
> + * the alignment in the 'shdr.sh_offset' field, at the end the
> + * field is filled with the end loading address of a prior
> + * section rather than the aligned address of .bss section.
> + * This leads to mess for later parsing .bss symbols.
> + *
> + * Since .data and .bss sections are in the continuous virtual
> + * address space, and .data section's info is reliable. So if
> + * detects it's a bss symbol, we retrieve .data section info
> + * for adjusting address.
> + */
> + if (!strcmp(elf_sec__name(&shdr, secstrs_sym), ".bss")) {
> + sec = elf_getscn(syms_ss->elf, dso->data_sec_index);
> + if (!sec)
> + goto out_elf_end;
> +
> + gelf_getshdr(sec, &shdr);
> + }
> +
> secstrs = secstrs_sym;
>
> /*
> --
> 2.25.1
>
Powered by blists - more mailing lists