Re: [PATCH v12 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] Dereferencing a NULL pointer in the hashmap hash function when built with REFCNT_CHECKING.
- [Medium] Hardcoded 64-bit kernel address breaks cross-platform analysis for 32-bit architectures.
--

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

perf inject/aslr: Add ASLR tool infrastructure and MMAP tracking

This patch introduces the core ASLR remapping tool infrastructure and
implements remapping/tracking for metadata events.

> diff --git a/tools/perf/util/aslr.c b/tools/perf/util/aslr.c
> new file mode 100644
> index 0000000000000..ac24fda658a57
> --- /dev/null
> +++ b/tools/perf/util/aslr.c
[ ... ]
> +static size_t remap_addresses__hash(long _key, void *ctx __maybe_unused)
> +{
> +	struct remap_addresses_key *key = (struct remap_addresses_key *)_key;
> +
> +	return (size_t)key->machine ^ (size_t)RC_CHK_ACCESS(key->dso) ^ key->invariant ^ key->pid;

[Severity: High]
Does this code crash when built with REFCNT_CHECKING?

In aslr_tool__findnew_mapping(), if thread__find_map() fails, the fallback
code explicitly sets remap_key.dso to NULL:

	} else {
		remap_key.dso = NULL;
		remap_key.invariant = start;
	}

When this key is passed to hashmap__find(), the hash function
remap_addresses__hash() uses RC_CHK_ACCESS(key->dso).

When perf is built with REFCNT_CHECKING (e.g., via Address Sanitizer),
RC_CHK_ACCESS(object) expands strictly to object->orig. Since key->dso is
NULL, this expands to NULL->orig, resulting in an unconditional segmentation
fault when processing unmapped or anonymous memory events.

[ ... ]
> +static u64 aslr_tool__findnew_mapping(struct aslr_tool *aslr,
> +				      struct thread *aslr_thread,
> +				      u8 cpumode, u64 start,
> +				      u64 len, u64 pgoff)
> +{
[ ... ]
> +	} else {
> +		/* First address of the process, allocate key and first top address. */
> +		struct top_addresses_key *tk;
> +		struct process_top_address *top_val;
> +
> +		remap_addr = (cpumode == PERF_RECORD_MISC_KERNEL ||
> +			      cpumode == PERF_RECORD_MISC_GUEST_KERNEL) ?
> +			     kernel_space_start : user_space_start;
> +		remap_addr = round_up_to_page_size(remap_addr);

[Severity: Medium]
Will this hardcoded 64-bit kernel address break cross-platform analysis for
32-bit architectures?

The code hardcodes kernel_space_start to 0xffff800010000000 and
unconditionally applies it to any mapping with a kernel cpumode.

If perf inject --aslr is run on a perf.data file collected from a 32-bit
machine, this will inject 64-bit virtual addresses into the output stream.

Downstream analysis tools expecting 32-bit address spaces based on the ELF
e_machine type might encounter out-of-bounds addresses and fail.

Should the tool dynamically determine the architecture's address space bounds
via struct perf_env instead?

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