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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Direct access to fields of the reference-counted `struct thread` breaks the build when `REFCNT_CHECKING` is enabled.
- [High] Double-swapping of `PERF_SAMPLE_BRANCH_STACK` flags corrupts branch data when analyzing cross-endian perf.data files.
--

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

perf inject/aslr: Implement sample address remapping

This commit adds the sample address remapping logic to the ASLR tool. It
parses sample events and remaps IPs, ADDRs, callchains, and branch stacks
using the mappings collected from metadata events.

> diff --git a/tools/perf/util/aslr.c b/tools/perf/util/aslr.c
> index be7280f884308..fc619b9f1f40a 100644
> --- a/tools/perf/util/aslr.c
> +++ b/tools/perf/util/aslr.c
> @@ -109,6 +109,60 @@ static u64 round_up_to_page_size(u64 addr)
>  	return (addr + page_size - 1) & ~((u64)page_size - 1);
>  }
>  
> +static u64 aslr_tool__remap_address(struct aslr_tool *aslr,
> +				    struct thread *aslr_thread,
> +				    u8 cpumode,
> +				    u64 addr)
> +{
[ ... ]
> +		if (!thread__find_map(aslr_thread, effective_cpumode, addr, &al)) {
> +			addr_location__exit(&al);
> +			return 0; /* No mmap. */
> +		}
> +	}
> +
> +	key.machine = maps__machine(aslr_thread->maps);
> +	key.dso = map__dso(al.map);
> +	key.invariant = map__start(al.map) - map__pgoff(al.map);
> +	key.pid = (effective_cpumode == PERF_RECORD_MISC_KERNEL ||
> +		   effective_cpumode == PERF_RECORD_MISC_GUEST_KERNEL) ?
> +		  kernel_pid : aslr_thread->pid_;

[Severity: High]
Will this direct field access break the build when REFCNT_CHECKING is enabled?

When perf is built with REFCNT_CHECKING=1 for ASAN or LSAN builds, the thread
struct is wrapped in an opaque pointer, hiding the actual fields. Direct access
to maps and pid_ usually causes compilation failures under these builds. 
Should this use the proper thread__maps() and thread__pid() accessors instead?

> @@ -554,13 +608,399 @@ static int aslr_tool__process_sample(const struct perf_tool *tool,
>  				     struct perf_sample *sample,
>  				     struct machine *machine)
>  {
[ ... ]
> +	if (sample_type & PERF_SAMPLE_BRANCH_STACK) {
> +		u64 nr;
[ ... ]
> +		for (u64 cntr = 0; cntr < nr; cntr++) {
> +			out_array[j++] = aslr_tool__remap_address(aslr, thread,
> +								  sample->cpumode,
> +								  in_array[i++]); /* from */
> +			out_array[j++] = aslr_tool__remap_address(aslr, thread,
> +								  sample->cpumode,
> +								  in_array[i++]); /* to */
> +			out_array[j++] = in_array[i++]; /* flags */
> +		}
[ ... ]
> +	new_event->sample.header.size = sizeof(struct perf_event_header) + j * sizeof(u64);
> +
> +	perf_sample__init(&new_sample, /*all=*/ true);
> +	ret = evsel__parse_sample(evsel, new_event, &new_sample);

[Severity: High]
Does this second call to evsel__parse_sample() corrupt branch stack flags
when analyzing cross-endian perf.data files?

The initial event processing loop calls evsel__parse_sample() on the original
event, which mutates the event array in place by swapping the branch stack
flags. 

When the already-mutated, native-endian payload is copied into new_event here,
calling evsel__parse_sample() a second time will bitfield-swap the flags
again. Does this inadvertently restore the original foreign-endian layout and
feed corrupted branch flags to the downstream delegate tool?

> +	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/20260604172850.683329-1-irogers@google.com?part=3