Re: [PATCH v2 03/25] perf sample: Add evsel to struct perf_sample

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Uninitialized memory usage for `sample.evsel` in `powerpc_vpadtl_sample` leads to a fatal assertion or crash during trace delivery.
--

--- Patch [3]: [PATCH v2 03/25] perf sample: Add evsel to struct perf_sample ---
commit b98d97d18e2ec81b0bc54af759fddbd9c49c126f
Author: Ian Rogers <[email protected]>

perf sample: Add evsel to struct perf_sample

This commit adds evsel to struct perf_sample so that the sample can
carry its associated evsel. This avoids the need to pass evsel and
perf_sample as a pair through various functions.

> diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
> index c48e840da7d48..3794d3a04afb3 100644
> --- a/tools/perf/util/session.c
> +++ b/tools/perf/util/session.c
[ ... ]
> @@ -1451,8 +1456,12 @@ static int machines__deliver_event(struct machines *machines,
>  
>  	dump_event(evlist, event, file_offset, sample, file_path);
>  
> -	evsel = evlist__id2evsel(evlist, sample->id);
> +	if (!sample->evsel)
> +		sample->evsel = evlist__id2evsel(evlist, sample->id);
> +	else
> +		assert(sample->evsel == evlist__id2evsel(evlist, sample->id));

[Severity: High]
Could this assert trigger a crash due to an uninitialized evsel field?

Looking at powerpc_vpadtl_sample() in tools/perf/util/powerpc-vpadtl.c, it
allocates struct perf_sample on the stack without zero-initializing it:

	struct perf_sample sample;
	union perf_event event;

	sample.ip = be64_to_cpu(record->srr0);
	sample.period = 1;
	sample.cpu = cpu;
	sample.id = vpa->sample_id;

Since it doesn't call perf_sample__init() or memset() on the sample, the
newly added evsel field will contain stack garbage.

When this sample is delivered via perf_session__deliver_synth_event(), it
eventually reaches machines__deliver_event(). Because the uninitialized
sample->evsel pointer is likely non-NULL, won't it take the else branch, fail
the assertion, and crash the perf tool?

> +	evsel = sample->evsel;
>  	machine = machines__find_for_cpumode(machines, event, sample);

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