linux-kernel - Re: [tip: locking/core] lockdep: Fix lockdep recursion

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]

Date:   Thu, 15 Oct 2020 00:39:54 +0200
From:   Peter Zijlstra <peterz@...radead.org>
To:     "Paul E. McKenney" <paulmck@...nel.org>
Cc:     Boqun Feng <boqun.feng@...il.com>, Qian Cai <cai@...hat.com>,
        Steven Rostedt <rostedt@...dmis.org>,
        Ingo Molnar <mingo@...nel.org>, x86 <x86@...nel.org>,
        linux-kernel@...r.kernel.org, linux-tip-commits@...r.kernel.org,
        Linux Next Mailing List <linux-next@...r.kernel.org>,
        Stephen Rothwell <sfr@...b.auug.org.au>
Subject: Re: [tip: locking/core] lockdep: Fix lockdep recursion

On Wed, Oct 14, 2020 at 03:11:52PM -0700, Paul E. McKenney wrote:
> On Wed, Oct 14, 2020 at 11:53:19PM +0200, Peter Zijlstra wrote:
> > On Wed, Oct 14, 2020 at 11:34:05AM -0700, Paul E. McKenney wrote:
> > > commit 7deaa04b02298001426730ed0e6214ac20d1a1c1
> > > Author: Paul E. McKenney <paulmck@...nel.org>
> > > Date:   Tue Oct 13 12:39:23 2020 -0700
> > > 
> > >     rcu: Prevent lockdep-RCU splats on lock acquisition/release
> > >     
> > >     The rcu_cpu_starting() and rcu_report_dead() functions transition the
> > >     current CPU between online and offline state from an RCU perspective.
> > >     Unfortunately, this means that the rcu_cpu_starting() function's lock
> > >     acquisition and the rcu_report_dead() function's lock releases happen
> > >     while the CPU is offline from an RCU perspective, which can result in
> > >     lockdep-RCU splats about using RCU from an offline CPU.  In reality,
> > >     aside from the splats, both transitions are safe because a new grace
> > >     period cannot start until these functions release their locks.
> > 
> > But we call the trace_* crud before we acquire the lock. Are you sure
> > that's a false-positive? 
> 
> You lost me on this one.
> 
> I am assuming that you are talking about rcu_cpu_starting(), because
> that is the one where RCU is not initially watching, that is, the
> case where tracing before the lock acquisition would be a problem.
> You cannot be talking about rcu_cpu_starting() itself, because it does
> not do any tracing before acquiring the lock.  But if you are talking
> about the caller of rcu_cpu_starting(), then that caller should put the
> rcu_cpu_starting() before the tracing.  But that would be the other
> patch earlier in this thread that was proposing moving the call to
> rcu_cpu_starting() much earlier in CPU bringup.
> 
> So what am I missing here?

rcu_cpu_starting();
  raw_spin_lock_irqsave();
    local_irq_save();
    preempt_disable();
    spin_acquire()
      lock_acquire()
        trace_lock_acquire() <--- *whoopsie-doodle*
	  /* uses RCU for tracing */
    arch_spin_lock_flags() <--- the actual spinlock