Re: [PATCH 5/5] alpha/perf: optionally randomize the ProfileMe sampling interval

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] alpha/perf: 64-bit overflow in alpha_perf_event_set_period() jitter math
--

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

alpha/perf: optionally randomize the ProfileMe sampling interval

This commit adds a randomize configuration bit for Alpha ProfileMe raw
perf events to vary each sampling interval by up to 1/16 of the period in
either direction using a per-CPU prandom state.

> diff --git a/arch/alpha/kernel/perf_event.c b/arch/alpha/kernel/perf_event.c
> index 06c9afeb7c05e..da8262c74d043 100644
> --- a/arch/alpha/kernel/perf_event.c
> +++ b/arch/alpha/kernel/perf_event.c

[ ... ]

> @@ -360,6 +368,20 @@ static int alpha_perf_event_set_period(struct perf_event *event,
>  		ret = 1;
>  	}
>  
> +	/*
> +	 * Vary each new interval by up to 1/16 of the period either way, so
> +	 * the samples cannot phase-lock with a loop whose length divides the
> +	 * period. The count stays exact because prev_count records the value
> +	 * actually written.
> +	 */
> +	if (ret && (hwc->flags & ALPHA_PERF_FLAG_RANDOMIZE) && period >= 8) {
> +		long jitter = period / 8;
> +		u32 r = prandom_u32_state(this_cpu_ptr(&alpha_perf_rnd));
> +
> +		left += (long)((u64)r * jitter >> 32) - jitter / 2;
> +		local64_set(&hwc->period_left, left);
> +	}

[Severity: Medium]
Can the multiplication (u64)r * jitter overflow 64 bits in
alpha_perf_event_set_period() in arch/alpha/kernel/perf_event.c when a user
specifies a large 64-bit sample_period?

In __perf_event_open() in kernel/events/core.c, userspace can supply any
63-bit attr.sample_period:

	if (attr.freq) {
		if (attr.sample_freq > sysctl_perf_event_sample_rate)
			return -EINVAL;
	} else {
		if (attr.sample_period & (1ULL << 63))
			return -EINVAL;
	}

When hwc->sample_period is at least 1UL << 35, jitter (period / 8) is at
least 1UL << 32, so (u64)r * jitter overflows 64-bit arithmetic and wraps.
Because the 64-bit product is then shifted right by 32, the positive term
is bounded to U32_MAX while jitter / 2 remains a full 64-bit value, causing
every interval to be systematically shortened by roughly period / 16 rather
than varying symmetrically within [-period / 16, +period / 16). In addition,
if sample_period is near LONG_MAX, adding a positive 1/16 offset to left can
overflow signed long into a negative period_left.

Would using mul_u64_u32_shr(jitter, r, 32) avoid the multiplication overflow?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261028-alpha-profileme-v1-0-d44b52bf2500@gmail.com?part=5