linux-kernel - Re: [Patch 0/2] sysfs: fix s

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: <1265283496.24455.1939.camel@laptop>
Date:	Thu, 04 Feb 2010 12:38:16 +0100
From:	Peter Zijlstra <peterz@...radead.org>
To:	"Eric W. Biederman" <ebiederm@...ssion.com>
Cc:	Greg KH <gregkh@...e.de>, Cong Wang <amwang@...hat.com>,
	linux-kernel@...r.kernel.org, Tejun Heo <tj@...nel.org>,
	Miles Lane <miles.lane@...il.com>,
	Heiko Carstens <heiko.carstens@...ibm.com>,
	Benjamin Herrenschmidt <benh@...nel.crashing.org>,
	Larry Finger <Larry.Finger@...inger.net>,
	akpm@...ux-foundation.org, Alan Stern <stern@...land.harvard.edu>
Subject: Re: [Patch 0/2] sysfs: fix s_active lockdep warning

On Fri, 2010-01-29 at 12:30 -0800, Eric W. Biederman wrote:

> We get false positives when the code of a sysfs attribute
> synchronously removes other sysfs attributes.  In general that is not
> safe due to hotplug etc, but there are specific instances of static
> sysfs entries like the pm_core where it appears to be safe.
> 
> I am not familiar with the device core lockdep issues.  Are they similar?

The device tree had the problem that we could basically hold a device
lock and an unspecified number of parent locks (iirc this was due to
device probing, where we hold the bus lock while probing/adding child
device, recursively). 

If we place each dev->lock into the same class (which would naively
happen), then this would lead to recursive lock warnings. The proposed
solution for this is to create MAX_LOCK_DEPTH classes and assign them to
the dev->lock depending on the depth in the device tree (Alan said that
MAX_LOCK_DEPTH is sufficient for all practical cases).

static struct lock_class_key dev_tree_classes[MAX_LOCK_DEPTH];

device_add() or thereabouts would have something like:

#ifdef CONFIG_PROVE_LOCKING
	BUG_ON(dev->depth >= MAX_LOCK_DEPTH);
	lockdep_set_class(dev->lock, &dev_tree_classes[dev->depth]);
#endif

Then there was a problem were we could lock all child devices while
holding the parent device lock (forgot why though), this would, on
taking the second child dev->lock, again lead to recursive lock
warnings. 

We have an annotation for that: lock_nest_lock (currently only
spin_lock_nest_lock exists, but mutex_lock_nest_lock is easily created),
and this would allow you to do things like:

	mutex_lock(&parent->lock);
	for_each_device_child(child, parent) {
		mutex_lock_nest_lock(&child->lock, &parent->lock);
		...
	}

I hope this helps in figuring out the sysfs case..

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