linux-kernel - Re: [PATCH 3/4] cpumask: Convert cpumask_any

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200420154929.taacfhjyeku4e5bx@e107158-lin.cambridge.arm.com>
Date:   Mon, 20 Apr 2020 16:49:29 +0100
From:   Qais Yousef <qais.yousef@....com>
To:     Steven Rostedt <rostedt@...dmis.org>
Cc:     Ingo Molnar <mingo@...hat.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Juri Lelli <juri.lelli@...hat.com>,
        Vincent Guittot <vincent.guittot@...aro.org>,
        Dietmar Eggemann <dietmar.eggemann@....com>,
        Ben Segall <bsegall@...gle.com>, Mel Gorman <mgorman@...e.de>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Yury Norov <yury.norov@...il.com>,
        Paul Turner <pjt@...gle.com>,
        Alexey Dobriyan <adobriyan@...il.com>,
        Josh Don <joshdon@...gle.com>,
        Pavan Kondeti <pkondeti@...eaurora.org>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/4] cpumask: Convert cpumask_any_but() to the new random
 function

On 04/14/20 12:28, Steven Rostedt wrote:
> On Tue, 14 Apr 2020 16:05:55 +0100
> Qais Yousef <qais.yousef@....com> wrote:
> 
> > +int cpumask_any_but(const struct cpumask *srcp, unsigned int cpu)
> > +{
> > +	unsigned int i;
> > +
> > +	cpumask_check(cpu);
> > +
> > +	for_each_cpu(i, srcp) {
> > +		i = cpumask_any(srcp);
> 
> Hmm, if the current CPU is the last CPU in the mask, and cpumask_any()
> happens to return it, what happens?

cpumask_any() will wrap.

> 
> > +		if (i != cpu)
> > +			return i;
> 
> We loop again, and wouldn't i being the last CPU in the mask cause this
> loop to exit, and return nr_cpu_ids?

No, because if we happen to start from the last cpu, on the next call, we'll
wrap again to the beginning.

But this implementation is crap indeed. No matter how unlikely, there's no
guarantee that for all the iters cpumask_any() will return a different cpu.
So if there are 3 cpus in the mask, cpumask_any() could potentially always
return 'cpu' if the planets aligned correctly.

I'll open code it instead to guarantee robustness.

Thanks!

--
Qais Yousef