Re: [PATCH v20 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 1 potential issue(s) to consider:
- [Medium] Heap out-of-bounds read/write in `perf_event__repipe_attr` via TOCTOU race condition
--

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

perf inject: Add ASLR virtual address remapping

This patch introduces the core ASLR remapping tool infrastructure and
implements remapping/tracking for metadata events. Sample events are
delegated without remapping for now.

> diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
> index 75ffe31d03fe..8bb37095e2de 100644
> --- a/tools/perf/builtin-inject.c
> +++ b/tools/perf/builtin-inject.c
> @@ -234,20 +237,36 @@ static int perf_event__repipe_attr(const struct perf_tool *tool,
>  	u64 *ids;
>  	int ret;
>  
> +	union perf_event *aslr_event = NULL;
> +
>  	ret = perf_event__process_attr(tool, event, pevlist);
>  	if (ret)
>  		return ret;
>  
> +	if (inject->aslr) {
> +		aslr_event = malloc(event->header.size);
> +		if (!aslr_event)
> +			return -ENOMEM;
> +		memcpy(aslr_event, event, event->header.size);
> +		aslr_tool__strip_attr_event(aslr_event, pevlist);
> +		event = aslr_event;
> +	}

[Severity: Medium]
Could this introduce a TOCTOU (Time-Of-Check to Time-Of-Use) race condition?

Since event points directly to the memory-mapped perf.data file contents,
reading event->header.size multiple times without READ_ONCE allows a
concurrent modification to change the size between the malloc() and memcpy()
calls.

This could result in a heap buffer overflow during memcpy(), or bypass the
earlier size validation done in perf_event__process_attr(). This would leave
malicious sizes in the copied event->attr.attr.size for
aslr_tool__strip_attr_event() to blindly trust.

> diff --git a/tools/perf/util/aslr.c b/tools/perf/util/aslr.c
> new file mode 100644
> index 000000000000..56fc444fbf54
> --- /dev/null
> +++ b/tools/perf/util/aslr.c
[ ... ]
> +void aslr_tool__strip_attr_event(union perf_event *event, struct evlist **pevlist __maybe_unused)
> +{
> +	u32 attr_size;
> +
> +	attr_size = event->attr.attr.size ?: PERF_ATTR_SIZE_VER0;
> +
> +	if (attr_size >= (offsetof(struct perf_event_attr, sample_type) + sizeof(u64))) {
> +		event->attr.attr.sample_type &= ASLR_SUPPORTED_SAMPLE_TYPE;
> +	}
> +
> +	if (attr_size >= (offsetof(struct perf_event_attr, type) + sizeof(u32))) {
> +		u32 type = event->attr.attr.type;
> +
> +		if (type == PERF_TYPE_BREAKPOINT &&
> +		    attr_size >= (offsetof(struct perf_event_attr, bp_addr) + sizeof(u64))) {
> +			event->attr.attr.bp_addr = 0;
> +		} else if (type >= PERF_TYPE_MAX) {
> +			struct perf_pmu *pmu;
> +
> +			pmu = perf_pmus__find_by_type(type);
> +			if (pmu && (!strcmp(pmu->name, "kprobe") ||
> +				    !strcmp(pmu->name, "uprobe"))) {
> +				if (attr_size >= (offsetof(struct perf_event_attr, config1) + sizeof(u64)))
> +					event->attr.attr.config1 = 0;
> +				if (attr_size >= (offsetof(struct perf_event_attr, config2) + sizeof(u64)))
> +					event->attr.attr.config2 = 0;

[Severity: Medium]
If a malicious size is copied into the heap buffer due to the TOCTOU race in
perf_event__repipe_attr(), does this code perform an out-of-bounds write?

Since aslr_tool__strip_attr_event() implicitly trusts the attr_size read
from the heap copy, clearing config1 and config2 could overwrite adjacent
heap memory if attr_size is large, but the actual allocated
event->header.size was small.

> +			}
> +		}
> +	}
> +}

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