From: Ian Rogers <irogers@google.com>
To: Peter Zijlstra <peterz@infradead.org>,
Ingo Molnar <mingo@redhat.com>,
Arnaldo Carvalho de Melo <acme@kernel.org>,
Mark Rutland <mark.rutland@arm.com>,
Alexander Shishkin <alexander.shishkin@linux.intel.com>,
Jiri Olsa <jolsa@kernel.org>, Namhyung Kim <namhyung@kernel.org>,
Ian Rogers <irogers@google.com>,
Adrian Hunter <adrian.hunter@intel.com>,
Nick Terrell <terrelln@fb.com>,
Kan Liang <kan.liang@linux.intel.com>,
Andi Kleen <ak@linux.intel.com>, Kajol Jain <kjain@linux.ibm.com>,
Athira Rajeev <atrajeev@linux.vnet.ibm.com>,
Huacai Chen <chenhuacai@kernel.org>,
Masami Hiramatsu <mhiramat@kernel.org>,
Vincent Whitchurch <vincent.whitchurch@axis.com>,
"Steinar H. Gunderson" <sesse@google.com>,
Liam Howlett <liam.howlett@oracle.com>,
Miguel Ojeda <ojeda@kernel.org>,
Colin Ian King <colin.i.king@gmail.com>,
Dmitrii Dolgov <9erthalion6@gmail.com>,
Yang Jihong <yangjihong1@huawei.com>,
Ming Wang <wangming01@loongson.cn>,
James Clark <james.clark@arm.com>,
K Prateek Nayak <kprateek.nayak@amd.com>,
Sean Christopherson <seanjc@google.com>,
Leo Yan <leo.yan@linaro.org>,
Ravi Bangoria <ravi.bangoria@amd.com>,
German Gomez <german.gomez@arm.com>,
Changbin Du <changbin.du@huawei.com>,
Paolo Bonzini <pbonzini@redhat.com>, Li Dong <lidong@vivo.com>,
Sandipan Das <sandipan.das@amd.com>,
liuwenyu <liuwenyu7@huawei.com>,
linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org,
Guilherme Amadio <amadio@gentoo.org>
Subject: [PATCH v6 25/47] perf maps: Get map before returning in maps__find_by_name
Date: Wed, 6 Dec 2023 17:16:59 -0800 [thread overview]
Message-ID: <20231207011722.1220634-26-irogers@google.com> (raw)
In-Reply-To: <20231207011722.1220634-1-irogers@google.com>
Finding a map is done under a lock, returning the map without a
reference count means it can be removed without notice and causing
uses after free. Grab a reference count to the map within the lock
region and return this. Fix up locations that need a map__put
following this. Also fix some reference counted pointer comparisons.
Signed-off-by: Ian Rogers <irogers@google.com>
---
tools/perf/tests/vmlinux-kallsyms.c | 5 +++--
tools/perf/util/machine.c | 6 ++++--
tools/perf/util/maps.c | 6 +++---
tools/perf/util/probe-event.c | 1 +
tools/perf/util/symbol-elf.c | 4 +++-
tools/perf/util/symbol.c | 18 +++++++++++-------
6 files changed, 25 insertions(+), 15 deletions(-)
diff --git a/tools/perf/tests/vmlinux-kallsyms.c b/tools/perf/tests/vmlinux-kallsyms.c
index e808e6fc8f76..fecbf851bb2e 100644
--- a/tools/perf/tests/vmlinux-kallsyms.c
+++ b/tools/perf/tests/vmlinux-kallsyms.c
@@ -131,9 +131,10 @@ static int test__vmlinux_matches_kallsyms_cb1(struct map *map, void *data)
struct map *pair = maps__find_by_name(args->kallsyms.kmaps,
(dso->kernel ? dso->short_name : dso->name));
- if (pair)
+ if (pair) {
map__set_priv(pair, 1);
- else {
+ map__put(pair);
+ } else {
if (!args->header_printed) {
pr_info("WARN: Maps only in vmlinux:\n");
args->header_printed = true;
diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index e8eb9f0b073f..7031f6fddcae 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -1537,8 +1537,10 @@ static int maps__set_module_path(struct maps *maps, const char *path, struct kmo
return 0;
long_name = strdup(path);
- if (long_name == NULL)
+ if (long_name == NULL) {
+ map__put(map);
return -ENOMEM;
+ }
dso = map__dso(map);
dso__set_long_name(dso, long_name, true);
@@ -1552,7 +1554,7 @@ static int maps__set_module_path(struct maps *maps, const char *path, struct kmo
dso->symtab_type++;
dso->comp = m->comp;
}
-
+ map__put(map);
return 0;
}
diff --git a/tools/perf/util/maps.c b/tools/perf/util/maps.c
index 17aa894721a7..b85147cc8723 100644
--- a/tools/perf/util/maps.c
+++ b/tools/perf/util/maps.c
@@ -886,7 +886,7 @@ struct map *maps__find_by_name(struct maps *maps, const char *name)
struct dso *dso = map__dso(maps__maps_by_name(maps)[i]);
if (dso && strcmp(dso->short_name, name) == 0) {
- result = maps__maps_by_name(maps)[i]; // TODO: map__get
+ result = map__get(maps__maps_by_name(maps)[i]);
done = true;
}
}
@@ -898,7 +898,7 @@ struct map *maps__find_by_name(struct maps *maps, const char *name)
sizeof(*mapp), map__strcmp_name);
if (mapp) {
- result = *mapp; // TODO: map__get
+ result = map__get(*mapp);
i = mapp - maps__maps_by_name(maps);
RC_CHK_ACCESS(maps)->last_search_by_name_idx = i;
}
@@ -923,7 +923,7 @@ struct map *maps__find_by_name(struct maps *maps, const char *name)
struct dso *dso = map__dso(pos);
if (dso && strcmp(dso->short_name, name) == 0) {
- result = pos; // TODO: map__get
+ result = map__get(pos);
break;
}
}
diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index a1a796043691..be71abe8b9b0 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -358,6 +358,7 @@ static int kernel_get_module_dso(const char *module, struct dso **pdso)
map = maps__find_by_name(machine__kernel_maps(host_machine), module_name);
if (map) {
dso = map__dso(map);
+ map__put(map);
goto found;
}
pr_debug("Failed to find module %s.\n", module);
diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index 4b934ed3bfd1..5990e3fabdb5 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -1470,8 +1470,10 @@ static int dso__process_kernel_symbol(struct dso *dso, struct map *map,
dso__set_loaded(curr_dso);
*curr_mapp = curr_map;
*curr_dsop = curr_dso;
- } else
+ } else {
*curr_dsop = map__dso(curr_map);
+ map__put(curr_map);
+ }
return 0;
}
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 1710b89e207c..0785a54e832e 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -814,7 +814,7 @@ static int maps__split_kallsyms(struct maps *kmaps, struct dso *dso, u64 delta,
struct map *initial_map)
{
struct machine *machine;
- struct map *curr_map = initial_map;
+ struct map *curr_map = map__get(initial_map);
struct symbol *pos;
int count = 0, moved = 0;
struct rb_root_cached *root = &dso->symbols;
@@ -858,13 +858,14 @@ static int maps__split_kallsyms(struct maps *kmaps, struct dso *dso, u64 delta,
dso__set_loaded(curr_map_dso);
}
+ map__zput(curr_map);
curr_map = maps__find_by_name(kmaps, module);
if (curr_map == NULL) {
pr_debug("%s/proc/{kallsyms,modules} "
"inconsistency while looking "
"for \"%s\" module!\n",
machine->root_dir, module);
- curr_map = initial_map;
+ curr_map = map__get(initial_map);
goto discard_symbol;
}
curr_map_dso = map__dso(curr_map);
@@ -888,7 +889,7 @@ static int maps__split_kallsyms(struct maps *kmaps, struct dso *dso, u64 delta,
* symbols at this point.
*/
goto discard_symbol;
- } else if (curr_map != initial_map) {
+ } else if (!RC_CHK_EQUAL(curr_map, initial_map)) {
char dso_name[PATH_MAX];
struct dso *ndso;
@@ -899,7 +900,8 @@ static int maps__split_kallsyms(struct maps *kmaps, struct dso *dso, u64 delta,
}
if (count == 0) {
- curr_map = initial_map;
+ map__zput(curr_map);
+ curr_map = map__get(initial_map);
goto add_symbol;
}
@@ -913,6 +915,7 @@ static int maps__split_kallsyms(struct maps *kmaps, struct dso *dso, u64 delta,
kernel_range++);
ndso = dso__new(dso_name);
+ map__zput(curr_map);
if (ndso == NULL)
return -1;
@@ -926,6 +929,7 @@ static int maps__split_kallsyms(struct maps *kmaps, struct dso *dso, u64 delta,
map__set_mapping_type(curr_map, MAPPING_TYPE__IDENTITY);
if (maps__insert(kmaps, curr_map)) {
+ map__zput(curr_map);
dso__put(ndso);
return -1;
}
@@ -936,7 +940,7 @@ static int maps__split_kallsyms(struct maps *kmaps, struct dso *dso, u64 delta,
pos->end -= delta;
}
add_symbol:
- if (curr_map != initial_map) {
+ if (!RC_CHK_EQUAL(curr_map, initial_map)) {
struct dso *curr_map_dso = map__dso(curr_map);
rb_erase_cached(&pos->rb_node, root);
@@ -951,12 +955,12 @@ static int maps__split_kallsyms(struct maps *kmaps, struct dso *dso, u64 delta,
symbol__delete(pos);
}
- if (curr_map != initial_map &&
+ if (!RC_CHK_EQUAL(curr_map, initial_map) &&
dso->kernel == DSO_SPACE__KERNEL_GUEST &&
machine__is_default_guest(maps__machine(kmaps))) {
dso__set_loaded(map__dso(curr_map));
}
-
+ map__put(curr_map);
return count + moved;
}
--
2.43.0.rc2.451.g8631bc7472-goog
next prev parent reply other threads:[~2023-12-07 1:19 UTC|newest]
Thread overview: 56+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-12-07 1:16 [PATCH v6 00/47] maps/threads/dsos memory improvements and fixes Ian Rogers
2023-12-07 1:16 ` [PATCH v6 01/47] perf map: Improve map/unmap parameter names Ian Rogers
2023-12-11 23:38 ` Namhyung Kim
2023-12-11 23:39 ` Namhyung Kim
2023-12-11 23:52 ` Ian Rogers
2023-12-19 6:06 ` Namhyung Kim
2023-12-07 1:16 ` [PATCH v6 02/47] perf maps: Add maps__for_each_map to iterate maps holding the lock Ian Rogers
2023-12-07 1:16 ` [PATCH v6 03/47] perf events x86: Use function to add missing lock Ian Rogers
2023-12-07 1:16 ` [PATCH v6 04/47] perf report: Use function to add missing maps lock Ian Rogers
2023-12-07 1:16 ` [PATCH v6 05/47] perf tests: " Ian Rogers
2023-12-07 1:16 ` [PATCH v6 06/47] perf machine: " Ian Rogers
2023-12-07 1:16 ` [PATCH v6 07/47] perf probe-event: " Ian Rogers
2023-12-07 1:16 ` [PATCH v6 08/47] perf symbol: " Ian Rogers
2023-12-07 1:16 ` [PATCH v6 09/47] perf synthetic-events: " Ian Rogers
2023-12-07 1:16 ` [PATCH v6 10/47] perf thread: " Ian Rogers
2023-12-07 1:16 ` [PATCH v6 11/47] perf unwind: " Ian Rogers
2023-12-07 1:16 ` [PATCH v6 12/47] perf vdso: " Ian Rogers
2023-12-07 1:16 ` [PATCH v6 13/47] perf maps: Reduce scope of maps__for_each_entry Ian Rogers
2023-12-07 1:16 ` [PATCH v6 14/47] perf maps: Add remove maps function to remove a map based on callback Ian Rogers
2023-12-07 1:16 ` [PATCH v6 15/47] perf debug: Expose debug file Ian Rogers
2023-12-07 1:16 ` [PATCH v6 16/47] perf maps: Refactor maps__fixup_overlappings Ian Rogers
2023-12-07 1:16 ` [PATCH v6 17/47] perf maps: Do simple merge if given map doesn't overlap Ian Rogers
2023-12-07 1:16 ` [PATCH v6 18/47] perf maps: Rename clone to copy from Ian Rogers
2023-12-07 1:16 ` [PATCH v6 19/47] perf maps: Add maps__load_first Ian Rogers
2023-12-07 1:16 ` [PATCH v6 20/47] perf maps: Add find next entry to give entry after the given map Ian Rogers
2023-12-07 1:16 ` [PATCH v6 21/47] perf maps: Reduce scope of map_rb_node and maps internals Ian Rogers
2023-12-07 1:16 ` [PATCH v6 22/47] perf maps: Fix up overlaps during fixup_end Ian Rogers
2023-12-07 1:16 ` [PATCH v6 23/47] perf maps: Switch from rbtree to lazily sorted array for addresses Ian Rogers
2023-12-07 1:16 ` [PATCH v6 24/47] perf maps: Get map before returning in maps__find Ian Rogers
2023-12-07 1:16 ` Ian Rogers [this message]
2023-12-07 1:17 ` [PATCH v6 26/47] perf maps: Get map before returning in maps__find_next_entry Ian Rogers
2023-12-07 1:17 ` [PATCH v6 27/47] perf maps: Hide maps internals Ian Rogers
2023-12-07 1:17 ` [PATCH v6 28/47] perf maps: Locking tidy up of nr_maps Ian Rogers
2023-12-07 1:17 ` [PATCH v6 29/47] perf dso: Reorder variables to save space in struct dso Ian Rogers
2023-12-07 1:17 ` [PATCH v6 30/47] perf report: Sort child tasks by tid Ian Rogers
2023-12-07 1:17 ` [PATCH v6 31/47] perf trace: Ignore thread hashing in summary Ian Rogers
2023-12-07 1:17 ` [PATCH v6 32/47] perf machine: Move fprintf to for_each loop and a callback Ian Rogers
2023-12-07 1:17 ` [PATCH v6 33/47] perf threads: Move threads to its own files Ian Rogers
2023-12-07 1:17 ` [PATCH v6 34/47] perf threads: Switch from rbtree to hashmap Ian Rogers
2023-12-07 1:17 ` [PATCH v6 35/47] perf threads: Reduce table size from 256 to 8 Ian Rogers
2023-12-07 1:17 ` [PATCH v6 36/47] perf dsos: Attempt to better abstract dsos internals Ian Rogers
2023-12-07 1:17 ` [PATCH v6 37/47] perf dsos: Tidy reference counting and locking Ian Rogers
2023-12-07 1:17 ` [PATCH v6 38/47] perf dsos: Add dsos__for_each_dso Ian Rogers
2023-12-07 1:17 ` [PATCH v6 39/47] perf dso: Move dso functions out of dsos Ian Rogers
2023-12-07 1:17 ` [PATCH v6 40/47] perf dsos: Switch more loops to dsos__for_each_dso Ian Rogers
2023-12-07 1:17 ` [PATCH v6 41/47] perf dsos: Switch backing storage to array from rbtree/list Ian Rogers
2023-12-07 1:17 ` [PATCH v6 42/47] perf dsos: Remove __dsos__addnew Ian Rogers
2023-12-07 1:17 ` [PATCH v6 43/47] perf dsos: Remove __dsos__findnew_link_by_longname_id Ian Rogers
2023-12-07 1:17 ` [PATCH v6 44/47] perf dsos: Switch hand code to bsearch Ian Rogers
2023-12-07 1:17 ` [PATCH v6 45/47] perf dso: Add reference count checking and accessor functions Ian Rogers
2023-12-07 1:17 ` [PATCH v6 46/47] perf dso: Reference counting related fixes Ian Rogers
2023-12-07 1:17 ` [PATCH v6 47/47] perf dso: Use container_of to avoid a pointer in dso_data Ian Rogers
2023-12-18 20:53 ` [PATCH v6 00/47] maps/threads/dsos memory improvements and fixes Arnaldo Carvalho de Melo
2023-12-19 1:27 ` Arnaldo Carvalho de Melo
2023-12-20 17:46 ` Arnaldo Carvalho de Melo
2023-12-20 18:07 ` Arnaldo Carvalho de Melo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20231207011722.1220634-26-irogers@google.com \
--to=irogers@google.com \
--cc=9erthalion6@gmail.com \
--cc=acme@kernel.org \
--cc=adrian.hunter@intel.com \
--cc=ak@linux.intel.com \
--cc=alexander.shishkin@linux.intel.com \
--cc=amadio@gentoo.org \
--cc=atrajeev@linux.vnet.ibm.com \
--cc=changbin.du@huawei.com \
--cc=chenhuacai@kernel.org \
--cc=colin.i.king@gmail.com \
--cc=german.gomez@arm.com \
--cc=james.clark@arm.com \
--cc=jolsa@kernel.org \
--cc=kan.liang@linux.intel.com \
--cc=kjain@linux.ibm.com \
--cc=kprateek.nayak@amd.com \
--cc=leo.yan@linaro.org \
--cc=liam.howlett@oracle.com \
--cc=lidong@vivo.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-perf-users@vger.kernel.org \
--cc=liuwenyu7@huawei.com \
--cc=mark.rutland@arm.com \
--cc=mhiramat@kernel.org \
--cc=mingo@redhat.com \
--cc=namhyung@kernel.org \
--cc=ojeda@kernel.org \
--cc=pbonzini@redhat.com \
--cc=peterz@infradead.org \
--cc=ravi.bangoria@amd.com \
--cc=sandipan.das@amd.com \
--cc=seanjc@google.com \
--cc=sesse@google.com \
--cc=terrelln@fb.com \
--cc=vincent.whitchurch@axis.com \
--cc=wangming01@loongson.cn \
--cc=yangjihong1@huawei.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®