Re: [PATCH v1] perf dwarf-aux: Fix libdw segmentation fault in cu_walk_functions_at

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] AI-generated patch introduces unnecessary performance penalty based on hallucinated API contracts and prior state.
- [Medium] The patch introduces a guaranteed segmentation fault by unconditionally dereferencing uninitialized memory via the newly added `dwarf_dieoffset` call.
--

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

perf dwarf-aux: Fix libdw segmentation fault in cu_walk_functions_at

This commit modifies DWARF processing in perf kmem to prevent alleged
segmentation faults. It reconstructs Dwarf_Die structures using offsets
to avoid dangling pointers and rewrites a loop in cu_walk_functions_at
to address an uninitialized memory read.

> 2. Uninitialized memory access in cu_walk_functions_at:
> A logic bug in the for loop of cu_walk_functions_at attempted to
> avoid in-place modifications by using a separate next_die buffer.
> However, it performed a memcpy(&die_mem, &next_die) at the end of the
> loop body *before* next_die was actually initialized by

Is this description accurate?

Looking at the original code in the diff, there was no next_die variable
at all before this commit. The original loop passed &die_mem directly to
die_find_child.

> 1. Dangling pointers from dwarf_getfuncs:
> dwarf_getfuncs passes temporary Dwarf_Die structures to its
> callback. Copying these via memcpy leads to dangling internal
> pointers (such as to Dwarf_Abbrev structures) once dwarf_getfuncs
> returns and cleans up its temporary state.

Does libdw actually invalidate these pointers when dwarf_getfuncs returns?

libdw Dwarf_CU pointers typically remain valid until the Dwarf session is
closed, so the copied Dwarf_Die should not have dangling pointers.

Reconstructing it via dwarf_offdie appears to add binary search overhead
without changing the underlying pointer.

> diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
> index 92db2fccc788..52fdf6d49d3b 100644
> --- a/tools/perf/util/dwarf-aux.c
> +++ b/tools/perf/util/dwarf-aux.c

[ ... ]

> @@ -661,8 +670,11 @@ Dwarf_Die *die_find_realfunc(Dwarf_Die *cu_die, Dwarf_Addr addr,
>  	/* dwarf_getscopes can't find subprogram. */
>  	if (!dwarf_getfuncs(cu_die, __die_search_func_cb, &ad, 0))
>  		return NULL;
> -	else
> -		return die_mem;
> +
> +	if (dwarf_offdie(dwarf_cu_getdwarf(cu_die->cu), dwarf_dieoffset(die_mem), die_mem) == NULL)
> +		return NULL;
> +
> +	return die_mem;

Could this introduce a regression where it crashes if dwarf_getfuncs returns
an error?

dwarf_getfuncs returns -1 on error. If it returns -1, !dwarf_getfuncs
evaluates to false, causing execution to bypass the return NULL and
fall through to dwarf_offdie.

Since the callback wasn't executed on an error, the die_mem structure
remains uninitialized stack memory from the caller. Calling
dwarf_dieoffset(die_mem) will dereference this uninitialized memory and
likely cause a segmentation fault.

Should the error check be changed to if (dwarf_getfuncs(...) <= 0) to
safely abort on errors?

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