linux-kernel - Re: [PATCH] smp,cpumask: Don't call functions on offline CPUs

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20190527105202.GY2623@hirez.programming.kicks-ass.net>
Date:   Mon, 27 May 2019 12:52:02 +0200
From:   Peter Zijlstra <peterz@...radead.org>
To:     Andrew Murray <andrew.murray@....com>
Cc:     Thomas Gleixner <tglx@...utronix.de>,
        Rik van Riel <riel@...riel.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] smp,cpumask: Don't call functions on offline CPUs

On Wed, May 22, 2019 at 04:14:23PM +0100, Andrew Murray wrote:
> On Wed, May 22, 2019 at 04:49:18PM +0200, Peter Zijlstra wrote:
> > On Wed, May 22, 2019 at 03:37:11PM +0100, Andrew Murray wrote:
> > > > Is perhaps the problem that on_each_cpu_cond() uses cpu_onlne_mask
> > > > without protection?
> > > 
> > > Does this prevent racing with a CPU going offline? I guess this prevents
> > > the warning at the expense of a lock - but is only beneficial in the
> > > unlikely path. (In the likely path this prevents new CPUs going offline
> > > but we don't care because we don't WARN if they aren't they when we
> > > attempt to call functions).
> > > 
> > > At least this is my limited understanding.
> > 
> > Hmm.. I don't think it could matter, we only use the mask when
> > preempt_disable(), which would already block offline, due to it using
> > stop_machine().
> 
> OK, so as long as all arches use stop_machine to bring down a CPU then
> preeempt_disable will stop racing with CPUs going offline (I don't know
> if they all do or not).

kernel/cpu.c:takedown_cpu() is generic code :-)

> > So the patch is a no-op.
> > 
> > What's the WARN you see? TLB invalidation should pass mm_cpumask(),
> > which similarly should not contain offline CPUs I'm thinking.
> 
> So the only remaining issue is if callers pass offline CPUs to the
> function (I guess there is still the chance of a race between calling
> on_each_cpu_cond_mask and entering the preempt disabled'd bit). As you
> suggest they probably don't.

Yes, it is possible to construct fail that way.