Re: [PATCH v2 3/4] perf script: Add --lazy-load-symbols for lazy symbol loading

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] perf: Use-After-Free of dso->symbol_names during concurrent lazy materialization
- [Medium] perf: missing memory barrier in dso__ondemand check leads to rbtree corruption
--

commit 2498c78d5ec3921571fc9c4a22667912c81f3010
Author: Alireza Haghdoost <haghdoost@uber.com>

perf script: Add --lazy-load-symbols for lazy symbol loading

This commit adds a lazy-load-symbols option to perf script to resolve symbols
lazily instead of loading full symtabs. It builds a compact sorted index and
materializes symbols that appear in samples, which reduces memory footprint
when profiling large binaries.

> diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
> --- a/tools/perf/util/dso.c
> +++ b/tools/perf/util/dso.c
[ ... ]
> +void dso__reset_symbol_names(struct dso *dso)
> +{
> +	zfree(&RC_CHK_ACCESS(dso)->symbol_names);

[Severity: High]
Can this zfree() lead to a use-after-free during concurrent lookups?

If dso__materialize_symbols_ondemand() returns early due to max-symbol-bytes
budget limits during a concurrent lookup by name in map__find_symbol_by_name_idx(),
the dso__ondemand pointer is left non-NULL. The thread then calls dso__sort_by_name(),
constructing the dso->symbol_names array, and begins a lockless binary search on it.

Concurrently, a lookup by address in another thread evaluates dso__ondemand,
takes the lock, calls dso__find_symbol_ondemand(), and successfully materializes
a different symbol. This triggers dso__reset_symbol_names(), which immediately frees
dso->symbol_names via zfree() while the first thread is actively reading it.

> +	RC_CHK_ACCESS(dso)->symbol_names_len = 0;
> +	RC_CHK_ACCESS(dso)->sorted_by_name = false;
> +}
> +
> +void dso__free_ondemand(struct dso *dso)
> +{
> +	struct dso_ondemand *od = RC_CHK_ACCESS(dso)->ondemand;
> +
> +	if (!od)
> +		return;
> +	RC_CHK_ACCESS(dso)->ondemand = NULL;

[Severity: Medium]
Does this missing release barrier risk exposing partially written rbtree nodes?

A thread evaluating dso__ondemand locklessly in map__find_symbol() could
read the state as NULL, bypassing the dso__lock acquisition, and proceed to
traverse the rbtree. Another thread executing dso__free_ondemand() stores NULL
into dso->ondemand as a plain write.

Without smp_store_release() on the writer and smp_load_acquire() on the
reader, a weakly-ordered CPU can reorder the reader's loads such that it
reads the rbtree nodes before ondemand is loaded, observing stale memory.

> +	free(od->sorted);
> +	symbol__unaccount_bytes(od->nr_alloc * sizeof(*od->sorted));
> +	dso__data_close(od->data_dso);
> +	dso__put(od->data_dso);
> +	free(od);
> +}

[ ... ]
> diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
> --- a/tools/perf/util/map.c
> +++ b/tools/perf/util/map.c
[ ... ]
>  struct symbol *map__find_symbol(struct map *map, u64 addr)
>  {
> +	struct dso *dso;
> +	struct symbol *sym;
> +
>  	if (map__load(map) < 0)
>  		return NULL;
>  
> -	return dso__find_symbol(map__dso(map), addr);
> +	dso = map__dso(map);
> +	if (dso__ondemand(dso)) {

[Severity: Medium]
This is the lockless read mentioned above. Is an smp_load_acquire() needed
here to pair with a release barrier in dso__free_ondemand()?

> +		/*
> +		 * On-demand lookup may materialize and insert a symbol.  Keep
> +		 * both the lookup and insertion under the DSO lock so another
> +		 * thread cannot traverse or modify the rb-tree concurrently.
> +		 */
> +		mutex_lock(dso__lock(dso));
> +		sym = dso__find_symbol(dso, addr);
> +		if (!sym)
> +			sym = dso__find_symbol_ondemand(dso, addr);
> +		mutex_unlock(dso__lock(dso));
> +	} else {
> +		sym = dso__find_symbol(dso, addr);
> +	}
> +	return sym;
>  }
>  
>  struct symbol *map__find_symbol_by_name_idx(struct map *map, const char *name, size_t *idx)
> @@ -396,6 +414,16 @@ struct symbol *map__find_symbol_by_name_idx(struct map *map, const char *name, s
>  		return NULL;
>  
>  	dso = map__dso(map);
> +	if (dso__ondemand(dso)) {
> +		mutex_lock(dso__lock(dso));
> +		/*
> +		 * Name lookup requires a complete name-sorted array. Preserve
> +		 * that API by materializing the remaining address index first.
> +		 * An explicit symbol-byte limit can leave a partial set.
> +		 */
> +		dso__materialize_symbols_ondemand(dso);
> +		mutex_unlock(dso__lock(dso));
> +	}
>  	dso__sort_by_name(dso);
>  
>  	return dso__find_symbol_by_name(dso, name, idx);

[Severity: High]
This is where the concurrent lockless binary search over dso->symbol_names
happens, which may race with the zfree() in dso__reset_symbol_names().

[ ... ]
> diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
> --- a/tools/perf/util/symbol-elf.c
> +++ b/tools/perf/util/symbol-elf.c
[ ... ]
> +static struct symbol *dso__materialize_symbol_ondemand(struct dso *dso, u32 pos)
> +	EXCLUSIVE_LOCKS_REQUIRED(dso__lock(dso))
> +{
[ ... ]
> +	s = symbol__new_bounded(idx->start, idx->end - idx->start,
> +				idx->binding, idx->type, name, &budget_exceeded);
> +	free(demangled);
> +	free(name_heap);
> +	if (!s && budget_exceeded)
> +		symbol_budget_warning();
> +	if (s) {
> +		if (idx->flags & SYM_IDX_FLAG_IFUNC_ALIAS)
> +			symbol__set_ifunc_alias(s, true);
> +		dso__reset_symbol_names(dso);

[Severity: High]
This is the call to dso__reset_symbol_names() that triggers the use-after-free
if a concurrent thread is searching the array locklessly in
map__find_symbol_by_name_idx().

> +		__symbols__insert(dso__symbols(dso), s);
> +		idx->flags |= SYM_IDX_FLAG_MATERIALIZED;
> +	}
> +	return s;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260919-perf-symbol-memory-send-v2-0-495b8f00ad7c@uber.com?part=3