Re: [PATCH v2 7/8] perf annotate-data: Resolve type DIEs in the debug file they came from

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Unbounded recursion and iteration in DWARF member type chasers (`die_find_member` and `die_get_member_type`).
--

commit 2cb5592a12267288ad7685ab9363967d18e8063c
Author: Arnaldo Carvalho de Melo <acme@redhat.com>
Subject: perf annotate-data: Resolve type DIEs in the debug file they came from

This commit resolves type DIE offsets in their correct debug file to prevent
an infinite loop in die_get_pointer_type(). It introduces bounded recursion
and iteration (MAX_TYPE_CHASE, MAX_MEMBER_DEPTH) to safely handle corrupted
or cyclic DWARF structures.

> diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
> index d7160f87ac7d7..e35f724ea6835 100644
> --- a/tools/perf/util/dwarf-aux.c
> +++ b/tools/perf/util/dwarf-aux.c
> @@ -266,16 +266,35 @@ Dwarf_Die *die_get_type(Dwarf_Die *vr_die, Dwarf_Die *die_mem)
>  		return NULL;
>  }
>  
> +/*
> + * The chases below cross typedefs and qualifiers to get to the type that
> + * is actually meant, and a DIE that is not what it looks like, e.g. one
> + * parsed at an offset that is not the start of a DIE in the file it was
> + * resolved in, can have a DW_AT_type that refers back to itself, which
> + * makes them spin forever: 'perf report -s type' did exactly that on the
> + * dwz compressed debug info of zlib-ng (libz.so.1), burning all of a CPU
> + * with no output while resolving a hist entry in build_tree().
> + *
> + * No sane chain is this long, so give up instead of hanging, telling about
> + * it so that the broken debug info can be looked at.
> + */
> +#define MAX_TYPE_CHASE 32
> +
>  /* Get a type die, but skip qualifiers */
>  Dwarf_Die *__die_get_real_type(Dwarf_Die *vr_die, Dwarf_Die *die_mem)

[Severity: High]
This is a pre-existing issue, but since we are bounding DWARF chases here
to prevent hangs on cyclic structures, should die_get_member_type() also
have similar limits?

When processing broken DWARF info, if a nested structure's real type resolves 
back to itself without advancing the offset, this loop in die_get_member_type() 
can spin infinitely:

tools/perf/util/dwarf-aux.c:die_get_member_type() {
    ...
    while (tag == DW_TAG_structure_type || tag == DW_TAG_union_type) {
        member = die_find_child(&mb_type, __die_find_member_offset_cb,
                                (void *)(long)offset, die_mem);
    ...
}

Similarly, does __die_find_member_cb() require a recursion bound?

If it encounters a self-referential unnamed structure, it can perform
unbounded recursion through die_find_member(), potentially leading to a
stack overflow:

tools/perf/util/dwarf-aux.c:__die_find_member_cb() {
    ...
    else if (!dwarf_diename(die_mem)) {
        Dwarf_Die type_die, tmp_die;
        if (die_get_type(die_mem, &type_die) &&
            die_find_member(&type_die, name, &tmp_die))
            return DIE_FIND_CB_END;
    ...
}

Could these unbound paths cause perf report to hang or crash when processing 
untrusted or corrupted perf.data profiles?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260913032632.116277-1-acme@kernel.org?part=7