Re: [RFC PATCH] perf sched latency: Add histogram and time interval options

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Integer overflow in linear histogram bucket calculation allows out-of-bounds array writes.
--

--- Patch [1]: [RFC PATCH] perf sched latency: Add histogram and time interval options ---
commit 94bdb73befc369d9beaf56650ada8fcf80ad4ac5
Author: Aaron Tomlin <atomlin@atomlin.com>

perf sched latency: Add histogram and time interval options

This patch introduces options to display a latency histogram in perf sched
latency. It includes both logarithmic and linear bucketing modes, as well
as a time interval filter.

> diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
> --- a/tools/perf/builtin-sched.c
> +++ b/tools/perf/builtin-sched.c
[ ... ]
> @@ -246,6 +313,59 @@ struct perf_sched {
>  	struct perf_data *data;
>  };
>  
> +static int latency_bucket(struct perf_sched *sched, u64 delta_ns)
> +{
> +	u64 delta_us = delta_ns / NSEC_PER_USEC;
> +	int b;
> +
> +	if (sched->hist_mode == HIST_MODE_LINEAR) {
> +		b = delta_us / 100;

[Severity: Medium]
Could a very large delta_us value cause an integer overflow here?

If perf.data contains a massive time difference between wakeup and sched-in, 
delta_us / 100 could exceed INT_MAX. Since b is a signed 32-bit integer,
this would cause b to wrap into a negative number.

> +	} else {
> +		if (delta_us == 0)
> +			return 0;
> +		b = 64 - __builtin_clzll(delta_us);
> +	}
> +
> +	if (b >= NUM_LAT_BUCKETS - 1)

Because b is signed, this bounds check will evaluate to false for negative
numbers, allowing a negative index to be returned.

> +		return NUM_LAT_BUCKETS - 1;
> +	return b;
> +}
[ ... ]
> @@ -1129,10 +1249,12 @@ add_runtime_event(struct work_atoms *atoms, u64 delta,
>  }
>  
>  static void
> -add_sched_in_event(struct work_atoms *atoms, u64 timestamp)
> +add_sched_in_event(struct perf_sched *sched, struct work_atoms *atoms,
> +		   u64 timestamp)
>  {
>  	struct work_atom *atom;
>  	u64 delta;
> +	int b;
>  
>  	if (list_empty(&atoms->work_list))
>  		return;
> @@ -1158,6 +1280,10 @@ add_sched_in_event(struct work_atoms *atoms, u64 timestamp)
>  		atoms->max_lat_end = timestamp;
>  	}
>  	atoms->nb_atoms++;
> +
> +	b = latency_bucket(sched, delta);
> +	atoms->hist[b]++;
> +	sched->global_hist[b]++;

If latency_bucket() returns a negative value, does this result in an
out-of-bounds memory write that could corrupt memory before these arrays?

>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260724201207.661300-1-atomlin@atomlin.com?part=1