Re: [PATCH v18 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] Incorrect `pgoff` overwrite for kernel modules breaks symbol resolution.
- [Medium] KSYMBOL REGISTER and UNREGISTER events receive mismatched remapped addresses due to incorrect lookup ordering.
--

commit bea273ddc97f1917658141d7ddbe3f4f2db6086b
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/util/aslr.c b/tools/perf/util/aslr.c
> new file mode 100644
> index 0000000000000..e45f68c604937
> --- /dev/null
> +++ b/tools/perf/util/aslr.c

[ ... ]

> +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__findnew_mapping(aslr, machine, thread, cpumode,
> +							   event->mmap.start,
> +							   event->mmap.len,
> +							   event->mmap.pgoff);
> +	/*
> +	 * For anonymous memory (and kernel maps), the kernel populates the
> +	 * event's pgoff field with the original un-obfuscated virtual address
> +	 * in bytes (i.e. (addr >> PAGE_SHIFT) << PAGE_SHIFT).
> +	 * We must overwrite pgoff with the new remapped byte address to prevent
> +	 * leaking the original ASLR layout.
> +	 */
> +	if (cpumode == PERF_RECORD_MISC_KERNEL || cpumode == PERF_RECORD_MISC_GUEST_KERNEL ||
> +	    is_anon_memory(event->mmap.filename) || is_no_dso_memory(event->mmap.filename))
> +		new_event->mmap.pgoff = new_event->mmap.start;

[Severity: High]
Will unconditionally overwriting pgoff with the remapped start address for
kernel maps break symbol resolution for kernel modules?

For kernel modules synthesized by perf_event__synthesize_modules(), mmap
events are emitted with pgoff = 0 or relative file offsets. By overwriting
pgoff with the remapped start address here, the correct map offset is
destroyed.

When perf report calculates the map ip (ip - start + pgoff), would it
yield the absolute remapped virtual address instead of the correct relative
offset, causing symbol lookups in .ko files to fail?

> +	err = delegate->mmap(delegate, new_event, sample, machine);
> +	thread__put(thread);
> +	return err;
> +}
> +
> +static int aslr_tool__process_mmap2(const struct perf_tool *tool,
> +				    union perf_event *event,
> +				    struct perf_sample *sample,
> +				    struct machine *machine)
> +{

[ ... ]

> +	/* Remaps the mmap.start. */
> +	new_event->mmap2.start = aslr_tool__findnew_mapping(aslr, machine, thread, cpumode,
> +							    event->mmap2.start,
> +							    event->mmap2.len,
> +							    event->mmap2.pgoff);
> +	/*
> +	 * For anonymous memory (and kernel maps), the kernel populates the
> +	 * event's pgoff field with the original un-obfuscated virtual address
> +	 * in bytes (i.e. (addr >> PAGE_SHIFT) << PAGE_SHIFT).
> +	 * We must overwrite pgoff with the new remapped byte address to prevent
> +	 * leaking the original ASLR layout.
> +	 */
> +	if (cpumode == PERF_RECORD_MISC_KERNEL || cpumode == PERF_RECORD_MISC_GUEST_KERNEL ||
> +	    is_anon_memory(event->mmap2.filename) || is_no_dso_memory(event->mmap2.filename))
> +		new_event->mmap2.pgoff = new_event->mmap2.start;

[Severity: High]
Like the issue in aslr_tool__process_mmap() above, does overwriting pgoff
here similarly corrupt map offsets for mmap2 events from kernel modules?

> +	err = delegate->mmap2(delegate, new_event, sample, machine);
> +	thread__put(thread);
> +	return err;
> +}

[ ... ]

> +static int aslr_tool__process_ksymbol(const struct perf_tool *tool,
> +				      union perf_event *event,
> +				      struct perf_sample *sample,
> +				      struct machine *machine)
> +{

[ ... ]

> +	memcpy(&new_event->ksymbol, &event->ksymbol, event->ksymbol.header.size);
> +	/* Remaps the ksymbol.start before process_ksymbol potentially deletes the map */
> +	new_event->ksymbol.addr = aslr_tool__findnew_mapping(aslr, machine, thread,
> +							     PERF_RECORD_MISC_KERNEL,
> +							     event->ksymbol.addr,
> +							     event->ksymbol.len,
> +							     /*pgoff=*/0);
> +
> +	err = perf_event__process_ksymbol(tool, event, sample, aslr_machine);

[Severity: Medium]
Does calling aslr_tool__findnew_mapping() before perf_event__process_ksymbol()
cause ksymbol unregister events to receive a mismatched remapped address?

For a bpf register event, the map does not exist yet in aslr_machine, so
aslr_tool__findnew_mapping() records the new remapping using a key with a
NULL dso.

When the corresponding unregister event arrives, aslr_tool__findnew_mapping()
executes before perf_event__process_ksymbol() removes the map. It finds the
existing map, extracts its dso, and looks up the remapping using a key with
a non-NULL dso.

Since this lookup fails, could the unregister event get assigned a
completely different remapped address and break bpf symbol tracking?

> +	if (err) {
> +		thread__put(thread);
> +		return err;
> +	}
> +
> +	err = delegate->ksymbol(delegate, new_event, sample, machine);
> +	thread__put(thread);
> +	return err;
> +}

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