Re: [PATCH v10 3/5] perf inject/aslr: Implement sample address remapping

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] aslr_tool__process_sample disables needs_swap when parsing new_event, causing 32-bit sample fields (like PID and TID) to be swapped during cross-endian analysis, which silently breaks downstream sample processing like build ID injection.
--

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

perf inject/aslr: Implement sample address remapping

This patch introduces the logic for aslr_tool__process_sample to parse sample
events and remap addresses using mapping metadata. It filters out specific
sample types to prevent ASLR leaks and delegates the modified sample to the
next tool.

> diff --git a/tools/perf/util/aslr.c b/tools/perf/util/aslr.c
> index 375a0355f2812..8cfefa23030d2 100644
> --- a/tools/perf/util/aslr.c
> +++ b/tools/perf/util/aslr.c

[ ... ]

> @@ -554,13 +608,405 @@ static int aslr_tool__process_sample(const struct perf_tool *tool,

[ ... ]

> +	new_event->sample.header.size = sizeof(struct perf_event_header) + j * sizeof(u64);
> +
> +	orig_needs_swap = evsel->needs_swap;
>  
> -	return delegate->sample(delegate, event, sample, machine);
> +	perf_sample__init(&new_sample, /*all=*/ true);
> +	evsel->needs_swap = false;
> +	ret = evsel__parse_sample(evsel, new_event, &new_sample);

[Severity: High]
Does disabling evsel->needs_swap here break 32-bit fields when processing
cross-endian perf.data files?

When processing a cross-endian perf.data file, the core perf session loop
blindly 64-bit swaps the sample array before invoking the tool callback. For
fields containing packed 32-bit values (like PERF_SAMPLE_TID containing pid
and tid), the 64-bit swap reverses their order in memory.

evsel__parse_sample() normally corrects this by undoing the 64-bit swap and
reswapping the 32-bit words if evsel->needs_swap is true. By explicitly
setting it to false in aslr_tool__process_sample(), we bypass this 32-bit
unswapping logic. Could this cause new_sample.pid to receive the TID and
new_sample.tid to receive the PID (and similarly for PERF_SAMPLE_CPU)?

If these fields are swapped, wouldn't this silently break downstream sample
processing, such as build ID injection failing to find the correct thread
because the PID is completely invalid?

> +	evsel->needs_swap = orig_needs_swap;
> +
> +	if (ret) {
> +		perf_sample__exit(&new_sample);
> +		goto out_put;
> +	}
> +
> +	new_sample.evsel = evsel;
> +	ret = delegate->sample(delegate, new_event, &new_sample, machine);
> +	perf_sample__exit(&new_sample);
> +
> +out_put:
> +	thread__put(thread);
> +	return ret;
>  }

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