linux-kernel - Re: check && lockdep_no_validate (Was: lockdep: Introduce wait-type checks)

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: <Pine.LNX.4.44L0.1401161515190.1109-100000@iolanthe.rowland.org>
Date:	Thu, 16 Jan 2014 15:26:03 -0500 (EST)
From:	Alan Stern <stern@...land.harvard.edu>
To:	Peter Zijlstra <peterz@...radead.org>
cc:	Oleg Nesterov <oleg@...hat.com>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	<linux-kernel@...r.kernel.org>, Ingo Molnar <mingo@...nel.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Steven Rostedt <rostedt@...dmis.org>,
	Paul McKenney <paulmck@...ux.vnet.ibm.com>,
	Linus Torvalds <torvalds@...ux-foundation.org>
Subject: Re: check && lockdep_no_validate (Was: lockdep: Introduce wait-type
 checks)

On Thu, 16 Jan 2014, Peter Zijlstra wrote:

> On Thu, Jan 16, 2014 at 06:43:48PM +0100, Oleg Nesterov wrote:
> > But with or without this change the following code
> > 
> > 		static DEFINE_MUTEX(m1);
> > 		static DEFINE_MUTEX(mx);
> > 
> > 		lockdep_set_novalidate_class(&mx);
> > 
> > 		// m1 -> mx
> > 		mutex_lock(&m1);
> > 		mutex_lock(&mx);
> > 		mutex_unlock(&mx);
> > 		mutex_unlock(&m1);
> > 
> > 		// mx -> m1 ; should trigger the warning ???
> > 		mutex_lock(&mx);
> > 		mutex_lock(&m1);
> > 		mutex_unlock(&m1);
> > 		mutex_unlock(&mx);
> > 
> > doesn't trigger the warning too. This is correct because
> > lockdep_set_novalidate_class() means, well, no-validate.
> > The question is: do we really want to avoid all validations?
> 
> Good question.
> 
> > Why lockdep_set_novalidate_class() was added? Unlees I missed
> > something the problem is that (say) __driver_attach() can take
> > the "same" lock twice, drivers/base/ lacks annotations.
> 
> Indeed, the driver model locking always slips my mind but yes its
> creative. Alan Stern seems to have a good grasp on it though.

Mostly my "grasp" is just a firm belief that trying to manage locking
throughout the entire driver tree is hopeless.  Individual sub-portions
of it are usually well behaved, but the thing as a whole is a mess.

> > Perhaps we should change the meaning of lockdep_set_novalidate_class?
> > (perhaps with rename). What do you think about the patch below?
> > 
> > With this patch __lockdep_no_validate__ means "automatically nested",
> 
> Yes, I suppose that might work, it would allow some validation.

I haven't seen the patch, but I'm not so sure it will work.  Suppose we
have two devices, D1 and D2, and some other mutex, M.  Then the locking
pattern:

	lock(D1);
	lock(M);
	unlock(M);
	unlock(D1);

generally should not conflict with:

	lock(M);
	lock(D2);
	unlock(D2);
	unlock(M);

even though D1's and D2's locks belong to the same class.  For example,
M might be a mutex embedded in the private data associated with D1, and
D2 might be a child of D1.

Alan Stern

--
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/