Re: [PATCH v15 5/5] perf aslr: Strip sample registers

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Pre-existing out-of-bounds heap write of 8 bytes when zeroing `config2` in `perf_event__repipe_attr` for small `perf_event_attr` sizes.
- [High] Pre-existing ASLR leak via incomplete attribute stripping in `aslr_tool__strip_evlist`.
--

commit 5ac431e4dd0a910e235e94ecc9652b8c0f318e54
Author: Ian Rogers <irogers@google.com>
Subject: perf aslr: Strip sample registers

This commit refactors the ASLR tool to strip out only the register dump 
payload by masking out the relevant perf_event_attr fields when the delegated 
tool is handling the data. It maintains the original perf_event_attr values 
in struct aslr_evsel_priv to keep samples that would otherwise be dropped.

> diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
> index 6ac6e6fb3b47e..96b90af8264c6 100644
> --- a/tools/perf/builtin-inject.c
> +++ b/tools/perf/builtin-inject.c
> @@ -254,6 +254,12 @@ static int perf_event__repipe_attr(const struct perf_tool *tool,
>  				return -ENOMEM;
>  			memcpy(stripped_event, event, event->header.size);
>  			stripped_event->attr.attr.sample_type &= ASLR_SUPPORTED_SAMPLE_TYPE;
> +			if (stripped_event->attr.attr.size >=
> +			    (offsetof(struct perf_event_attr, sample_regs_user) + sizeof(u64)))
> +				stripped_event->attr.attr.sample_regs_user = 0;
> +			if (stripped_event->attr.attr.size >=
> +			    (offsetof(struct perf_event_attr, sample_regs_intr) + sizeof(u64)))
> +				stripped_event->attr.attr.sample_regs_intr = 0;
>  
>  			if (stripped_event->attr.attr.type == PERF_TYPE_BREAKPOINT) {
>  				stripped_event->attr.attr.bp_addr = 0;

[Severity: High]
This isn't a bug introduced by this patch, but can this lead to an
out-of-bounds heap write?

In pipe mode, stripped_event is dynamically allocated based on
event->header.size. For older perf formats or dynamically sized events
(like PERF_ATTR_SIZE_VER0 which is 64 bytes), the total event size may be
just 72 bytes.

Immediately following this block, the code does:

    } else if (stripped_event->attr.attr.type >= PERF_TYPE_MAX) {
        ...
        if (pmu && (!strcmp(pmu->name, "kprobe") ||
                    !strcmp(pmu->name, "uprobe"))) {
            stripped_event->attr.attr.config1 = 0;
            stripped_event->attr.attr.config2 = 0;
        }
    }

If the event type matches a kprobe or uprobe PMU, the code zeroes config2
unconditionally. Because config2 starts at offset 64 within the attr
structure (offset 72 in the perf_event), this results in an 8-byte
out-of-bounds write of zeros on the heap. Could a size check be added
similar to the ones just introduced for the sample registers?

> diff --git a/tools/perf/util/aslr.c b/tools/perf/util/aslr.c
> index 7bd646a5c2fb1..99a6d73181746 100644
> --- a/tools/perf/util/aslr.c
> +++ b/tools/perf/util/aslr.c
[ ... ]
> +void aslr_tool__strip_evlist(const struct perf_tool *tool __maybe_unused, struct evlist *evlist)
> +{
> +	struct evsel *evsel;
> +
> +	evlist__for_each_entry(evlist, evsel) {
> +		evsel->core.attr.sample_type &= ASLR_SUPPORTED_SAMPLE_TYPE;
> +		evsel->core.attr.sample_regs_user = 0;
> +		evsel->core.attr.sample_regs_intr = 0;
> +		evsel->sample_size = __evsel__sample_size(evsel->core.attr.sample_type);
> +		evsel__calc_id_pos(evsel);
> +
> +		if (evsel->core.attr.type == PERF_TYPE_BREAKPOINT)
> +			evsel->core.attr.bp_addr = 0;

[Severity: High]
This is a pre-existing issue, but does this function miss zeroing config1
and config2 for kprobes and uprobes?

While perf_event__repipe_attr() attempts to handle this for pipe mode to
prevent ASLR leaks, it looks like aslr_tool__strip_evlist(), which is used
for non-pipe mode, entirely omits stripping these attributes.

Could this cause the unstripped config1 and config2 attributes to be
written into the file header, potentially leaking precise probe addresses
and defeating the ASLR mitigation?

> +	}
> +}

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