| 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: <20220211103415.2737789-18-irogers@google.com>
Date: Fri, 11 Feb 2022 02:34:10 -0800
From: Ian Rogers <irogers@...gle.com>
To: Peter Zijlstra <peterz@...radead.org>,
Ingo Molnar <mingo@...hat.com>,
Arnaldo Carvalho de Melo <acme@...nel.org>,
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>,
"André Almeida" <andrealmeid@...labora.com>,
James Clark <james.clark@....com>,
John Garry <john.garry@...wei.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>,
Jin Yao <yao.jin@...ux.intel.com>,
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>
Cc: eranian@...gle.com, Ian Rogers <irogers@...gle.com>
Subject: [PATCH v3 17/22] perf map: Changes to reference counting
When a pointer to a map exists do a get, when that pointer is
overwritten or freed, put the map. This avoids issues with gets and
puts being inconsistently used causing, use after puts, etc. Reference
count checking and address sanitizer were used to identify issues.
Signed-off-by: Ian Rogers <irogers@...gle.com>
---
tools/perf/tests/hists_cumulate.c | 14 ++++-
tools/perf/tests/hists_filter.c | 14 ++++-
tools/perf/tests/hists_link.c | 18 +++++-
tools/perf/tests/hists_output.c | 12 +++-
tools/perf/tests/mmap-thread-lookup.c | 3 +-
tools/perf/util/callchain.c | 9 +--
tools/perf/util/event.c | 8 ++-
tools/perf/util/hist.c | 10 ++--
tools/perf/util/machine.c | 80 ++++++++++++++++-----------
9 files changed, 118 insertions(+), 50 deletions(-)
diff --git a/tools/perf/tests/hists_cumulate.c b/tools/perf/tests/hists_cumulate.c
index 17f4fcd6bdce..28f5eb41eed9 100644
--- a/tools/perf/tests/hists_cumulate.c
+++ b/tools/perf/tests/hists_cumulate.c
@@ -112,6 +112,7 @@ static int add_hist_entries(struct hists *hists, struct machine *machine)
}
fake_samples[i].thread = al.thread;
+ map__put(fake_samples[i].map);
fake_samples[i].map = al.map;
fake_samples[i].sym = al.sym;
}
@@ -147,15 +148,23 @@ static void del_hist_entries(struct hists *hists)
}
}
+static void put_fake_samples(void)
+{
+ size_t i;
+
+ for (i = 0; i < ARRAY_SIZE(fake_samples); i++)
+ map__put(fake_samples[i].map);
+}
+
typedef int (*test_fn_t)(struct evsel *, struct machine *);
#define COMM(he) (thread__comm_str(he->thread))
-#define DSO(he) (he->ms.map->dso->short_name)
+#define DSO(he) (map__dso(he->ms.map)->short_name)
#define SYM(he) (he->ms.sym->name)
#define CPU(he) (he->cpu)
#define PID(he) (he->thread->tid)
#define DEPTH(he) (he->callchain->max_depth)
-#define CDSO(cl) (cl->ms.map->dso->short_name)
+#define CDSO(cl) (map__dso(cl->ms.map)->short_name)
#define CSYM(cl) (cl->ms.sym->name)
struct result {
@@ -733,6 +742,7 @@ static int test__hists_cumulate(struct test_suite *test __maybe_unused, int subt
/* tear down everything */
evlist__delete(evlist);
machines__exit(&machines);
+ put_fake_samples();
return err;
}
diff --git a/tools/perf/tests/hists_filter.c b/tools/perf/tests/hists_filter.c
index 08cbeb9e39ae..bcd46244182a 100644
--- a/tools/perf/tests/hists_filter.c
+++ b/tools/perf/tests/hists_filter.c
@@ -89,6 +89,7 @@ static int add_hist_entries(struct evlist *evlist,
}
fake_samples[i].thread = al.thread;
+ map__put(fake_samples[i].map);
fake_samples[i].map = al.map;
fake_samples[i].sym = al.sym;
}
@@ -101,6 +102,14 @@ static int add_hist_entries(struct evlist *evlist,
return TEST_FAIL;
}
+static void put_fake_samples(void)
+{
+ size_t i;
+
+ for (i = 0; i < ARRAY_SIZE(fake_samples); i++)
+ map__put(fake_samples[i].map);
+}
+
static int test__hists_filter(struct test_suite *test __maybe_unused, int subtest __maybe_unused)
{
int err = TEST_FAIL;
@@ -194,7 +203,7 @@ static int test__hists_filter(struct test_suite *test __maybe_unused, int subtes
hists__filter_by_thread(hists);
/* now applying dso filter for 'kernel' */
- hists->dso_filter = fake_samples[0].map->dso;
+ hists->dso_filter = map__dso(fake_samples[0].map);
hists__filter_by_dso(hists);
if (verbose > 2) {
@@ -288,7 +297,7 @@ static int test__hists_filter(struct test_suite *test __maybe_unused, int subtes
/* now applying all filters at once. */
hists->thread_filter = fake_samples[1].thread;
- hists->dso_filter = fake_samples[1].map->dso;
+ hists->dso_filter = map__dso(fake_samples[1].map);
hists__filter_by_thread(hists);
hists__filter_by_dso(hists);
@@ -322,6 +331,7 @@ static int test__hists_filter(struct test_suite *test __maybe_unused, int subtes
evlist__delete(evlist);
reset_output_field();
machines__exit(&machines);
+ put_fake_samples();
return err;
}
diff --git a/tools/perf/tests/hists_link.c b/tools/perf/tests/hists_link.c
index c575e13a850d..060e8731feff 100644
--- a/tools/perf/tests/hists_link.c
+++ b/tools/perf/tests/hists_link.c
@@ -6,6 +6,7 @@
#include "evsel.h"
#include "evlist.h"
#include "machine.h"
+#include "map.h"
#include "parse-events.h"
#include "hists_common.h"
#include "util/mmap.h"
@@ -94,6 +95,7 @@ static int add_hist_entries(struct evlist *evlist, struct machine *machine)
}
fake_common_samples[k].thread = al.thread;
+ map__put(fake_common_samples[k].map);
fake_common_samples[k].map = al.map;
fake_common_samples[k].sym = al.sym;
}
@@ -126,11 +128,24 @@ static int add_hist_entries(struct evlist *evlist, struct machine *machine)
return -1;
}
+static void put_fake_samples(void)
+{
+ size_t i, j;
+
+ for (i = 0; i < ARRAY_SIZE(fake_common_samples); i++)
+ map__put(fake_common_samples[i].map);
+ for (i = 0; i < ARRAY_SIZE(fake_samples); i++) {
+ for (j = 0; j < ARRAY_SIZE(fake_samples[0]); j++)
+ map__put(fake_samples[i][j].map);
+ }
+}
+
static int find_sample(struct sample *samples, size_t nr_samples,
struct thread *t, struct map *m, struct symbol *s)
{
while (nr_samples--) {
- if (samples->thread == t && samples->map == m &&
+ if (samples->thread == t &&
+ samples->map == m &&
samples->sym == s)
return 1;
samples++;
@@ -336,6 +351,7 @@ static int test__hists_link(struct test_suite *test __maybe_unused, int subtest
evlist__delete(evlist);
reset_output_field();
machines__exit(&machines);
+ put_fake_samples();
return err;
}
diff --git a/tools/perf/tests/hists_output.c b/tools/perf/tests/hists_output.c
index 0bde4a768c15..4af6916491e5 100644
--- a/tools/perf/tests/hists_output.c
+++ b/tools/perf/tests/hists_output.c
@@ -78,6 +78,7 @@ static int add_hist_entries(struct hists *hists, struct machine *machine)
}
fake_samples[i].thread = al.thread;
+ map__put(fake_samples[i].map);
fake_samples[i].map = al.map;
fake_samples[i].sym = al.sym;
}
@@ -113,10 +114,18 @@ static void del_hist_entries(struct hists *hists)
}
}
+static void put_fake_samples(void)
+{
+ size_t i;
+
+ for (i = 0; i < ARRAY_SIZE(fake_samples); i++)
+ map__put(fake_samples[i].map);
+}
+
typedef int (*test_fn_t)(struct evsel *, struct machine *);
#define COMM(he) (thread__comm_str(he->thread))
-#define DSO(he) (he->ms.map->dso->short_name)
+#define DSO(he) (map__dso(he->ms.map)->short_name)
#define SYM(he) (he->ms.sym->name)
#define CPU(he) (he->cpu)
#define PID(he) (he->thread->tid)
@@ -620,6 +629,7 @@ static int test__hists_output(struct test_suite *test __maybe_unused, int subtes
/* tear down everything */
evlist__delete(evlist);
machines__exit(&machines);
+ put_fake_samples();
return err;
}
diff --git a/tools/perf/tests/mmap-thread-lookup.c b/tools/perf/tests/mmap-thread-lookup.c
index a4301fc7b770..898eda55b7a8 100644
--- a/tools/perf/tests/mmap-thread-lookup.c
+++ b/tools/perf/tests/mmap-thread-lookup.c
@@ -202,7 +202,8 @@ static int mmap_events(synth_cb synth)
break;
}
- pr_debug("map %p, addr %" PRIx64 "\n", al.map, al.map->start);
+ pr_debug("map %p, addr %" PRIx64 "\n", al.map, map__start(al.map));
+ map__put(al.map);
}
machine__delete_threads(machine);
diff --git a/tools/perf/util/callchain.c b/tools/perf/util/callchain.c
index a8cfd31a3ff0..ae65b7bc9ab7 100644
--- a/tools/perf/util/callchain.c
+++ b/tools/perf/util/callchain.c
@@ -583,7 +583,7 @@ fill_node(struct callchain_node *node, struct callchain_cursor *cursor)
}
call->ip = cursor_node->ip;
call->ms = cursor_node->ms;
- map__get(call->ms.map);
+ call->ms.map = map__get(call->ms.map);
call->srcline = cursor_node->srcline;
if (cursor_node->branch) {
@@ -1061,7 +1061,7 @@ int callchain_cursor_append(struct callchain_cursor *cursor,
node->ip = ip;
map__zput(node->ms.map);
node->ms = *ms;
- map__get(node->ms.map);
+ node->ms.map = map__get(node->ms.map);
node->branch = branch;
node->nr_loop_iter = nr_loop_iter;
node->iter_cycles = iter_cycles;
@@ -1109,7 +1109,8 @@ int fill_callchain_info(struct addr_location *al, struct callchain_cursor_node *
struct machine *machine = maps__machine(node->ms.maps);
al->maps = node->ms.maps;
- al->map = node->ms.map;
+ map__put(al->map);
+ al->map = map__get(node->ms.map);
al->sym = node->ms.sym;
al->srcline = node->srcline;
al->addr = node->ip;
@@ -1530,7 +1531,7 @@ int callchain_node__make_parent_list(struct callchain_node *node)
goto out;
*new = *chain;
new->has_children = false;
- map__get(new->ms.map);
+ new->ms.map = map__get(new->ms.map);
list_add_tail(&new->list, &head);
}
parent = parent->parent;
diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index 54a1d4df5f70..266318d5d006 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -484,13 +484,14 @@ size_t perf_event__fprintf_text_poke(union perf_event *event, struct machine *ma
if (machine) {
struct addr_location al;
- al.map = maps__find(machine__kernel_maps(machine), tp->addr);
+ al.map = map__get(maps__find(machine__kernel_maps(machine), tp->addr));
if (al.map && map__load(al.map) >= 0) {
al.addr = map__map_ip(al.map, tp->addr);
al.sym = map__find_symbol(al.map, al.addr);
if (al.sym)
ret += symbol__fprintf_symname_offs(al.sym, &al, fp);
}
+ map__put(al.map);
}
ret += fprintf(fp, " old len %u new len %u\n", tp->old_len, tp->new_len);
old = true;
@@ -581,6 +582,7 @@ struct map *thread__find_map(struct thread *thread, u8 cpumode, u64 addr,
al->filtered = 0;
if (machine == NULL) {
+ map__put(al->map);
al->map = NULL;
return NULL;
}
@@ -599,6 +601,7 @@ struct map *thread__find_map(struct thread *thread, u8 cpumode, u64 addr,
al->level = 'u';
} else {
al->level = 'H';
+ map__put(al->map);
al->map = NULL;
if ((cpumode == PERF_RECORD_MISC_GUEST_USER ||
@@ -613,7 +616,7 @@ struct map *thread__find_map(struct thread *thread, u8 cpumode, u64 addr,
return NULL;
}
- al->map = maps__find(maps, al->addr);
+ al->map = map__get(maps__find(maps, al->addr));
if (al->map != NULL) {
/*
* Kernel maps might be changed when loading symbols so loading
@@ -768,6 +771,7 @@ int machine__resolve(struct machine *machine, struct addr_location *al,
*/
void addr_location__put(struct addr_location *al)
{
+ map__zput(al->map);
thread__zput(al->thread);
}
diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index f19ac6eb4775..4dbb1dbf3679 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -446,7 +446,7 @@ static int hist_entry__init(struct hist_entry *he,
memset(&he->stat, 0, sizeof(he->stat));
}
- map__get(he->ms.map);
+ he->ms.map = map__get(he->ms.map);
if (he->branch_info) {
/*
@@ -461,13 +461,13 @@ static int hist_entry__init(struct hist_entry *he,
memcpy(he->branch_info, template->branch_info,
sizeof(*he->branch_info));
- map__get(he->branch_info->from.ms.map);
- map__get(he->branch_info->to.ms.map);
+ he->branch_info->from.ms.map = map__get(he->branch_info->from.ms.map);
+ he->branch_info->to.ms.map = map__get(he->branch_info->to.ms.map);
}
if (he->mem_info) {
- map__get(he->mem_info->iaddr.ms.map);
- map__get(he->mem_info->daddr.ms.map);
+ he->mem_info->iaddr.ms.map = map__get(he->mem_info->iaddr.ms.map);
+ he->mem_info->daddr.ms.map = map__get(he->mem_info->daddr.ms.map);
}
if (hist_entry__has_callchains(he) && symbol_conf.use_callchain)
diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index 940fb2a50dfd..49e4891e92b7 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -783,33 +783,42 @@ static int machine__process_ksymbol_register(struct machine *machine,
{
struct symbol *sym;
struct map *map = maps__find(machine__kernel_maps(machine), event->ksymbol.addr);
+ bool put_map = false;
+ int err = 0;
if (!map) {
struct dso *dso = dso__new(event->ksymbol.name);
- int err;
- if (dso) {
- dso->kernel = DSO_SPACE__KERNEL;
- map = map__new2(0, dso);
- dso__put(dso);
+ if (!dso) {
+ err = -ENOMEM;
+ goto out;
}
-
- if (!dso || !map) {
- return -ENOMEM;
+ dso->kernel = DSO_SPACE__KERNEL;
+ map = map__new2(0, dso);
+ dso__put(dso);
+ if (!map) {
+ err = -ENOMEM;
+ goto out;
}
-
+ /*
+ * The inserted map has a get on it, we need to put to release
+ * the reference count here, but do it after all accesses are
+ * done.
+ */
+ put_map = true;
if (event->ksymbol.ksym_type == PERF_RECORD_KSYMBOL_TYPE_OOL) {
- map->dso->binary_type = DSO_BINARY_TYPE__OOL;
- map->dso->data.file_size = event->ksymbol.len;
- dso__set_loaded(map->dso);
+ map__dso(map)->binary_type = DSO_BINARY_TYPE__OOL;
+ map__dso(map)->data.file_size = event->ksymbol.len;
+ dso__set_loaded(map__dso(map));
}
map->start = event->ksymbol.addr;
- map->end = map->start + event->ksymbol.len;
+ map->end = map__start(map) + event->ksymbol.len;
err = maps__insert(machine__kernel_maps(machine), map);
- map__put(map);
- if (err)
- return err;
+ if (err) {
+ err = -ENOMEM;
+ goto out;
+ }
dso__set_loaded(dso);
@@ -819,13 +828,18 @@ static int machine__process_ksymbol_register(struct machine *machine,
}
}
- sym = symbol__new(map->map_ip(map, map->start),
+ sym = symbol__new(map__map_ip(map, map__start(map)),
event->ksymbol.len,
0, 0, event->ksymbol.name);
- if (!sym)
- return -ENOMEM;
- dso__insert_symbol(map->dso, sym);
- return 0;
+ if (!sym) {
+ err = -ENOMEM;
+ goto out;
+ }
+ dso__insert_symbol(map__dso(map), sym);
+out:
+ if (put_map)
+ map__put(map);
+ return err;
}
static int machine__process_ksymbol_unregister(struct machine *machine,
@@ -925,14 +939,11 @@ static struct map *machine__addnew_module_map(struct machine *machine, u64 start
goto out;
err = maps__insert(machine__kernel_maps(machine), map);
-
- /* Put the map here because maps__insert already got it */
- map__put(map);
-
/* If maps__insert failed, return NULL. */
- if (err)
+ if (err) {
+ map__put(map);
map = NULL;
-
+ }
out:
/* put the dso here, corresponding to machine__findnew_module_dso */
dso__put(dso);
@@ -1228,6 +1239,7 @@ __machine__create_kernel_maps(struct machine *machine, struct dso *kernel)
/* In case of renewal the kernel map, destroy previous one */
machine__destroy_kernel_maps(machine);
+ map__put(machine->vmlinux_map);
machine->vmlinux_map = map__new2(0, kernel);
if (machine->vmlinux_map == NULL)
return -ENOMEM;
@@ -1513,6 +1525,7 @@ static int machine__create_module(void *arg, const char *name, u64 start,
map->end = start + size;
dso__kernel_module_get_build_id(map__dso(map), machine->root_dir);
+ map__put(map);
return 0;
}
@@ -1558,16 +1571,18 @@ static void machine__set_kernel_mmap(struct machine *machine,
static int machine__update_kernel_mmap(struct machine *machine,
u64 start, u64 end)
{
- struct map *map = machine__kernel_map(machine);
+ struct map *orig, *updated;
int err;
- map__get(map);
- maps__remove(machine__kernel_maps(machine), map);
+ orig = machine->vmlinux_map;
+ updated = map__get(orig);
+ machine->vmlinux_map = updated;
machine__set_kernel_mmap(machine, start, end);
+ maps__remove(machine__kernel_maps(machine), orig);
+ err = maps__insert(machine__kernel_maps(machine), updated);
+ map__put(orig);
- err = maps__insert(machine__kernel_maps(machine), map);
- map__put(map);
return err;
}
@@ -2246,6 +2261,7 @@ static int add_callchain_ip(struct thread *thread,
err = callchain_cursor_append(cursor, ip, &ms,
branch, flags, nr_loop_iter,
iter_cycles, branch_from, srcline);
+ map__put(al.map);
return err;
}
--
2.35.1.265.g69c8d7142f-goog
Powered by blists - more mailing lists