* [PATCH v1] perf symbol: Avoid use after free
@ 2023-03-28 23:44 Ian Rogers
2023-03-29 4:52 ` Adrian Hunter
0 siblings, 1 reply; 4+ messages in thread
From: Ian Rogers @ 2023-03-28 23:44 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
Ian Rogers, Adrian Hunter, Leo Yan, linux-perf-users,
linux-kernel
If demangling succeeds then sym_name is set to the demangled string
that is freed. Rather than test if sym_name is empty and possibly
use-after-free on the return path, expand out the alternatives.
Signed-off-by: Ian Rogers <irogers@google.com>
---
tools/perf/util/symbol-elf.c | 18 ++++++++++--------
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index c0a2de42c51b..b7e3e492bff3 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -577,15 +577,17 @@ static bool get_plt_got_name(GElf_Shdr *shdr, size_t i,
/* Get the associated symbol */
gelf_getsym(di->dynsym_data, vr->sym_idx, &sym);
sym_name = elf_sym__name(&sym, di->dynstr_data);
- demangled = demangle_sym(di->dso, 0, sym_name);
- if (demangled != NULL)
- sym_name = demangled;
-
- snprintf(buf, buf_sz, "%s@plt", sym_name);
-
- free(demangled);
+ if (*sym_name == '\0')
+ return false;
- return *sym_name;
+ demangled = demangle_sym(di->dso, 0, sym_name);
+ if (demangled != NULL) {
+ snprintf(buf, buf_sz, "%s@plt", demangled);
+ free(demangled);
+ } else {
+ snprintf(buf, buf_sz, "%s@plt", sym_name);
+ }
+ return true;
}
static int dso__synthesize_plt_got_symbols(struct dso *dso, Elf *elf,
--
2.40.0.348.gf938b09366-goog
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v1] perf symbol: Avoid use after free
2023-03-28 23:44 [PATCH v1] perf symbol: Avoid use after free Ian Rogers
@ 2023-03-29 4:52 ` Adrian Hunter
2023-03-29 5:35 ` Ian Rogers
0 siblings, 1 reply; 4+ messages in thread
From: Adrian Hunter @ 2023-03-29 4:52 UTC (permalink / raw)
To: Ian Rogers, Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
Leo Yan, linux-perf-users, linux-kernel
On 29/03/23 02:44, Ian Rogers wrote:
> If demangling succeeds then sym_name is set to the demangled string
> that is freed. Rather than test if sym_name is empty and possibly
> use-after-free on the return path, expand out the alternatives.
Looks the same as:
https://lore.kernel.org/linux-perf-users/20230316194156.8320-2-adrian.hunter@intel.com/
>
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
> tools/perf/util/symbol-elf.c | 18 ++++++++++--------
> 1 file changed, 10 insertions(+), 8 deletions(-)
>
> diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
> index c0a2de42c51b..b7e3e492bff3 100644
> --- a/tools/perf/util/symbol-elf.c
> +++ b/tools/perf/util/symbol-elf.c
> @@ -577,15 +577,17 @@ static bool get_plt_got_name(GElf_Shdr *shdr, size_t i,
> /* Get the associated symbol */
> gelf_getsym(di->dynsym_data, vr->sym_idx, &sym);
> sym_name = elf_sym__name(&sym, di->dynstr_data);
> - demangled = demangle_sym(di->dso, 0, sym_name);
> - if (demangled != NULL)
> - sym_name = demangled;
> -
> - snprintf(buf, buf_sz, "%s@plt", sym_name);
> -
> - free(demangled);
> + if (*sym_name == '\0')
> + return false;
>
> - return *sym_name;
> + demangled = demangle_sym(di->dso, 0, sym_name);
> + if (demangled != NULL) {
> + snprintf(buf, buf_sz, "%s@plt", demangled);
> + free(demangled);
> + } else {
> + snprintf(buf, buf_sz, "%s@plt", sym_name);
> + }
> + return true;
> }
>
> static int dso__synthesize_plt_got_symbols(struct dso *dso, Elf *elf,
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v1] perf symbol: Avoid use after free
2023-03-29 4:52 ` Adrian Hunter
@ 2023-03-29 5:35 ` Ian Rogers
2023-03-29 12:53 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 4+ messages in thread
From: Ian Rogers @ 2023-03-29 5:35 UTC (permalink / raw)
To: Adrian Hunter
Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
Leo Yan, linux-perf-users, linux-kernel
On Tue, Mar 28, 2023 at 9:52 PM Adrian Hunter <adrian.hunter@intel.com> wrote:
>
> On 29/03/23 02:44, Ian Rogers wrote:
> > If demangling succeeds then sym_name is set to the demangled string
> > that is freed. Rather than test if sym_name is empty and possibly
> > use-after-free on the return path, expand out the alternatives.
>
> Looks the same as:
>
> https://lore.kernel.org/linux-perf-users/20230316194156.8320-2-adrian.hunter@intel.com/
Sorry for missing that. In the case it returns false, this avoids an
unnecessary snprintf.
Thanks,
Ian
> >
> > Signed-off-by: Ian Rogers <irogers@google.com>
> > ---
> > tools/perf/util/symbol-elf.c | 18 ++++++++++--------
> > 1 file changed, 10 insertions(+), 8 deletions(-)
> >
> > diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
> > index c0a2de42c51b..b7e3e492bff3 100644
> > --- a/tools/perf/util/symbol-elf.c
> > +++ b/tools/perf/util/symbol-elf.c
> > @@ -577,15 +577,17 @@ static bool get_plt_got_name(GElf_Shdr *shdr, size_t i,
> > /* Get the associated symbol */
> > gelf_getsym(di->dynsym_data, vr->sym_idx, &sym);
> > sym_name = elf_sym__name(&sym, di->dynstr_data);
> > - demangled = demangle_sym(di->dso, 0, sym_name);
> > - if (demangled != NULL)
> > - sym_name = demangled;
> > -
> > - snprintf(buf, buf_sz, "%s@plt", sym_name);
> > -
> > - free(demangled);
> > + if (*sym_name == '\0')
> > + return false;
> >
> > - return *sym_name;
> > + demangled = demangle_sym(di->dso, 0, sym_name);
> > + if (demangled != NULL) {
> > + snprintf(buf, buf_sz, "%s@plt", demangled);
> > + free(demangled);
> > + } else {
> > + snprintf(buf, buf_sz, "%s@plt", sym_name);
> > + }
> > + return true;
> > }
> >
> > static int dso__synthesize_plt_got_symbols(struct dso *dso, Elf *elf,
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v1] perf symbol: Avoid use after free
2023-03-29 5:35 ` Ian Rogers
@ 2023-03-29 12:53 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 4+ messages in thread
From: Arnaldo Carvalho de Melo @ 2023-03-29 12:53 UTC (permalink / raw)
To: Ian Rogers
Cc: Adrian Hunter, Peter Zijlstra, Ingo Molnar, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Namhyung Kim, Leo Yan,
linux-perf-users, linux-kernel
Em Tue, Mar 28, 2023 at 10:35:07PM -0700, Ian Rogers escreveu:
> On Tue, Mar 28, 2023 at 9:52 PM Adrian Hunter <adrian.hunter@intel.com> wrote:
> >
> > On 29/03/23 02:44, Ian Rogers wrote:
> > > If demangling succeeds then sym_name is set to the demangled string
> > > that is freed. Rather than test if sym_name is empty and possibly
> > > use-after-free on the return path, expand out the alternatives.
> >
> > Looks the same as:
> >
> > https://lore.kernel.org/linux-perf-users/20230316194156.8320-2-adrian.hunter@intel.com/
>
> Sorry for missing that. In the case it returns false, this avoids an
> unnecessary snprintf.
I applied Adrian's fixes,
- Arnaldo
> Thanks,
> Ian
>
> > >
> > > Signed-off-by: Ian Rogers <irogers@google.com>
> > > ---
> > > tools/perf/util/symbol-elf.c | 18 ++++++++++--------
> > > 1 file changed, 10 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
> > > index c0a2de42c51b..b7e3e492bff3 100644
> > > --- a/tools/perf/util/symbol-elf.c
> > > +++ b/tools/perf/util/symbol-elf.c
> > > @@ -577,15 +577,17 @@ static bool get_plt_got_name(GElf_Shdr *shdr, size_t i,
> > > /* Get the associated symbol */
> > > gelf_getsym(di->dynsym_data, vr->sym_idx, &sym);
> > > sym_name = elf_sym__name(&sym, di->dynstr_data);
> > > - demangled = demangle_sym(di->dso, 0, sym_name);
> > > - if (demangled != NULL)
> > > - sym_name = demangled;
> > > -
> > > - snprintf(buf, buf_sz, "%s@plt", sym_name);
> > > -
> > > - free(demangled);
> > > + if (*sym_name == '\0')
> > > + return false;
> > >
> > > - return *sym_name;
> > > + demangled = demangle_sym(di->dso, 0, sym_name);
> > > + if (demangled != NULL) {
> > > + snprintf(buf, buf_sz, "%s@plt", demangled);
> > > + free(demangled);
> > > + } else {
> > > + snprintf(buf, buf_sz, "%s@plt", sym_name);
> > > + }
> > > + return true;
> > > }
> > >
> > > static int dso__synthesize_plt_got_symbols(struct dso *dso, Elf *elf,
> >
--
- Arnaldo
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-03-29 12:53 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-03-28 23:44 [PATCH v1] perf symbol: Avoid use after free Ian Rogers
2023-03-29 4:52 ` Adrian Hunter
2023-03-29 5:35 ` Ian Rogers
2023-03-29 12:53 ` Arnaldo Carvalho de Melo
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.