Re: [PATCH v6 06/13] alpha/perf: implement ProfileMe sampling

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] alpha/perf: out-of-bounds access and counter clobber in alpha_pmu_del()
--

commit 121343eeb88cb4425f51a24aa10437522d101e9f
Author: Matt Turner <mattst88@gmail.com>

alpha/perf: implement ProfileMe sampling

This commit implements ProfileMe sampling mode on Alpha EV67 and later
processors, selectable via the profileme config attribute or precise_ip.
It configures PCTR0 and PCTR1 together for ProfileMe events, attributes
samples to the latched PMPC address, and parks stopped ProfileMe counters
away from overflow to avoid PALcode interrupt lockups.

> diff --git a/arch/alpha/kernel/perf_event.c b/arch/alpha/kernel/perf_event.c
> index dd7fc4ac1396e..7208d09a32d1f 100644
> --- a/arch/alpha/kernel/perf_event.c
> +++ b/arch/alpha/kernel/perf_event.c

[ ... ]

> @@ -562,7 +644,10 @@ static void alpha_pmu_del(struct perf_event *event, int flags)
>  			alpha_perf_event_update(event, hwc, idx, 0);
>  			perf_event_update_userpage(event);
>  
> -			cpuc->idx_mask &= ~(1UL<<idx);
> +			if (hwc->flags & ALPHA_PERF_FLAG_PROFILEME)
> +				alpha_perf_event_park(hwc);
> +
> +			cpuc->idx_mask &= ~alpha_event_idx_mask(hwc, idx);

[Severity: High]
What happens in alpha_pmu_del() if an event is removed during group
scheduling rollback before maybe_change_configuration() has assigned it a
counter index?

When group_sched_in() schedules a group while the PMU is disabled,
event_sched_in() -> alpha_pmu_add() sets cpuc->current_idx[n0] to
PMC_NO_INDEX (-1):

arch/alpha/kernel/perf_event.c:alpha_pmu_add() {
	...
	if (n0 < alpha_pmu->num_pmcs) {
		cpuc->event[n0] = event;
		cpuc->evtype[n0] = event->hw.event_base;
		cpuc->current_idx[n0] = PMC_NO_INDEX;
	...
}

Because the PMU remains disabled across the group transaction,
perf_pmu_enable() does not invoke alpha_pmu_enable() ->
maybe_change_configuration() until the entire group is added.

If a subsequent sibling hardware event in the group fails alpha_pmu_add()
(for instance, if a ProfileMe event was opened with attr.disabled = 1 so
collect_events() skipped it while in PERF_EVENT_STATE_OFF when a second
hardware sibling was initialized, and the group is later enabled; or when an
Aggregate group only partially fits alongside an already-active event),
group_sched_in() rolls back the already-added event via event_sched_out() ->
alpha_pmu_del().

In alpha_pmu_del(), idx is read from cpuc->current_idx[j], which is still
PMC_NO_INDEX (-1):

1. alpha_perf_event_update(event, hwc, -1, 0) indexes
   alpha_pmu->pmc_count_shift[-1], alpha_pmu->pmc_count_mask[-1], and
   alpha_pmu->pmc_max_period[-1] out of bounds, corrupting event->count and
   hwc->period_left.

2. If the rolled-back event has ALPHA_PERF_FLAG_PROFILEME set,
   alpha_perf_event_park() writes PCTR0 and zeroes PCTR1 in hardware via
   alpha_write_pmc_profileme() even though the event was never programmed
   onto the PMU, and cpuc->idx_mask &= ~alpha_event_idx_mask(hwc, idx) clears
   EV67_PCTR_0 | EV67_PCTR_1 from cpuc->idx_mask.

3. For a non-ProfileMe event, alpha_event_idx_mask(hwc, -1) evaluates
   1UL << -1.

Should alpha_pmu_del() check that idx != PMC_NO_INDEX before updating the
event count, parking the ProfileMe counter, and clearing the counter bits in
cpuc->idx_mask (and also decrement cpuc->n_added when rolling back an
unassigned event)?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261104-alpha-profileme-v6-0-63362e9290a3@gmail.com?part=6