Re: [PATCH 2/4] perf dwarf-aux: Add die_has_flex_array() helper

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] die_has_flex_array() uses a flawed heuristic (`loc == size`) to identify flexible arrays, which falsely rejects standard C structs with trailing alignment padding.
- [Medium] Nested structures bypass the bounds and padding validation for flexible arrays, creating inconsistent logic compared to direct array members.
- [Medium] The function fails to detect flexible arrays that are embedded inside a trailing anonymous union.
- [Medium] Infinite recursion in die_has_flex_array() when parsing maliciously crafted cyclic DWARF structures, leading to stack exhaustion.
--

commit 8b9b8180d24f8e4da056fe22e70f6b84446a7257
Author: Namhyung Kim <namhyung@kernel.org>

perf dwarf-aux: Add die_has_flex_array() helper

The die_has_flex_array() returns true when the given type is a struct
and contains an array at the end of the struct.

> diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
> index d7160f87ac7d7..98d018798fa75 100644
> --- a/tools/perf/util/dwarf-aux.c
> +++ b/tools/perf/util/dwarf-aux.c
[ ... ]
> +bool die_has_flex_array(Dwarf_Die *parent_die)
> +{
> +	Dwarf_Die die_mem, type_die;
> +	Dwarf_Word size, loc;
> +
> +	if (dwarf_tag(parent_die) != DW_TAG_structure_type)
> +		return false;

[Severity: Medium]
Does this rigid check against DW_TAG_structure_type fail to detect valid
flexible arrays embedded inside trailing anonymous unions, which are frequently
used in the kernel?

[ ... ]
> +	if (dwarf_tag(&type_die) == DW_TAG_array_type)
> +		return die_get_data_member_location(&die_mem, &loc) == 0 && loc == size;

[Severity: High]
Does this strict loc == size equality accurately detect flexible arrays when
standard C structs contain trailing alignment padding?

When preceding members do not perfectly align with the struct's maximum
alignment, the compiler pads the total struct size. For example, if a struct
has an 8-byte long, a 4-byte int, and a trailing flexible array, its total
size is padded to 16 bytes. However, the data member location (loc) of the
flexible array is 12.

Could this cause perf annotate to falsely reject legitimate samples targeting
flexible array members in padded structs by evaluating as PERF_TMR_BAD_OFFSET?

> +
> +	if (dwarf_tag(&type_die) == DW_TAG_structure_type)
> +		return die_has_flex_array(&type_die);

[Severity: Medium]
Does this recursive call bypass bounds and padding validation for flexible
arrays nested inside structures, making the logic inconsistent compared to
direct array members?

If the last member is a nested struct with no padding, it returns true,
completely ignoring any padding that may exist between the nested struct and
the end of the parent struct.

[Severity: Medium]
Is there a risk of infinite recursion and stack exhaustion when parsing
maliciously crafted cyclic DWARF structures?

If an untrusted DWARF file defines a struct A whose last member is struct B,
and struct B's last member is struct A, this recursion could overflow the
stack since it lacks depth limits or cycle tracking.

> +
> +	return false;

[Severity: Medium]
Is this fall-through where DW_TAG_union_type is unhandled, dropping support
for flexible arrays wrapped in trailing unions?

> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260912054706.1475583-1-namhyung@kernel.org?part=2