Re: [PATCH v9 1/5] perf maps: Add maps__mutate_mapping

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Ignoring the return value of maps__load_maps() and a TOCTOU race expose a recursive read-to-write self-deadlock in maps__find_symbol_by_name().
--

commit 0d10c2abb7793e097fe64599f4a0512e9d3d888d
Author: Ian Rogers <irogers@google.com>

perf maps: Add maps__mutate_mapping

This patch introduces maps__mutate_mapping() to safely update map boundaries
under a write lock. It also adds maps__load_maps() to pre-load kernel symbol
maps up front, aiming to avoid recursive read-to-write self-deadlocks during
lazy symbol loading.

> diff --git a/tools/perf/util/maps.c b/tools/perf/util/maps.c
> index 923935ee21b66..332222365f237 100644
> --- a/tools/perf/util/maps.c
> +++ b/tools/perf/util/maps.c
[ ... ]
> @@ -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)
> +{
[ ... ]
> +	maps_copy = calloc(nr_maps, sizeof(*maps_copy));
> +	if (!maps_copy) {
> +		up_read(maps__lock(maps));
> +		return -ENOMEM;
> +	}
[ ... ]
> @@ -700,6 +776,7 @@ struct symbol *maps__find_symbol_by_name(struct maps *maps, const char *name, st
>  		.sym = NULL,
>  	};
>  
> +	maps__load_maps(maps);
>  	maps__for_each_map(maps, maps__find_symbol_by_name_cb, &args);

[Severity: Medium]
Does this expose a recursive read-to-write self-deadlock if maps__load_maps()
fails?

If the maps_copy allocation in maps__load_maps() fails with -ENOMEM, it
returns early and leaves the maps unloaded. Because the return value is
ignored here, execution proceeds to maps__for_each_map() which takes a read
lock. When it attempts to lazy-load the unloaded maps, it will call
maps__mutate_mapping() which attempts to acquire the write lock, triggering
the deadlock.

Is there also a race window between these two calls?

If a concurrent thread inserts a new map after maps__load_maps() releases
its lock, but before or during maps__for_each_map(), the new unloaded map
will be iterated over under the read lock.

Would this again trigger the same self-deadlock when lazy loading calls
maps__mutate_mapping()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260604172850.683329-1-irogers@google.com?part=1