Re: [PATCH 05/11] perf sched: Bounds-check prio before test_bit() in timehist

[Arnaldo Carvalho de Melo <acme@kernel.org>]

On Wed, Jun 10, 2026 at 05:04:00PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Low] The patch introduces a logic error that causes invalid negative priorities (<-1) to bypass the `--prio` filter.
> --
> 
> commit 3165982942697d89513b8b104d95d745a4412391
> Author: Arnaldo Carvalho de Melo <acme@redhat.com>
> 
> perf sched: Bounds-check prio before test_bit() in timehist
> 
> This commit adds a bounds check to timehist_skip_sample() to ensure that
> priority values from untrusted tracepoint data do not exceed MAX_PRIO before
> calling test_bit(). It also updates the sentinel check from != -1 to >= 0.
> 
> > diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
> > index 1ff01f03d2ad1..ded511d851880 100644
> > --- a/tools/perf/builtin-sched.c
> > +++ b/tools/perf/builtin-sched.c
> > @@ -2645,7 +2645,9 @@ static bool timehist_skip_sample(struct perf_sched *sched,
> >  		else if (evsel__name_is(sample->evsel, "sched:sched_switch"))
> >  			prio = perf_sample__intval(sample, "prev_prio");
> >  
> > -		if (prio != -1 && !test_bit(prio, sched->prio_bitmap)) {
> > +		/* prio comes from untrusted tracepoint data — bounds-check before test_bit */

Updated the comment to:

-               /* prio comes from untrusted tracepoint data — bounds-check before test_bit */
+               /* negative prio means no info; out-of-range prio can't match the filter */

To address the comment below.

- Arnaldo

> > +		if (prio >= 0 &&
> > +		    (prio >= MAX_PRIO || !test_bit(prio, sched->prio_bitmap))) {
> 
> [Severity: Low]
> Does this logic inadvertently allow invalid negative priorities (less than -1)
> to bypass the --prio filter?
> 
> By explicitly replacing the prio != -1 sentinel check with prio >= 0, the new
> logic evaluates to false for any priority less than -1. This causes the block
> to be bypassed, meaning rc remains false and the invalid negative priority is
> not skipped by the filter.
> 
> Should the check be restructured to ensure that invalid negative priorities
> from crafted or corrupted perf.data files are also properly skipped?
> 
> >  			rc = true;
> >  			sched->skipped_samples++;
> >  		}
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260610165207.2077258-1-acme@kernel.org?part=5