Re: [PATCH v2 4/4] perf annotate-data: Adjust type offset for flex-array

[Namhyung Kim <namhyung@kernel.org>]

On Mon, Sep 14, 2026 at 07:01:04AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> - [Medium] Off-by-one error and tail padding overlap in flex-array offset adjustment causes accesses to be silently dropped and incorrectly attributed.
> - [Medium] Calling `strstr()` on `child->type_name` without checking for NULL causes a segmentation fault if memory allocation failed during type name extraction.
> - [Medium] The newly added `strstr(child->type_name, "[]")` call reads uninitialized heap memory, leading to out-of-bounds reads and potential crashes when analyzing malformed or unknown DWARF entries.
> --
> 
> commit 546415e2740f53443f133b3600542636f3d84f31
> Author: Namhyung Kim <namhyung@kernel.org>
> perf annotate-data: Adjust type offset for flex-array
> 
> This commit adjusts the type offset for flexible array members that are located
> beyond the original type size. It ensures a field name is found for flex arrays
> by adjusting the offset in an array to find a corresponding element.
> 
> > diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c
> > index e8aff6a916c0a..a66b72feb6e2f 100644
> > --- a/tools/perf/util/annotate-data.c
> > +++ b/tools/perf/util/annotate-data.c
> 
> [ ... ]
> 
> > @@ -355,15 +376,37 @@ static int fill_member_name(char *buf, size_t sz, struct annotated_member *m,
> >  			len = 0;
> >  		}
> >  
> > -		return fill_member_name(buf + len, sz - len, child, offset, first) + len;
> > +		return fill_member_name(buf + len, sz - len, child, offset, first,
> > +					has_flex_array) + len;
> >  	}
> > +
> >  	return 0;
> >  }
> >  
> >  int annotated_data_type__get_member_name(struct annotated_data_type *adt,
> >  					 char *buf, size_t sz, int member_offset)
> >  {
> > -	return fill_member_name(buf, sz, &adt->self, member_offset, /*first=*/true);
> > +	return fill_member_name(buf, sz, &adt->self, member_offset, /*first=*/true,
> > +				adt->flex_array);
> > +}
> > +
> > +static struct annotated_member *find_flex_array(struct annotated_member *m)
> > +{
> > +	struct annotated_member *child;
> > +
> > +	if (list_empty(&m->children))
> > +		return NULL;
> > +
> > +	child = list_last_entry(&m->children, struct annotated_member, node);
> > +	if (strstr(child->type_name, "[]"))
> 
> [Severity: Medium]
> Could this call to strstr() safely handle scenarios where child->type_name is
> NULL or uninitialized?
> 
> During DWARF parsing, for instance in __add_member_cb(), if die_get_typename()
> fails due to a malformed DWARF entry or memory exhaustion, strbuf_detach()
> might return an uninitialized heap buffer or NULL. If that happens, does this
> scan risk triggering a segmentation fault or an out-of-bounds read when looking
> for the "[]" string?

Ok, will check.

> 
> > +		return child;
> > +
> > +	return find_flex_array(child);
> > +}
> 
> [ ... ]
> 
> > @@ -1740,6 +1783,7 @@ struct annotated_data_type *find_data_type(struct data_loc_info *dloc)
> >  {
> >  	struct dso *dso = map__dso(dloc->ms->map);
> >  	Dwarf_Die type_die;
> > +	struct annotated_data_type *result;
> >  
> >  	/*
> >  	 * The type offset is the same as instruction offset by default.
> > @@ -1752,7 +1796,25 @@ struct annotated_data_type *find_data_type(struct data_loc_info *dloc)
> >  	if (find_data_type_die(dloc, &type_die) < 0)
> >  		return NULL;
> >  
> > -	return dso__findnew_data_type(dso, &type_die);
> > +	result = dso__findnew_data_type(dso, &type_die);
> > +	if (result == NULL)
> > +		return NULL;
> > +
> > +	if (result->flex_array && dloc->type_offset > result->self.size) {
> 
> [Severity: Medium]
> Will this boundary check correctly catch accesses that fall exactly on the
> boundary or within the struct's tail padding?
> 
> If an event samples a flexible array element whose offset is greater than 
> flex_array->offset but less than or equal to result->self.size (the total
> padded size of the struct), it appears this condition evaluates to false.
> Could this cause the offset adjustment to be skipped, causing valid sample
> counts to be silently dropped during data presentation?

I think it's ok.  If it's less than or equal to result->self.size, it'd
have high chances to fall into the flex array element so I think they
will be processed properly without adjusting the offset.

Basically this adjustment is to improve visualization of the member
name.  It won't to drop samples anyway.

Thanks,
Namhyung

> 
> > +		struct annotated_member *flex_array = get_flex_array_member(result);
> > +
> > +		if (flex_array && flex_array->size > 0) {
> > +			int offset = dloc->type_offset;
> > +
> > +			/* adjust offset in the flex array */
> > +			offset -= flex_array->offset;
> > +			offset %= flex_array->size;
> > +			offset += flex_array->offset;
> > +
> > +			dloc->type_offset = offset;
> > +		}
> > +	}
> > +	return result;
> >  }
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260914064535.1671939-1-namhyung@kernel.org?part=4