linux-kernel - Re: [PATCHv3] rcu: tree: correctly handle sparse possible 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] [day] [month] [year] [list]

Message-ID: <20160602131023.GF5231@linux.vnet.ibm.com>
Date:	Thu, 2 Jun 2016 06:10:23 -0700
From:	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>
To:	Mark Rutland <mark.rutland@....com>
Cc:	catalin.marinas@....com, dennis.chen@....com,
	jiangshanlai@...il.com, josh@...htriplett.org,
	linux-kernel@...r.kernel.org, mathieu.desnoyers@...icios.com,
	rostedt@...dmis.org, steve.capper@....com, will.deacon@....com
Subject: Re: [PATCHv3] rcu: tree: correctly handle sparse possible cpus

On Tue, May 31, 2016 at 12:03:15PM +0100, Mark Rutland wrote:
> On Mon, May 23, 2016 at 09:13:33AM -0700, Paul E. McKenney wrote:
> > On Mon, May 23, 2016 at 11:42:59AM +0100, Mark Rutland wrote:
> > > In many cases in the RCU tree code, we iterate over the set of cpus for
> > > a leaf node described by rcu_node::grplo and rcu_node::grphi, checking
> > > per-cpu data for each cpu in this range. However, if the set of possible
> > > cpus is sparse, some cpus described in this range are not possible, and
> > > thus no per-cpu region will have been allocated (or initialised) for
> > > them by the generic percpu code.
> > > 
> > > Erroneous accesses to a per-cpu area for these !possible cpus may fault
> > > or may hit other data depending on the addressed generated when the
> > > erroneous per cpu offset is applied. In practice, both cases have been
> > > observed on arm64 hardware (the former being silent, but detectable with
> > > additional patches).
> > > 
> > > To avoid issues resulting from this, we must iterate over the set of
> > > *possible* cpus for a given leaf node. This patch add a new helper,
> > > for_each_leaf_node_possible_cpu, to enable this. As iteration is often
> > > intertwined with rcu_node local bitmask manipulation, a new
> > > leaf_node_cpu_bit helper is added to make this simpler and more
> > > consistent. The RCU tree code is made to use both of these where
> > > appropriate.
> > 
> > Thank you, Mark, queued for review and further testing.
> > 
> > 							Thanx, Paul
> 
> Thanks Paul.
> 
> Unfortunately, it seems that in my haste to spin v3, I missed your suggested
> logic to handle the !cpu_possible(rnp->grplo) case [1]. Sorry about that,
> evidently I was not being sufficiently thorough.
> 
> Would you be happy to fold that in, as per the diff below? Otherwise I can send
> that as a fixup patch, or a respin the whole thing as v4, per your preference.
> 
> I've given the below a spin on arm64 atop of -rcu/dev, with and without
> RCU_BOOST and/or RCU_TRACE, and I've build-tested likewise for x86. I've
> devised and tested the !cpu_possible(rnp->grplo) case by messing with the arm64
> SMP code and the RCU tree fanout. So far everything seems happy: no build
> issues, UBSAN splats, or other runtime failures.
> 
> So fingers crossed, that's the last issue with this patch blatted...

Please do respin the patch -- cleaner history and easier for people to
dig through.

							Thanx, Paul

> Thanks,
> Mark.
> 
> [1] lkml.kernel.org/r/20160517190106.GJ3528@...ux.vnet.ibm.com
> 
> ---->8----
> diff --git a/kernel/rcu/tree.h b/kernel/rcu/tree.h
> index dc0b7da..f714f87 100644
> --- a/kernel/rcu/tree.h
> +++ b/kernel/rcu/tree.h
> @@ -291,7 +291,7 @@ struct rcu_node {
>   * Iterate over all possible CPUs in a leaf RCU node.
>   */
>  #define for_each_leaf_node_possible_cpu(rnp, cpu) \
> -       for ((cpu) = rnp->grplo; \
> +       for ((cpu) = cpumask_next(rnp->grplo - 1, cpu_possible_mask); \
>              cpu <= rnp->grphi; \
>              cpu = cpumask_next((cpu), cpu_possible_mask))
> 
>