Re: [PATCH v3 2/2] x86: Fix /proc/cpuinfo cpumask warning

From: Andrew Jones
Date: Mon Oct 31 2022 - 06:03:35 EST


On Mon, Oct 31, 2022 at 09:58:57AM +0100, Borislav Petkov wrote:
> On Mon, Oct 31, 2022 at 09:06:04AM +0100, Andrew Jones wrote:
> > The valid cpumask range is [0, nr_cpu_ids) and cpumask_next() always
> > returns a CPU ID greater than its input, which results in its input
> > range being [-1, nr_cpu_ids - 1). Ensure showing CPU info avoids
> > triggering error conditions in cpumask_next() by stopping its loop
>
> What error conditions?
>
> What would happen if @n is outside of the valid range?

Currently (after the revert of 78e5a3399421) with DEBUG_PER_CPU_MAPS we'll
get a warning splat when the cpu is outside the range [-1, nr_cpu_ids) and
cpumask_next() will call find_next_bit() with the input plus one anyway.
find_next_bit() doesn't explicity document what happens when an input is
outside the range, but it currently returns the bitmap size without any
side effects, which means cpumask_next() will return nr_cpu_ids.
show_cpuinfo() doesn't try to show anything in that case and stops its
loop, or, IOW, things work fine now with an input of nr_cpu_ids - 1. But,
show_cpuinfo() is just getting away with a violated cpumask_next()
contract, which 78e5a3399421 exposed. How about a new commit message like
this

seq_read_iter() and cpuinfo's start and next seq operations implement a
pattern like

n = cpumask_next(n - 1, mask);
show(n);
while (1) {
++n;
n = cpumask_next(n - 1, mask);
if (n >= nr_cpu_ids)
break;
show(n);
}

which loops until cpumask_next() identifies its CPU ID input is out of
its valid range, [-1, nr_cpu_ids - 1). seq_read_iter() assumes the
result of an invalid input is to return nr_cpu_ids or larger without any
side effects, however the cpumask API does not document that and it
reserves the right to change how it responds to invalid inputs. Ensure
inputs from seq_read_iter() are valid.

Thanks,
drew