| 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
| ||
|
Date: Mon, 12 Jun 2023 12:17:24 -0700
From: Ian Rogers <irogers@...gle.com>
To: Arnaldo Carvalho de Melo <acme@...nel.org>
Cc: Changbin Du <changbin.du@...wei.com>,
Peter Zijlstra <peterz@...radead.org>,
Ingo Molnar <mingo@...hat.com>,
Mark Rutland <mark.rutland@....com>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
Jiri Olsa <jolsa@...nel.org>,
Namhyung Kim <namhyung@...nel.org>,
Adrian Hunter <adrian.hunter@...el.com>,
Nathan Chancellor <nathan@...nel.org>,
Nick Desaulniers <ndesaulniers@...gle.com>,
Tom Rix <trix@...hat.com>, linux-perf-users@...r.kernel.org,
linux-kernel@...r.kernel.org, llvm@...ts.linux.dev
Subject: Re: [PATCH v1 2/2] perf srcline: Make sentinel reading for binutils
addr2line more robust
On Mon, Jun 12, 2023 at 12:11 PM Arnaldo Carvalho de Melo
<acme@...nel.org> wrote:
>
> Em Fri, Jun 09, 2023 at 04:54:19PM -0700, Ian Rogers escreveu:
> > The addr2line process is sent an address then multiple function,
> > filename:line "records" are read. To detect the end of output a ',' is
> > sent and for llvm-addr2line a ',' is then read back showing the end of
> > addrline's output. For binutils addr2line the ',' translates to
> > address 0 and we expect the bogus filename marker "??:0" (see
> > filename_split) to be sent from addr2line. For some kernels address 0
> > may have a mapping and so a seemingly valid inline output is given and
> > breaking the sentinel discovery:
> >
> > ```
> > $ addr2line -e vmlinux -f -i
> > ,
> > __per_cpu_start
> > ./arch/x86/kernel/cpu/common.c:1850
> > ```
> >
> > To avoid this problem enable the address dumping for addr2line (the -a
> > option). If an address of 0x0000000000000000 is read then this is the
> > sentinel value working around the problem above. The filename_split
> > still needs to check for "??:0" as bogus non-zero addresses also need
> > handling.
> >
> > Reported-by: Changbin Du <changbin.du@...wei.com>
>
> Changbin,
>
> Did this fix the problem you reported? If so can I have your
> Tested-by?
There are ongoing issues that Changbin has found, the thread is here:
https://lore.kernel.org/linux-perf-users/CAP-5=fWdvMVOiZDhqiz1R9hucz+6+ho_L3jBixqP3YLcJD7Lew@mail.gmail.com/
Thanks,
Ian
> Thanks,
>
> - Arnaldo
>
> > Signed-off-by: Ian Rogers <irogers@...gle.com>
> > ---
> > tools/perf/util/srcline.c | 61 ++++++++++++++++++++++++++++++++++-----
> > 1 file changed, 53 insertions(+), 8 deletions(-)
> >
> > diff --git a/tools/perf/util/srcline.c b/tools/perf/util/srcline.c
> > index fc85cdd6c8f9..c99a001453b4 100644
> > --- a/tools/perf/util/srcline.c
> > +++ b/tools/perf/util/srcline.c
> > @@ -406,7 +406,7 @@ static struct child_process *addr2line_subprocess_init(const char *addr2line_pat
> > const char *argv[] = {
> > addr2line_path ?: "addr2line",
> > "-e", binary_path,
> > - "-i", "-f", NULL
> > + "-a", "-i", "-f", NULL
> > };
> > struct child_process *a2l = zalloc(sizeof(*a2l));
> > int start_command_status = 0;
> > @@ -461,10 +461,10 @@ static enum a2l_style addr2line_configure(struct child_process *a2l, const char
> > style = LLVM;
> > cached = true;
> > lines = 1;
> > - } else if (ch == '?') {
> > + } else if (ch == '0') {
> > style = GNU_BINUTILS;
> > cached = true;
> > - lines = 2;
> > + lines = 3;
> > } else {
> > if (!symbol_conf.disable_add2line_warn) {
> > char *output;
> > @@ -516,20 +516,64 @@ static int read_addr2line_record(struct io *io,
> > if (line_nr != NULL)
> > *line_nr = 0;
> >
> > + /*
> > + * Read the first line. Without an error this will be either an address
> > + * like 0x1234 or for llvm-addr2line the sentinal ',' character.
> > + */
> > if (io__getline(io, &line, &line_len) < 0 || !line_len)
> > goto error;
> >
> > - if (style == LLVM && line_len == 2 && line[0] == ',') {
> > - zfree(&line);
> > - return 0;
> > + if (style == LLVM) {
> > + if (line_len == 2 && line[0] == ',') {
> > + zfree(&line);
> > + return 0;
> > + }
> > + } else {
> > + int zero_count = 0, non_zero_count = 0;
> > +
> > + /* The address should always start 0x. */
> > + if (line_len < 2 || line[0] != '0' || line[1] != 'x')
> > + goto error;
> > +
> > + for (size_t i = 2; i < line_len; i++) {
> > + if (line[i] == '0')
> > + zero_count++;
> > + else
> > + non_zero_count++;
> > + }
> > + if (!non_zero_count) {
> > + int ch;
> > +
> > + if (!zero_count) {
> > + /* Line was erroneous just '0x'. */
> > + goto error;
> > + }
> > + /*
> > + * Line was 0x0..0, the sentinel for binutils. Remove
> > + * the function and filename lines.
> > + */
> > + zfree(&line);
> > + do {
> > + ch = io__get_char(io);
> > + } while (ch > 0 && ch != '\n');
> > + do {
> > + ch = io__get_char(io);
> > + } while (ch > 0 && ch != '\n');
> > + return 0;
> > + }
> > }
> >
> > + /* Read the second function name line. */
> > + if (io__getline(io, &line, &line_len) < 0 || !line_len)
> > + goto error;
> > +
> > if (function != NULL)
> > *function = strdup(strim(line));
> >
> > zfree(&line);
> > line_len = 0;
> >
> > + /* Read the third filename and line number line. */
> > if (io__getline(io, &line, &line_len) < 0 || !line_len)
> > goto error;
> >
> > @@ -633,8 +677,9 @@ static int addr2line(const char *dso_name, u64 addr,
> > goto out;
> > case 0:
> > /*
> > - * The first record was invalid, so return failure, but first read another
> > - * record, since we asked a junk question and have to clear the answer out.
> > + * The first record was invalid, so return failure, but first
> > + * read another record, since we sent a sentinel ',' for the
> > + * sake of detected the last inlined function.
> > */
> > switch (read_addr2line_record(&io, a2l_style, NULL, NULL, NULL)) {
> > case -1:
> > --
> > 2.41.0.162.gfafddb0af9-goog
> >
>
> --
>
> - Arnaldo
Powered by blists - more mailing lists