lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <xhsmhbkqz4rqr.mognet@vschneid.remote.csb> Date: Wed, 28 Sep 2022 13:18:20 +0100 From: Valentin Schneider <vschneid@...hat.com> To: Yury Norov <yury.norov@...il.com>, linux-kernel@...r.kernel.org, netdev@...r.kernel.org, Andy Shevchenko <andriy.shevchenko@...ux.intel.com>, "David S . Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, Rasmus Villemoes <linux@...musvillemoes.dk>, Yury Norov <yury.norov@...il.com> Subject: Re: [PATCH 1/7] cpumask: fix checking valid cpu range On 19/09/22 14:05, Yury Norov wrote: > The range of valid CPUs is [0, nr_cpu_ids). Some cpumask functions are > passed with a shifted CPU index, and for them, the valid range is > [-1, nr_cpu_ids-1). Currently for those functions, we check the index > against [-1, nr_cpu_ids), which is wrong. > > Signed-off-by: Yury Norov <yury.norov@...il.com> > --- > include/linux/cpumask.h | 19 ++++++++----------- > 1 file changed, 8 insertions(+), 11 deletions(-) > > diff --git a/include/linux/cpumask.h b/include/linux/cpumask.h > index e4f9136a4a63..a1cd4eb1a3d6 100644 > --- a/include/linux/cpumask.h > +++ b/include/linux/cpumask.h > @@ -174,9 +174,8 @@ static inline unsigned int cpumask_last(const struct cpumask *srcp) > static inline > unsigned int cpumask_next(int n, const struct cpumask *srcp) > { > - /* -1 is a legal arg here. */ > - if (n != -1) > - cpumask_check(n); > + /* n is a prior cpu */ > + cpumask_check(n + 1); > return find_next_bit(cpumask_bits(srcp), nr_cpumask_bits, n + 1); I'm confused, this makes passing nr_cpu_ids-1 to cpumask_next*() trigger a warning. The documentation does states: * @n: the cpu prior to the place to search (ie. return will be > @n) So n is a valid CPU number (with -1 being the exception for scan initialization), this shouldn't exclude nr_cpu_ids-1. IMO passing nr_cpu_ids-1 should be treated the same as passing the last set bit in a bitmap: no warning, and returns the bitmap size. Otherwise reaching nr_cpu_ids-1 has to be special-cased by the calling code which seems like unnecessary boiler plate For instance, I trigger the cpumask_check() warning there: 3d2dcab932d0:block/blk-mq.c @l2047 if (--hctx->next_cpu_batch <= 0) { select_cpu: next_cpu = cpumask_next_and(next_cpu, hctx->cpumask, <----- cpu_online_mask); if (next_cpu >= nr_cpu_ids) next_cpu = blk_mq_first_mapped_cpu(hctx); hctx->next_cpu_batch = BLK_MQ_CPU_WORK_BATCH; } next_cpu is a valid CPU number, shifting it doesn't seem to make sense, and we do want it to reach nr_cpu_ids-1.
Powered by blists - more mailing lists