linux-kernel - Re: [PATCH] locking/lockdep: Fix expected depth value in __lock

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: <20151020144011.GA4277@gmail.com>
Date:	Tue, 20 Oct 2015 10:40:12 -0400
From:	Jerome Glisse <j.glisse@...il.com>
To:	Peter Zijlstra <peterz@...radead.org>
Cc:	linux-kernel@...r.kernel.org,
	Jérôme Glisse <jglisse@...hat.com>,
	Ingo Molnar <mingo@...nel.org>,
	Sasha Levin <sasha.levin@...cle.com>
Subject: Re: [PATCH] locking/lockdep: Fix expected depth value in
 __lock_release()

On Tue, Oct 20, 2015 at 03:07:29PM +0200, Peter Zijlstra wrote:
> On Tue, Oct 20, 2015 at 08:42:19AM -0400, Jerome Glisse wrote:
> > On Tue, Oct 20, 2015 at 02:18:53PM +0200, Peter Zijlstra wrote:
> 
> > > What code did you find that triggered this? That is, what code is taking
> > > nested locks with other locks in the middle? (Not wrong per-se, just
> > > curious how that would come about).
> > 
> > Well i am not able to reproduce myself but it happens as part of
> > mm_drop_all_locks() as to which lock does trigger i am unsure as
> > all the i_mmap_rwsem are taken one after the other and same for
> > anon_vma rwsem so they should already coalesce properly. My guess
> > is that code calling all lock also have a mutex and once all vma
> > lock are drop the mutex coalesce with mm_all_locks_mutex.
> 
> Ah yes, mm_all_locks_mutex looks like a likely candidate.
> 
> Curious, this code is ancient, and I've never seen a report of this
> triggering.
> 
> > > > This patch adjust the expect depth value by decrementing it if
> > > > what was previously 2 entry inside the stack are coalesced into
> > > > only one entry.
> > > 
> > > Would it not make more sense to scan the entire hlock stack on
> > > __lock_acquire() and avoid getting collapsible entries in the first
> > > place?
> > > 
> > > Something like so...
> > 
> > It would work too, probably more compute intensive than my solution
> > but this is lockdep code so i guess it is fine. Also dunno if we loose
> > any valuable information by not keeping the stack ordered so one
> > can check order in whick lock are taken.
> 
> Right; its a little bit more expensive, but only for acquires with
> nest_lock set, which should be rare.
> 
> As to the order; since they're all of the same class, its fine to
> collapse them.
> 
> However the proposed alternative avoids 'strange' boundary cases like:
> 
> 	mutex_lock(&top_lock);
> 
> 	for (...) {
> 		mutex_lock_nest_lock(&obj->lock1, &top_lock);
> 		mutex_lock_nest_lock(&obj->lock2, &top_lock);
> 	}
> 
> Which would currently result in running our of lock stack space real
> quick since it would never be able to collapse.
> 
> In any case, can you 'test' the proposed alternative in any way?

I will ask for it to be tested probably gonna take couple days before
i hear back. I will report as soon as i have confirmation.

Cheers,
Jérôme
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/