Re: [PATCH 03/10] tracing/filters: Check CPU bounds before cpumask_test_cpu() in do_filter_cpumask_scalar()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] tracing/filters: unreachable CPU bounds check in do_filter_cpumask_scalar()
--

commit 3eff3e01752bc54a2dfd5503608817b42e7186d2
Author: Masami Hiramatsu (Google) <mhiramat@kernel.org>

tracing/filters: Check CPU bounds before cpumask_test_cpu() in do_filter_cpumask_scalar()

This commit adds a bounds check comparing cpu against nr_cpu_ids at the
beginning of do_filter_cpumask_scalar() to avoid out-of-bounds bit tests
when evaluating single-CPU cpumask predicates.

> diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
> index c2c35b4de85e0..69da95f425410 100644
> --- a/kernel/trace/trace_events_filter.c
> +++ b/kernel/trace/trace_events_filter.c
> @@ -695,6 +695,9 @@ do_filter_scalar_cpumask(int op, unsigned int cpu, const struct cpumask *mask)
>  static inline int
>  do_filter_cpumask_scalar(int op, const struct cpumask *mask, unsigned int cpu)
>  {
> +	if (cpu >= nr_cpu_ids)
> +		return op == OP_NE;

[Severity: Medium]
Can cpu >= nr_cpu_ids ever be true in do_filter_cpumask_scalar()?

The commit description states that cpulist_parse() checks against
nr_cpumask_bits (NR_CPUS). However, since commit aa47a7c215e7 ("lib/cpumask:
deprecate nr_cpumask_bits"), include/linux/cpumask.h unconditionally defines
nr_cpumask_bits as nr_cpu_ids:

    #define nr_cpumask_bits nr_cpu_ids

    static __always_inline int cpulist_parse(const char *buf, struct cpumask *dstp)
    {
        return bitmap_parselist(buf, cpumask_bits(dstp), nr_cpumask_bits);
    }

When parse_pred() in kernel/trace/trace_events_filter.c parses a cpumask
filter via cpulist_parse()->bitmap_parselist()->bitmap_check_region(), any
CPU number >= nr_cpu_ids is rejected with -ERANGE:

    if (cpulist_parse(tmp, pred->mask)) {
        kfree(tmp);
        parse_error(pe, FILT_ERR_INVALID_CPULIST, pos + i);
        goto err_free;
    }
    ...
    single = cpumask_weight(pred->mask) == 1;
    if (single) {
        pred->val = cpumask_first(pred->mask);
        kfree(pred->mask);
        pred->mask = NULL;
    }

Since filter_pred_cpumask_cpu() is the only caller of
do_filter_cpumask_scalar() and passes pred->val as cpu, isn't cpu guaranteed
to be less than nr_cpu_ids here, making this check unreachable?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/179285912318.466249.7425922682086130057.stgit@devnote2?part=3