Re: [PATCH v3 3/4] perf inject/aslr: Add aslr tool to remap/obfuscate virtual addresses

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Breakpoint addresses (bp_addr) are not scrubbed when processing regular perf.data files or synthesizing for a pipe, defeating the ASLR protection.
- [High] The --aslr option defeats kernel ASLR obfuscation by deterministically encoding the original kernel start address into the emitted mmap.pgoff field.
- [Medium] The `skipn` function does not handle `EINTR` when reading from a pipe, which can cause spurious tool aborts.
--

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

perf inject/aslr: Add aslr tool to remap/obfuscate virtual addresses

This commit adds an --aslr option to perf inject to remap virtual addresses
in perf.data files. This prevents virtual addresses from leaking when
transferring perf data between machines, thus weakening ASLR. Unhandled
events are dropped to conservatively avoid leaking ASLR information.

> diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
> index f174bc69cec4..6e6bf6b67956 100644
> --- a/tools/perf/builtin-inject.c
> +++ b/tools/perf/builtin-inject.c
[ ... ]
> @@ -2763,12 +2777,12 @@ int cmd_inject(int argc, const char **argv)
>  		/*
>  		 * If the input is already a pipe then the features and
>  		 * attributes don't need synthesizing, they will be present in
>  		 * the input.
>  		 */
>  		if (!data.is_pipe) {
>  			ret = perf_event__synthesize_for_pipe(&inject.tool,
>  							      inject.session,
>  							      &inject.output,
>  							      perf_event__repipe);

Does this bypass the ASLR tool wrapper for ATTR events and leave breakpoint
addresses unscrubbed?

The aslr_tool__process_attr() function zeros out bp_addr, but this callback
only intercepts ATTR events in the main event stream. If the output is a pipe,
perf_event__synthesize_for_pipe() uses perf_event__repipe directly, bypassing
the scrubbing and exposing the original bp_addr in the output stream.

When processing regular input files, ATTR configurations are parsed from the
file header and stored in session->evlist. Does perf_session__inject_header()
write this un-scrubbed evlist directly into the output file's header, also
leaking the original bp_addr?

[ ... ]
> diff --git a/tools/perf/util/aslr.c b/tools/perf/util/aslr.c
> new file mode 100644
> index 000000000000..32548352e1e5
> --- /dev/null
> +++ b/tools/perf/util/aslr.c
[ ... ]
> +static int skipn(int fd, u64 n)
> +{
> +	char buf[4096];
> +	ssize_t ret;
> +
> +	while (n > 0) {
> +		ret = read(fd, buf, (n < (u64)sizeof(buf) ? n : (u64)sizeof(buf)));
> +		if (ret <= 0)
> +			return ret;
> +		n -= ret;
> +	}
> +
> +	return 0;
> +}

Can this cause spurious tool aborts if read() is interrupted by a signal
(like SIGWINCH)?

This function explicitly checks if ret <= 0 and returns it without checking
for EINTR, which allows the error to propagate back to the event processing
loop and prematurely abort the perf inject session.

Should this utilize readn() or preadn() from the perf library which correctly
handle EINTR?

[ ... ]
> +static int aslr_tool__process_mmap(const struct perf_tool *tool,
> +				union perf_event *event,
> +				struct perf_sample *sample,
> +				struct machine *machine)
> +{
[ ... ]
> +	/* Remaps the mmap.start. */
> +	new_event->mmap.start = aslr_tool__remap_mapping(aslr, thread, cpumode,
> +							   event->mmap.start,
> +							   event->mmap.len,
> +							   event->mmap.pgoff);
> +	if (aslr->first_kernel_mapping && cpumode == PERF_RECORD_MISC_KERNEL) {
> +		/*
> +		 * If this is the first kernel image, we need to adjust the pgoff by a
> +		 * similar delta.
> +		 */
> +		new_event->mmap.pgoff = event->mmap.pgoff - event->mmap.start +
> +					new_event->mmap.start;

Does this adjustment of pgoff allow recovering the original kernel ASLR base?

Since event->mmap.pgoff is typically a known public constant for the first
kernel image mapping (e.g., 0 or a fixed ELF offset), and the output trace
exposes both new_event->mmap.start and new_event->mmap.pgoff, could an
attacker trivially reverse the equation:

old_start = new_start - new_pgoff + old_pgoff

Does this mathematical leak nullify the ASLR obfuscation intended for the
kernel image?

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