| 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: <ZD26ZlqSbgSyH5lX@kernel.org>
Date: Mon, 17 Apr 2023 18:30:14 -0300
From: Arnaldo Carvalho de Melo <acme@...nel.org>
To: Ian Rogers <irogers@...gle.com>
Cc: 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>,
Thomas Gleixner <tglx@...utronix.de>,
Darren Hart <dvhart@...radead.org>,
Davidlohr Bueso <dave@...olabs.net>,
James Clark <james.clark@....com>,
John Garry <john.g.garry@...cle.com>,
Riccardo Mancini <rickyman7@...il.com>,
Yury Norov <yury.norov@...il.com>,
Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
Andrew Morton <akpm@...ux-foundation.org>,
Adrian Hunter <adrian.hunter@...el.com>,
Leo Yan <leo.yan@...aro.org>, Andi Kleen <ak@...ux.intel.com>,
Thomas Richter <tmricht@...ux.ibm.com>,
Kan Liang <kan.liang@...ux.intel.com>,
Madhavan Srinivasan <maddy@...ux.ibm.com>,
Shunsuke Nakamura <nakamura.shun@...itsu.com>,
Song Liu <song@...nel.org>,
Masami Hiramatsu <mhiramat@...nel.org>,
Steven Rostedt <rostedt@...dmis.org>,
Miaoqian Lin <linmq006@...il.com>,
Stephen Brennan <stephen.s.brennan@...cle.com>,
Kajol Jain <kjain@...ux.ibm.com>,
Alexey Bayduraev <alexey.v.bayduraev@...ux.intel.com>,
German Gomez <german.gomez@....com>,
linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org,
Eric Dumazet <edumazet@...gle.com>,
Dmitry Vyukov <dvyukov@...gle.com>,
Hao Luo <haoluo@...gle.com>,
Stephane Eranian <eranian@...gle.com>
Subject: Re: [PATCH v7 3/5] perf namespaces: Add reference count checking
Em Fri, Apr 07, 2023 at 04:04:03PM -0700, Ian Rogers escreveu:
> Add reference count checking controlled by REFCNT_CHECKING ifdef. The
> reference count checking interposes an allocated pointer between the
> reference counted struct on a get and frees the pointer on a put.
> Accesses after a put cause faults and use after free, missed puts are
> caughts as leaks and double puts are double frees.
>
> This checking helped resolve a memory leak and use after free:
> https://lore.kernel.org/linux-perf-users/CAP-5=fWZH20L4kv-BwVtGLwR=Em3AOOT+Q4QGivvQuYn5AsPRg@mail.gmail.com/
>
> Signed-off-by: Ian Rogers <irogers@...gle.com>
> ---
> tools/perf/builtin-inject.c | 2 +-
> tools/perf/util/annotate.c | 2 +-
> tools/perf/util/dso.c | 2 +-
> tools/perf/util/dsos.c | 2 +-
> tools/perf/util/namespaces.c | 132 ++++++++++++++++++++---------------
> tools/perf/util/namespaces.h | 3 +-
> tools/perf/util/symbol.c | 2 +-
> 7 files changed, 83 insertions(+), 62 deletions(-)
>
> diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
> index fd2b38458a5d..fe6ddcf7fb1e 100644
> --- a/tools/perf/builtin-inject.c
> +++ b/tools/perf/builtin-inject.c
> @@ -632,7 +632,7 @@ static int dso__read_build_id(struct dso *dso)
> else if (dso->nsinfo) {
> char *new_name;
>
> - new_name = filename_with_chroot(dso->nsinfo->pid,
> + new_name = filename_with_chroot(RC_CHK_ACCESS(dso->nsinfo)->pid,
> dso->long_name);
To reduce these:
>From c80478cfc86cf24f6f075576d731d99e0fa8fe4f Mon Sep 17 00:00:00 2001
From: Arnaldo Carvalho de Melo <acme@...hat.com>
Date: Mon, 17 Apr 2023 18:28:01 -0300
Subject: [PATCH 1/1] perf dso: Add dso__filename_with_chroot() to reduce
number of accesses to dso->nsinfo members
We'll need to reference count dso->nsinfo, so reduce the number of
direct accesses by having a shorter form of obtaining a filename with
a chroot (namespace one).
Cc: Adrian Hunter <adrian.hunter@...el.com>
Cc: Alexey Bayduraev <alexey.v.bayduraev@...ux.intel.com>
Cc: Dmitriy Vyukov <dvyukov@...gle.com>
Cc: Ian Rogers <irogers@...gle.com>
Cc: Jiri Olsa <jolsa@...nel.org>
Cc: Namhyung Kim <namhyung@...nel.org>
Cc: Riccardo Mancini <rickyman7@...il.com>
Cc: Stephane Eranian <eranian@...gle.com>
Cc: Stephen Brennan <stephen.s.brennan@...cle.com>
Link: https://lore.kernel.org/lkml/
Signed-off-by: Arnaldo Carvalho de Melo <acme@...hat.com>
---
tools/perf/builtin-inject.c | 4 +---
tools/perf/util/annotate.c | 3 +--
tools/perf/util/dso.c | 7 ++++++-
tools/perf/util/dso.h | 2 ++
tools/perf/util/dsos.c | 3 +--
tools/perf/util/symbol.c | 3 +--
6 files changed, 12 insertions(+), 10 deletions(-)
diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
index 76723ac314b60b80..61766eead4f48e34 100644
--- a/tools/perf/builtin-inject.c
+++ b/tools/perf/builtin-inject.c
@@ -630,10 +630,8 @@ static int dso__read_build_id(struct dso *dso)
if (filename__read_build_id(dso->long_name, &dso->bid) > 0)
dso->has_build_id = true;
else if (dso->nsinfo) {
- char *new_name;
+ char *new_name = dso__filename_with_chroot(dso, dso->long_name);
- new_name = filename_with_chroot(dso->nsinfo->pid,
- dso->long_name);
if (new_name && filename__read_build_id(new_name, &dso->bid) > 0)
dso->has_build_id = true;
free(new_name);
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index e2693a1c28d5989f..ca9f0add68f461e4 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -1692,8 +1692,7 @@ static int dso__disassemble_filename(struct dso *dso, char *filename, size_t fil
mutex_lock(&dso->lock);
if (access(filename, R_OK) && errno == ENOENT && dso->nsinfo) {
- char *new_name = filename_with_chroot(dso->nsinfo->pid,
- filename);
+ char *new_name = dso__filename_with_chroot(dso, filename);
if (new_name) {
strlcpy(filename, new_name, filename_size);
free(new_name);
diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index e36b418df2c68cc2..0bc288f29a548dc9 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -491,6 +491,11 @@ static int do_open(char *name)
return -1;
}
+char *dso__filename_with_chroot(const struct dso *dso, const char *filename)
+{
+ return filename_with_chroot(dso->nsinfo->pid, filename);
+}
+
static int __open_dso(struct dso *dso, struct machine *machine)
{
int fd = -EINVAL;
@@ -515,7 +520,7 @@ static int __open_dso(struct dso *dso, struct machine *machine)
if (errno != ENOENT || dso->nsinfo == NULL)
goto out;
- new_name = filename_with_chroot(dso->nsinfo->pid, name);
+ new_name = dso__filename_with_chroot(dso, name);
if (!new_name)
goto out;
diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h
index 58d94175e7148049..0b7c7633b9f6667d 100644
--- a/tools/perf/util/dso.h
+++ b/tools/perf/util/dso.h
@@ -266,6 +266,8 @@ static inline bool dso__has_symbols(const struct dso *dso)
return !RB_EMPTY_ROOT(&dso->symbols.rb_root);
}
+char *dso__filename_with_chroot(const struct dso *dso, const char *filename);
+
bool dso__sorted_by_name(const struct dso *dso);
void dso__set_sorted_by_name(struct dso *dso);
void dso__sort_by_name(struct dso *dso);
diff --git a/tools/perf/util/dsos.c b/tools/perf/util/dsos.c
index 2bd23e4cf19efaa7..cf80aa42dd07b036 100644
--- a/tools/perf/util/dsos.c
+++ b/tools/perf/util/dsos.c
@@ -91,8 +91,7 @@ bool __dsos__read_build_ids(struct list_head *head, bool with_hits)
have_build_id = true;
pos->has_build_id = true;
} else if (errno == ENOENT && pos->nsinfo) {
- char *new_name = filename_with_chroot(pos->nsinfo->pid,
- pos->long_name);
+ char *new_name = dso__filename_with_chroot(pos, pos->long_name);
if (new_name && filename__read_build_id(new_name,
&pos->bid) > 0) {
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 91ebf93e0c20bd24..e7f63670688e8e59 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -1963,8 +1963,7 @@ int dso__load(struct dso *dso, struct map *map)
is_reg = is_regular_file(name);
if (!is_reg && errno == ENOENT && dso->nsinfo) {
- char *new_name = filename_with_chroot(dso->nsinfo->pid,
- name);
+ char *new_name = dso__filename_with_chroot(dso, name);
if (new_name) {
is_reg = is_regular_file(new_name);
strlcpy(name, new_name, PATH_MAX);
--
2.39.2
Powered by blists - more mailing lists