Re: [PATCH 1/5] perf tools: Guard remaining test_bit calls from OOB sample CPU

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Logic error in CPU filtering bounds checks allows out-of-bounds CPU IDs to bypass the filter.
--

commit c7a6257406af3fb4587a3e6f3454c67efbc1b249
Author: Arnaldo Carvalho de Melo <acme@redhat.com>

perf tools: Guard remaining test_bit calls from OOB sample CPU

This commit adds bounds checking to filter_cpu in auxtrace.c and
builtin-script.c to prevent out-of-bounds reads in test_bit when
the CPU value is corrupt or absent.

> diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> index f4aa255fc3297..9ac29bdc3cd54 100644
> --- a/tools/perf/builtin-script.c
> +++ b/tools/perf/builtin-script.c
> @@ -2646,7 +2646,7 @@ static int cleanup_scripting(void)
>  
>  static bool filter_cpu(struct perf_sample *sample)
>  {
> -	if (cpu_list && sample->cpu != (u32)-1)
> +	if (cpu_list && sample->cpu != (u32)-1 && sample->cpu < MAX_NR_CPUS)
>  		return !test_bit(sample->cpu, cpu_bitmap);
>  	return false;
>  }

[Severity: Medium]
When sample->cpu is greater than or equal to MAX_NR_CPUS, the condition fails
and the function returns false. Does returning false here mean the out-of-bounds
sample is kept rather than dropped?

> diff --git a/tools/perf/util/auxtrace.c b/tools/perf/util/auxtrace.c
> index 5f4aa1701aef6..4cd2caf540152 100644
> --- a/tools/perf/util/auxtrace.c
> +++ b/tools/perf/util/auxtrace.c
> @@ -372,7 +372,8 @@ static bool filter_cpu(struct perf_session *session, struct perf_cpu cpu)
>  {
>  	unsigned long *cpu_bitmap = session->itrace_synth_opts->cpu_bitmap;
>  
> -	return cpu_bitmap && cpu.cpu != -1 && !test_bit(cpu.cpu, cpu_bitmap);
> +	return cpu_bitmap && cpu.cpu >= 0 && cpu.cpu < MAX_NR_CPUS &&
> +	       !test_bit(cpu.cpu, cpu_bitmap);
>  }

[Severity: Medium]
Similarly in auxtrace.c, if cpu.cpu is greater than or equal to MAX_NR_CPUS,
this boolean expression evaluates to false.

In auxtrace_queues__add_buffer, if filter_cpu returns false, the buffer is kept
and queued:

	if (filter_cpu(session, buffer->cpu))
		return 0;

	buffer = memdup(buffer, sizeof(*buffer));

Could this cause corrupted events to bypass the filter and be processed
downstream?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260605121515.1725549-1-acme@kernel.org?part=1