Re: [PATCH v9 2/5] perf inject/aslr: Add ASLR tool infrastructure and MMAP tracking

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Breakpoint addresses and register masks leak when `--itrace` is combined with `--aslr` in pipe mode due to missing attribute zeroing.
- [High] Auxiliary trace events (`auxtrace`, `auxtrace_info`, `auxtrace_error`) are silently dropped when `--aslr` is used, breaking `--itrace` decoding.
--

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

perf inject/aslr: Add ASLR tool infrastructure and MMAP tracking
    
If perf.data files are taken from one machine to another they may
leak virtual addresses and so weaken ASLR on the machine they are
coming from. Add an aslr option for perf inject that remaps all
virtual addresses, or drops data/events, so that the virtual address
information isn't leaked.

> diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
> index d8cb1f562f690..a9f0a3901e7bc 100644
> --- a/tools/perf/builtin-inject.c
> +++ b/tools/perf/builtin-inject.c

[ ... ]

> @@ -276,6 +295,8 @@ static int perf_event__repipe_attr(const struct perf_tool *tool,
>  
>  	attr.size = sizeof(struct perf_event_attr);
>  	attr.sample_type &= ~PERF_SAMPLE_AUX;
> +	if (inject->aslr)
> +		attr.sample_type &= ASLR_SUPPORTED_SAMPLE_TYPE;

[Severity: High]
Does this alternative synthesis path for itrace also need to clear bp_addr,
sample_regs_user, and sample_regs_intr?

When perf inject is run in pipe mode with both the aslr and itrace options,
perf_event__repipe_attr takes this path. The non-itrace path correctly clears 
the breakpoint addresses and register masks for ASLR sanitization, but this 
path only masks the sample type.

Could this leak sensitive kernel virtual addresses encoded in breakpoint 
attributes into the output stream, bypassing the ASLR protections?

>  
>  	if (inject->itrace_synth_opts.add_last_branch) {
>  		attr.sample_type |= PERF_SAMPLE_BRANCH_STACK;

[ ... ]

> diff --git a/tools/perf/util/aslr.c b/tools/perf/util/aslr.c
> new file mode 100644
> index 0000000000000..be7280f884308
> --- /dev/null
> +++ b/tools/perf/util/aslr.c

[ ... ]

> +static s64 aslr_tool__process_auxtrace(const struct perf_tool *tool __maybe_unused,
> +				       struct perf_session *session,
> +				       union perf_event *event)
> +{
> +	if (perf_data__is_pipe(session->data)) {
> +		/* Copy behavior of the stub by reading all pipe data. */
> +		int err = skipn(perf_data__fd(session->data), event->auxtrace.size);
> +
> +		if (err < 0)
> +			return err;
> +	}
> +	return event->auxtrace.size;
> +}

[ ... ]

> +static void aslr_tool__init(struct aslr_tool *aslr, struct perf_tool *delegate)
> +{
> +	delegate_tool__init(&aslr->tool, delegate);
> +	aslr->tool.tool.ordered_events = true;

[ ... ]

> +	/*
> +	 * event_update, tracing_data, finished_round, build_id, id_index,
> +	 * auxtrace_info, auxtrace_error, time_conv, thread_map, cpu_map,
> +	 * stat_config, stat, feature, finished_init, bpf_metadata, compressed,
> +	 * auxtrace - no virtual addresses.
> +	 */
> +	aslr->tool.tool.auxtrace = aslr_tool__process_auxtrace;
> +	aslr->tool.tool.auxtrace_info = aslr_tool__process_auxtrace_info;
> +	aslr->tool.tool.auxtrace_error = aslr_tool__process_auxtrace_error;
> +}

[Severity: High]
Will overwriting the auxtrace delegate callbacks cause silent data loss?

The auxtrace, auxtrace_info, and auxtrace_error callbacks are overridden here 
with internal stubs. As seen in aslr_tool__process_auxtrace above, the stub 
consumes the pipe data but returns without forwarding the event to the 
delegate tool.

Does this mean raw hardware traces like Intel PT are silently dropped, 
preventing them from reaching perf_event__process_auxtrace to generate 
synthetic samples? This looks like it would break itrace decoding.

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