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

[Arnaldo Carvalho de Melo <acme@kernel.org>]

On Thu, Jun 11, 2026 at 05:28:22PM +0000, sashiko-bot@kernel.org wrote:
> 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);

Don't we have memdup? I.e.

		aslr_event = memdup(event, event->header.size);
		if (!aslr_event)
			return -ENOMEM;

> > +		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;

Not for now, but this is a common pattern, we should eventually have:

	u32 attr_size = perf_event_attr__size(&event->attr);

acme@x1:~/git/perf-tools-next$ git grep '?: PERF_ATTR_SIZE_VER0'
tools/perf/builtin-inject.c:    attr_size = raw_attr_size ?: PERF_ATTR_SIZE_VER0;
tools/perf/util/header.c:       attr_size = raw_attr_size ?: PERF_ATTR_SIZE_VER0;
tools/perf/util/perf_event_attr_fprintf.c:      u32 attr_size = attr->size ?: PERF_ATTR_SIZE_VER0;
acme@x1:~/git/perf-tools-next$

And now the one in this series.

> > +	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