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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Useless calls to `evsel__reset_sample_bit` due to prior bitwise AND, masking a potential desynchronization hazard.
--

commit 407d38620d5d09a61109d6eb3da76fc9aad40847
Author: Ian Rogers <irogers@google.com>

perf aslr: Strip sample registers

Refactor 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.

> diff --git a/tools/perf/util/aslr.c b/tools/perf/util/aslr.c
> index c62ae5bcc124f..19fca84a7405e 100644
> --- a/tools/perf/util/aslr.c
> +++ b/tools/perf/util/aslr.c
[ ... ]
> @@ -1126,11 +1200,69 @@ void aslr_tool__delete(struct perf_tool *tool)
[ ... ]
> +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__reset_sample_bit(evsel, REGS_USER);
> +		evsel__reset_sample_bit(evsel, REGS_INTR);

[Severity: Low]
Are these evsel__reset_sample_bit() calls acting as no-ops?

Because sample_type is already bitwise ANDed with ASLR_SUPPORTED_SAMPLE_TYPE
just above, the bits are cleared before the helper functions are called.

This leaves the internal sample_size state temporarily unsynchronized with
the sample_type flags since evsel__reset_sample_bit() will see the bits
are already cleared and return silently without decrementing sample_size.

> +		evsel->core.attr.sample_regs_user = 0;
> +		evsel->core.attr.sample_regs_intr = 0;
> +
> +		if (evsel->core.attr.type == PERF_TYPE_BREAKPOINT)
> +			evsel->core.attr.bp_addr = 0;
> +	}
> +}
> +
> +void aslr_tool__restore_evlist(const struct perf_tool *tool, struct evlist *evlist)
> +{
> +	const struct delegate_tool *del_tool = container_of(tool, const struct delegate_tool, tool);
> +	const struct aslr_tool *aslr = container_of(del_tool, const struct aslr_tool, tool);
> +	struct evsel *evsel;
> +	struct aslr_evsel_priv *priv;
> +
> +	evlist__for_each_entry(evlist, evsel) {
> +		if (hashmap__find(&aslr->evsel_orig_attrs, evsel, &priv)) {
> +			evsel->core.attr.sample_type = priv->orig_sample_type;

[Severity: Low]
If sample_size had actually been modified by the reset helpers in
aslr_tool__strip_evlist(), would it remain corrupted here?

Only the sample_type and registers are restored. If evsel__reset_sample_bit()
had worked as expected and decremented sample_size, the sample_size would
remain incorrect here when the sample_type flags are put back.

> +			evsel->core.attr.sample_regs_user = priv->orig_sample_regs_user;
> +			evsel->core.attr.sample_regs_intr = priv->orig_sample_regs_intr;
> +		}
> +	}
> +}

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