Re: [PATCH 6.6.y] Revert "perf unwind-libdw: Fix invalid reference counts"

[sashiko-bot@kernel.org]

Sashiko AI review found 1 potential issue(s):
- [Medium] Reverting a valid bug fix to resolve a backport build failure reintroduces a use-after-free vulnerability in perf's libdw unwinder.
--

commit 476f57f8f24801b0104204e756b3e024069b04e7
Author: Kenta Akagi <k@mgml.me>

Revert "perf unwind-libdw: Fix invalid reference counts"

This commit reverts a backport to resolve a build failure in v6.6.y caused
by the absence of map_symbol__exit().

> diff --git a/tools/perf/util/unwind-libdw.c b/tools/perf/util/unwind-libdw.c
> --- a/tools/perf/util/unwind-libdw.c
> +++ b/tools/perf/util/unwind-libdw.c
> @@ -133,8 +133,8 @@ static int entry(u64 ip, struct unwind_info *ui)
>  	}
>  
>  	e->ip	  = ip;
> -	e->ms.maps = maps__get(al.maps);
> -	e->ms.map = map__get(al.map);
> +	e->ms.maps = al.maps;
> +	e->ms.map = al.map;
>  	e->ms.sym = al.sym;
>  
>  	pr_debug("unwind: %s:ip = 0x%" PRIx64 " (0x%" PRIx64 ")\n",

Does removing these reference increments reintroduce a use-after-free
regression?

Directly after these assignments, entry() calls addr_location__exit():

tools/perf/util/unwind-libdw.c:entry() {
        ...
        addr_location__exit(&al);
        return 0;
}

This drops the references held by al. If these were the last references,
the map and maps objects will be freed. 

The e->ms.maps and e->ms.map pointers are later accessed in
unwind__get_entries() when it invokes the callback, potentially using
memory that has already been freed:

tools/perf/util/unwind-libdw.c:unwind__get_entries() {
        ...
        err = ui->entries[j].ip ? ui->cb(&ui->entries[j], ui->arg) : 0;
        ...
}

[ ... ]

> @@ -319,9 +319,6 @@ int unwind__get_entries(unwind_entry_cb_t cb, void *arg,
>  	if (err)
>  		pr_debug("unwind: failed with '%s'\n", dwfl_errmsg(-1));
>  
> -	for (i = 0; i < ui->idx; i++)
> -		map_symbol__exit(&ui->entries[i].ms);
> -
>  	dwfl_end(ui->dwfl);
>  	free(ui);
>  	return 0;

Instead of reverting the fix entirely because map_symbol__exit() is absent in
v6.6.y, would it be safer to retain the map__get() and maps__get() calls in
entry() and replace this loop with manual decrements using map__zput() and
maps__zput() here?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260420163222.23517-1-k@mgml.me?part=1