Re: [PATCH v3 4/6] perf probe-finder: Fix libdw API contract violations

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Invalid type formatting for zero-sized structures due to incorrect `dwarf_bytesize` validation.
- [High] Silent failure to resolve line ranges for functions missing the `DW_AT_decl_line` attribute.
- [High] NULL pointer dereference when processing array element access for variables located in registers.
--

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

perf probe-finder: Fix libdw API contract violations

This commit adds checks for return values from various libdw API calls
to prevent crashes and API contract violations. It also fixes some C90
mixed declarations.

> diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
> index 64328abeef8b..bdef340dfd55 100644
> --- a/tools/perf/util/probe-finder.c
> +++ b/tools/perf/util/probe-finder.c

[ ... ]

> @@ -291,7 +296,7 @@ static int convert_variable_type(Dwarf_Die *vr_die,
>  			 probe_type_is_available(PROBE_TYPE_X) ? 'x' : 'u';
>  
>  	ret = dwarf_bytesize(&type);
> -	if (ret <= 0)
> +	if (ret < 0)
>  		/* No size ... try to use default type */
>  		return 0;

When processing a zero-sized type (such as an empty struct), dwarf_bytesize
returns 0. With this change, a size of 0 falls through to BYTES_TO_BITS(0)
and formats the type as u0, s0, or x0.

Does the kprobe-events parser support 0-bit basic types, or will it reject
the probe injection?

[ ... ]

> @@ -357,7 +362,13 @@ static int convert_variable_fields(Dwarf_Die *vr_die, const char *varname,
>  			else
>  				*ref_ptr = ref;
>  		}
> -		ref->offset += dwarf_bytesize(&type) * field->index;
> +		{
> +			int bsize = dwarf_bytesize(&type);
> +
> +			if (bsize < 0)
> +				return -EINVAL;
> +			ref->offset += bsize * field->index;
> +		}

This wasn't introduced by this patch, but if an array variable is located in
a register (like DW_OP_reg0), convert_variable_location leaves tvar->ref as
NULL.

Because there is no check to see if ref is NULL before calculating the array
index access offset here, will this result in a NULL pointer dereference and
crash the perf tool?

[ ... ]

> @@ -1794,7 +1819,8 @@ static int line_range_search_cb(Dwarf_Die *sp_die, void *data)
>  
>  	if (die_match_name(sp_die, lr->function) && die_is_func_def(sp_die)) {
>  		lf->fname = die_get_decl_file(sp_die);
> -		dwarf_decl_line(sp_die, &lr->offset);
> +		if (dwarf_decl_line(sp_die, &lr->offset) != 0)
> +			return DWARF_CB_OK; // Skip if no line info

The DW_AT_decl_line attribute is not strictly mandatory and can be omitted
by compilers for generated functions.

By returning DWARF_CB_OK when dwarf_decl_line fails, will perf probe -L
silently skip these functions and display no lines, whereas previously it
left the offset at 0 and successfully mapped the line range relative to 0?

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