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 PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Fri, 30 Sep 2022 19:02:50 -0700 From: Yury Norov <yury.norov@...il.com> To: Valentin Schneider <vschneid@...hat.com> Cc: 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> Subject: Re: [PATCH 1/7] cpumask: fix checking valid cpu range On Fri, Sep 30, 2022 at 06:04:08PM +0100, Valentin Schneider wrote: [...] > > next_cpu is a valid CPU number for all, but not for cpumask_next(). > > The warning is valid. If we are at the very last cpu, what for we look > > for next? > > > > Consider: > > nr_cpu_ids=4 > > A) > cpumask: 0.1.1.0 > CPU 0 1 2 3 > n ^ > result: nr_cpu_ids > > B) > cpumask: 0.0.1.1 > CPU 0 1 2 3 > n ^ > result: nr_cpu_ids + WARN > > Both scenarios are identical from a user perspective: a valid CPU number > was passed in (either from smp_processor_id() or from a previous call to > cpumask_next*()), but there are no more bits set in the cpumask. There's no > more CPUs to search for in both scenarios, but only one produces as WARN. It seems I have to repeat it for the 3rd time. cpumask_next() takes shifted cpu index. That's why cpumask_check() must shift the index in the other direction to keep all that checking logic consistent. This is a bad design, and all users of cpumask_next() must be aware of this pitfall. [...] > > Maybe we should consider nr_cpu_ids as a special valid index for > > cpumask_check(), a sign of the end of an array. This would help to > > silence many warnings, like this one. For now I'm leaning towards that > > it's more a hack than a meaningful change. > > > > I agree, we definitely want to warn for e.g. > > cpumask_set_cpu(nr_cpu_ids, ...); > > Could we instead make cpumask_next*() immediately return nr_cpu_ids when > passed n=nr_cpu_ids-1? This is what FIND_NEXT_BIT() does. If you're suggesting to silence the warning - what for do we need it at all? > Also, what about cpumask_next_wrap()? That uses cpumask_next() under the > hood and is bound to warn when wrapping after n=nr_cpu_ids-1, I think. I'm working on a fix for it. Hopefully will merge it in next window. Thanks, Yury
Powered by blists - more mailing lists