Re: [PATCH v11 13/33] perf inject: Don't pass evsel with sample

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

Pre-existing issues:
- [High] In `perf_inject__sched_stat`, a mismatch between the `sample_type` of the `sched:sched_switch` event and the `sched:sched_stat_*` event causes a heap out-of-bounds write.
--

--- Patch [13]: [PATCH v11 13/33] perf inject: Don't pass evsel with sample ---
commit 5a323f56faef1c75c44630fb2ace9d44a71d43aa
Author: Ian Rogers <[email protected]>

perf inject: Don't pass evsel with sample

This commit removes the explicit evsel parameter from several functions since
it is already contained within the sample structure. This cleans up the
handling of sample and mmap_evsel when synthesizing build id events and mmap2
events with build ids.

> diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
> --- a/tools/perf/builtin-inject.c
> +++ b/tools/perf/builtin-inject.c

[ ... ]

> @@ -1049,13 +1053,12 @@ static int perf_inject__sched_process_exit(const struct perf_tool *tool,
>  static int perf_inject__sched_switch(const struct perf_tool *tool,
>  				     union perf_event *event,
>  				     struct perf_sample *sample,
> -				     struct evsel *evsel,
>  				     struct machine *machine)
>  {
>  	struct perf_inject *inject = container_of(tool, struct perf_inject, tool);
>  	struct event_entry *ent;
>  
> -	perf_inject__sched_process_exit(tool, event, sample, evsel, machine);
> +	perf_inject__sched_process_exit(tool, event, sample, machine);
>  
>  	ent = malloc(event->header.size + sizeof(struct event_entry));

[ ... ]

> @@ -1074,13 +1077,13 @@ static int perf_inject__sched_switch(const struct perf_tool *tool,
>  static int perf_inject__sched_stat(const struct perf_tool *tool,
>  				   union perf_event *event __maybe_unused,
>  				   struct perf_sample *sample,
> -				   struct evsel *evsel,
>  				   struct machine *machine)
>  {
>  	struct event_entry *ent;
>  	union perf_event *event_sw;
>  	struct perf_sample sample_sw;
>  	struct perf_inject *inject = container_of(tool, struct perf_inject, tool);
> +	struct evsel *evsel = sample->evsel;

[Severity: High]
This is a pre-existing issue, but can this allocation and synthesis mismatch 
result in a heap out-of-bounds write?

When perf_inject__sched_switch() allocates the ent buffer, it sizes it
precisely for the sched_switch event:

ent = malloc(event->header.size + sizeof(struct event_entry));

However, when perf_inject__sched_stat() later repopulates this buffer, it calls
perf_event__synthesize_sample() using the sample_type bitmask of the sched_stat
event:

perf_event__synthesize_sample(event_sw, evsel->core.attr.sample_type,
                              evsel->core.attr.read_format, &sample_sw);

If the sched_stat event was configured with a larger sample_type payload than
the sched_switch event (such as callchains or raw data), could
perf_event__synthesize_sample() write sequentially out-of-bounds and
overflow the ent buffer?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=13