Linux Perf Users
 help / color / mirror / Atom feed
From: Ian Rogers <irogers@google.com>
To: irogers@google.com, acme@kernel.org, james.clark@linaro.org,
	 namhyung@kernel.org
Cc: adrian.hunter@intel.com, gmx@google.com, jolsa@kernel.org,
	 linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org,
	 mingo@redhat.com, peterz@infradead.org
Subject: [PATCH v8 1/4] perf maps: Add maps__mutate_mapping
Date: Tue, 19 May 2026 23:30:47 -0700	[thread overview]
Message-ID: <20260520063050.3917261-2-irogers@google.com> (raw)
In-Reply-To: <20260520063050.3917261-1-irogers@google.com>

During kernel ELF symbol parsing (dso__process_kernel_symbol), proc
kallsyms image loading (dso__load_kernel_sym,
dso__load_guest_kernel_sym), and dynamic kernel memory map alignment
updates (machine__update_kernel_mmap), the loader directly modifies
live virtual address boundary keys fields on map objects.  If these
boundaries are mutated while the map pointer actively resides inside
the parent maps cache array list (kmaps) outside of any lock closure,
an unsafe concurrent window is exposed where parallel worker lookup
threads (e.g., inside perf top) can mistakenly assume the cache
remains sorted based on stale parameters, executing binary search
queries (bsearch) across an unsorted range and triggering lookup
failures.

Fix this by introducing maps__mutate_mapping() that explicitly
acquires the parent maps write semaphore lock, executes an incoming
mutation callback block to perform the field updates under lock
protection, and invalidates the sorted tracking flags prior to
releasing the write lock. This guarantees synchronization invariants,
closing the concurrent lookup race window. The adjacent module
alignment pass inside machine__create_kernel_maps() is safely
preserved as a high-performance lockless pass, as its invocation
lifecycle bounds remain strictly single-threaded by contract during
session initialization construction.  To safely support this
unconditional down_write write lock mutator without recursive
read-to-write self-deadlock upgrades during lazy symbol loading, we
introduce a public maps__load_maps() API. It copies map pointers under
a brief read lock and force-loads all modules locklessly outside the
lock. Callers (such as perf inject) must pre-load all kernel symbol
maps up front at startup using maps__load_maps(), completely bypassing
dynamic runtime mutations.

Fixes: 39b12f781271 ("perf tools: Make it possible to read object code from vmlinux")
Assisted-by: Antigravity:gemini-3.5-flash
Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/util/machine.c    | 32 +++++++++------
 tools/perf/util/maps.c       | 76 ++++++++++++++++++++++++++++++++++++
 tools/perf/util/maps.h       |  3 ++
 tools/perf/util/symbol-elf.c | 41 ++++++++++++-------
 tools/perf/util/symbol.c     | 17 ++++++--
 5 files changed, 138 insertions(+), 31 deletions(-)

diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index e76f8c86e62a..ea918f75e3ad 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -1522,22 +1522,30 @@ static void machine__set_kernel_mmap(struct machine *machine,
 		map__set_end(machine->vmlinux_map, ~0ULL);
 }
 
-static int machine__update_kernel_mmap(struct machine *machine,
-				     u64 start, u64 end)
+struct kernel_mmap_mutation_ctx {
+	u64 start;
+	u64 end;
+};
+
+static int kernel_mmap_mutate_cb(struct map *map, void *data)
 {
-	struct map *orig, *updated;
-	int err;
+	struct kernel_mmap_mutation_ctx *ctx = data;
 
-	orig = machine->vmlinux_map;
-	updated = map__get(orig);
+	map__set_start(map, ctx->start);
+	map__set_end(map, ctx->end);
+	if (ctx->start == 0 && ctx->end == 0)
+		map__set_end(map, ~0ULL);
+	return 0;
+}
 
-	machine->vmlinux_map = updated;
-	maps__remove(machine__kernel_maps(machine), orig);
-	machine__set_kernel_mmap(machine, start, end);
-	err = maps__insert(machine__kernel_maps(machine), updated);
-	map__put(orig);
+static int machine__update_kernel_mmap(struct machine *machine,
+				       u64 start, u64 end)
+{
+	struct kernel_mmap_mutation_ctx ctx = { .start = start, .end = end };
 
-	return err;
+	return maps__mutate_mapping(machine__kernel_maps(machine),
+				     machine->vmlinux_map,
+				     kernel_mmap_mutate_cb, &ctx);
 }
 
 int machine__create_kernel_maps(struct machine *machine)
diff --git a/tools/perf/util/maps.c b/tools/perf/util/maps.c
index 923935ee21b6..7dce07e4d9b4 100644
--- a/tools/perf/util/maps.c
+++ b/tools/perf/util/maps.c
@@ -576,6 +576,49 @@ void maps__remove(struct maps *maps, struct map *map)
 #endif
 }
 
+/**
+ * maps__mutate_mapping - Apply write-protected mutations to a map.
+ * @maps: The maps collection containing the map.
+ * @map: The map to mutate.
+ * @mutate_cb: Callback function that performs the actual mutations.
+ * @data: Private data passed to the callback.
+ *
+ * This acquires the write lock on the maps semaphore to safely protect
+ * concurrent readers from seeing partially mutated or unsorted map boundaries.
+ *
+ * WARNING: Acquiring down_write() here can trigger a recursive self-deadlock if
+ * the caller already holds the read lock (e.g., during maps__for_each_map() or
+ * maps__find() iteration paths that trigger lazy symbol loading). To completely
+ * avoid this deadlock, all kernel/module maps must be pre-loaded up-front (via
+ * maps__load_maps()) under a clean, single-threaded context before entering
+ * multi-threaded event processing loops.
+ */
+int maps__mutate_mapping(struct maps *maps, struct map *map,
+			 int (*mutate_cb)(struct map *map, void *data), void *data)
+{
+	int err = 0;
+
+	if (maps)
+		down_write(maps__lock(maps));
+
+	err = mutate_cb(map, data);
+
+	if (maps) {
+		RC_CHK_ACCESS(maps)->maps_by_address_sorted = false;
+		RC_CHK_ACCESS(maps)->maps_by_name_sorted = false;
+	}
+
+	if (maps)
+		up_write(maps__lock(maps));
+
+#ifdef HAVE_LIBDW_SUPPORT
+	if (maps)
+		libdw__invalidate_dwfl(maps, maps__libdw_addr_space_dwfl(maps));
+#endif
+
+	return err;
+}
+
 bool maps__empty(struct maps *maps)
 {
 	bool res;
@@ -626,6 +669,39 @@ int maps__for_each_map(struct maps *maps, int (*cb)(struct map *map, void *data)
 	return ret;
 }
 
+int maps__load_maps(struct maps *maps)
+{
+	struct map **maps_copy;
+	unsigned int nr_maps;
+	int err = 0;
+
+	if (!maps)
+		return 0;
+
+	down_read(maps__lock(maps));
+	nr_maps = maps__nr_maps(maps);
+	if (nr_maps == 0) {
+		up_read(maps__lock(maps));
+		return 0;
+	}
+	maps_copy = calloc(nr_maps, sizeof(*maps_copy));
+	if (!maps_copy) {
+		up_read(maps__lock(maps));
+		return -ENOMEM;
+	}
+	for (unsigned int i = 0; i < nr_maps; i++)
+		maps_copy[i] = map__get(maps__maps_by_address(maps)[i]);
+	up_read(maps__lock(maps));
+
+	for (unsigned int i = 0; i < nr_maps; i++) {
+		if (map__load(maps_copy[i]) < 0)
+			err = -1;
+		map__put(maps_copy[i]);
+	}
+	free(maps_copy);
+	return err;
+}
+
 void maps__remove_maps(struct maps *maps, bool (*cb)(struct map *map, void *data), void *data)
 {
 	struct map **maps_by_address;
diff --git a/tools/perf/util/maps.h b/tools/perf/util/maps.h
index 5b80b199685e..4ec9b7453a3b 100644
--- a/tools/perf/util/maps.h
+++ b/tools/perf/util/maps.h
@@ -59,8 +59,11 @@ void maps__set_libdw_addr_space_dwfl(struct maps *maps, void *dwfl);
 
 size_t maps__fprintf(struct maps *maps, FILE *fp);
 
+int maps__load_maps(struct maps *maps);
 int maps__insert(struct maps *maps, struct map *map);
 void maps__remove(struct maps *maps, struct map *map);
+int maps__mutate_mapping(struct maps *maps, struct map *map,
+			 int (*mutate_cb)(struct map *map, void *data), void *data);
 
 struct map *maps__find(struct maps *maps, u64 addr);
 struct symbol *maps__find_symbol(struct maps *maps, u64 addr, struct map **mapp);
diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index 7afa8a117139..dc4ab58857b3 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -1341,6 +1341,24 @@ static u64 ref_reloc(struct kmap *kmap)
 void __weak arch__sym_update(struct symbol *s __maybe_unused,
 		GElf_Sym *sym __maybe_unused) { }
 
+struct remap_kernel_ctx {
+	u64 sh_addr;
+	u64 sh_size;
+	u64 sh_offset;
+	struct kmap *kmap;
+};
+
+static int remap_kernel_cb(struct map *map, void *data)
+{
+	struct remap_kernel_ctx *ctx = data;
+
+	map__set_start(map, ctx->sh_addr + ref_reloc(ctx->kmap));
+	map__set_end(map, map__start(map) + ctx->sh_size);
+	map__set_pgoff(map, ctx->sh_offset);
+	map__set_mapping_type(map, MAPPING_TYPE__DSO);
+	return 0;
+}
+
 static int dso__process_kernel_symbol(struct dso *dso, struct map *map,
 				      GElf_Sym *sym, GElf_Shdr *shdr,
 				      struct maps *kmaps, struct kmap *kmap,
@@ -1371,22 +1389,15 @@ static int dso__process_kernel_symbol(struct dso *dso, struct map *map,
 		 * map to the kernel dso.
 		 */
 		if (*remap_kernel && dso__kernel(dso) && !kmodule) {
+			struct remap_kernel_ctx ctx = {
+				.sh_addr = shdr->sh_addr,
+				.sh_size = shdr->sh_size,
+				.sh_offset = shdr->sh_offset,
+				.kmap = kmap
+			};
+
 			*remap_kernel = false;
-			map__set_start(map, shdr->sh_addr + ref_reloc(kmap));
-			map__set_end(map, map__start(map) + shdr->sh_size);
-			map__set_pgoff(map, shdr->sh_offset);
-			map__set_mapping_type(map, MAPPING_TYPE__DSO);
-			/* Ensure maps are correctly ordered */
-			if (kmaps) {
-				int err;
-				struct map *tmp = map__get(map);
-
-				maps__remove(kmaps, map);
-				err = maps__insert(kmaps, map);
-				map__put(tmp);
-				if (err)
-					return err;
-			}
+			maps__mutate_mapping(kmaps, map, remap_kernel_cb, &ctx);
 		}
 
 		/*
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index fcaeeddbbb6b..09b93e844887 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -48,6 +48,13 @@
 #include <symbol/kallsyms.h>
 #include <sys/utsname.h>
 
+static int map_fixup_cb(struct map *map, void *data __maybe_unused)
+{
+	map__fixup_start(map);
+	map__fixup_end(map);
+	return 0;
+}
+
 static int dso__load_kernel_sym(struct dso *dso, struct map *map);
 static int dso__load_guest_kernel_sym(struct dso *dso, struct map *map);
 static bool symbol__is_idle(const char *name);
@@ -2121,10 +2128,11 @@ static int dso__load_kernel_sym(struct dso *dso, struct map *map)
 	free(kallsyms_allocated_filename);
 
 	if (err > 0 && !dso__is_kcore(dso)) {
+		struct maps *kmaps = map__kmaps(map);
+
 		dso__set_binary_type(dso, DSO_BINARY_TYPE__KALLSYMS);
 		dso__set_long_name(dso, DSO__NAME_KALLSYMS, false);
-		map__fixup_start(map);
-		map__fixup_end(map);
+		maps__mutate_mapping(kmaps, map, map_fixup_cb, NULL);
 	}
 
 	return err;
@@ -2164,10 +2172,11 @@ static int dso__load_guest_kernel_sym(struct dso *dso, struct map *map)
 	if (err > 0)
 		pr_debug("Using %s for symbols\n", kallsyms_filename);
 	if (err > 0 && !dso__is_kcore(dso)) {
+		struct maps *kmaps = map__kmaps(map);
+
 		dso__set_binary_type(dso, DSO_BINARY_TYPE__GUEST_KALLSYMS);
 		dso__set_long_name(dso, machine->mmap_name, false);
-		map__fixup_start(map);
-		map__fixup_end(map);
+		maps__mutate_mapping(kmaps, map, map_fixup_cb, NULL);
 	}
 
 	return err;
-- 
2.54.0.631.ge1b05301d1-goog


  reply	other threads:[~2026-05-20  6:31 UTC|newest]

Thread overview: 70+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-24 22:05 [PATCH v1 1/2] perf inject/aslr: Add aslr tool to remap/obfuscate virtual addresses Ian Rogers
2026-04-24 22:05 ` [PATCH v1 2/2] perf test: Add inject ASLR test Ian Rogers
2026-04-24 22:47   ` sashiko-bot
2026-04-24 22:36 ` [PATCH v1 1/2] perf inject/aslr: Add aslr tool to remap/obfuscate virtual addresses sashiko-bot
2026-04-25  2:05 ` [PATCH v2 " Ian Rogers
2026-04-25  2:05   ` [PATCH v2 2/2] perf test: Add inject ASLR test Ian Rogers
2026-05-04  3:51   ` [PATCH v3 0/4] perf tools: Add inject --aslr feature and prerequisite robustness fixes Ian Rogers
2026-05-04  3:51     ` [PATCH v3 1/4] perf sched: Add missing mmap2 handler in timehist Ian Rogers
2026-05-04  3:51     ` [PATCH v3 2/4] perf tool: Fix missing schedstat delegates and dont_split_sample_group in delegate_tool Ian Rogers
2026-05-04  3:51     ` [PATCH v3 3/4] perf inject/aslr: Add aslr tool to remap/obfuscate virtual addresses Ian Rogers
2026-05-04  4:51       ` sashiko-bot
2026-05-04  3:51     ` [PATCH v3 4/4] perf test: Add inject ASLR test Ian Rogers
2026-05-04  5:02       ` sashiko-bot
2026-05-04  7:29     ` [PATCH v4 0/4] perf tools: Add inject --aslr feature and prerequisite robustness fixes Ian Rogers
2026-05-04  7:29       ` [PATCH v4 1/4] perf sched: Add missing mmap2 handler in timehist Ian Rogers
2026-05-04  7:29       ` [PATCH v4 2/4] perf tool: Fix missing schedstat delegates and dont_split_sample_group in delegate_tool Ian Rogers
2026-05-04  7:29       ` [PATCH v4 3/4] perf inject/aslr: Add aslr tool to remap/obfuscate virtual addresses Ian Rogers
2026-05-04  8:39         ` sashiko-bot
2026-05-04  7:29       ` [PATCH v4 4/4] perf test: Add inject ASLR test Ian Rogers
2026-05-04  8:48         ` sashiko-bot
2026-05-04  8:23       ` [PATCH v4 0/4] perf tools: Add inject --aslr feature and prerequisite robustness fixes Ian Rogers
2026-05-06  0:45       ` [PATCH v5 0/5] " Ian Rogers
2026-05-06  0:45         ` [PATCH v5 1/5] perf sched: Add missing mmap2 handler in timehist Ian Rogers
2026-05-06 13:22           ` Arnaldo Carvalho de Melo
2026-05-06 16:16             ` Ian Rogers
2026-05-06  0:45         ` [PATCH v5 2/5] perf tool: Fix missing schedstat delegates and dont_split_sample_group in delegate_tool Ian Rogers
2026-05-06  0:45         ` [PATCH v5 3/5] perf symbols: Fix map removal sequence inside dso__process_kernel_symbol() Ian Rogers
2026-05-06  1:45           ` sashiko-bot
2026-05-06  0:45         ` [PATCH v5 4/5] perf inject/aslr: Add aslr tool to remap/obfuscate virtual addresses Ian Rogers
2026-05-06  2:40           ` sashiko-bot
2026-05-06 18:52           ` Namhyung Kim
2026-05-06 20:01             ` Ian Rogers
2026-05-06  0:45         ` [PATCH v5 5/5] perf test: Add inject ASLR test Ian Rogers
2026-05-07 15:58           ` James Clark
2026-05-07 16:17             ` Ian Rogers
2026-05-08 10:42               ` James Clark
2026-05-08 10:49                 ` James Clark
2026-05-08  8:27         ` [PATCH v6 0/6] perf tools: Add inject --aslr feature and prerequisite robustness fixes Ian Rogers
2026-05-08  8:27           ` [PATCH v6 1/6] perf sched: Add missing mmap2 handler in timehist Ian Rogers
2026-05-08  8:27           ` [PATCH v6 2/6] perf tool: Missing delegate_tool schedstat delegates and dont_split_sample_group Ian Rogers
2026-05-08  8:27           ` [PATCH v6 3/6] perf maps: Add maps__mutate_mapping Ian Rogers
2026-05-08 10:57             ` James Clark
2026-05-08 20:37             ` sashiko-bot
2026-05-11  7:07             ` Namhyung Kim
2026-05-08  8:27           ` [PATCH v6 4/6] perf inject/aslr: Add aslr tool to remap/obfuscate virtual addresses Ian Rogers
2026-05-08 21:22             ` sashiko-bot
2026-05-11  7:32             ` Namhyung Kim
2026-05-08  8:27           ` [PATCH v6 5/6] perf test: Add inject ASLR test Ian Rogers
2026-05-08 13:29             ` James Clark
2026-05-08 14:29               ` James Clark
2026-05-11  7:34             ` Namhyung Kim
2026-05-08  8:27           ` [PATCH v6 6/6] perf aslr: Strip sample registers Ian Rogers
2026-05-08 21:49             ` sashiko-bot
2026-05-19  8:08           ` [PATCH v7 0/4] perf tools: Add inject --aslr feature, early maps loading, and decoupling fixes Ian Rogers
2026-05-19  8:08             ` [PATCH v7 1/4] perf maps: Add maps__mutate_mapping Ian Rogers
2026-05-19  8:38               ` sashiko-bot
2026-05-19  8:08             ` [PATCH v7 2/4] perf inject/aslr: Add aslr tool to remap/obfuscate virtual addresses Ian Rogers
2026-05-19  9:14               ` sashiko-bot
2026-05-19  8:08             ` [PATCH v7 3/4] perf test: Add inject ASLR test Ian Rogers
2026-05-19  8:08             ` [PATCH v7 4/4] perf aslr: Strip sample registers Ian Rogers
2026-05-19  9:55               ` sashiko-bot
2026-05-20  6:30             ` [PATCH v8 0/4] perf tools: Add inject --aslr feature, early maps loading, and decoupling fixes Ian Rogers
2026-05-20  6:30               ` Ian Rogers [this message]
2026-05-20  7:06                 ` [PATCH v8 1/4] perf maps: Add maps__mutate_mapping sashiko-bot
2026-05-20  6:30               ` [PATCH v8 2/4] perf inject/aslr: Add aslr tool to remap/obfuscate virtual addresses Ian Rogers
2026-05-20  7:50                 ` sashiko-bot
2026-05-20  6:30               ` [PATCH v8 3/4] perf test: Add inject ASLR test Ian Rogers
2026-05-20  8:02                 ` sashiko-bot
2026-05-20  6:30               ` [PATCH v8 4/4] perf aslr: Strip sample registers Ian Rogers
2026-05-20  8:41                 ` sashiko-bot

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=20260520063050.3917261-2-irogers@google.com \
    --to=irogers@google.com \
    --cc=acme@kernel.org \
    --cc=adrian.hunter@intel.com \
    --cc=gmx@google.com \
    --cc=james.clark@linaro.org \
    --cc=jolsa@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=namhyung@kernel.org \
    --cc=peterz@infradead.org \
    /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