linux-kernel - Re: [PATCH] configfs: Silence lockdep on mkdir(), rmdir() and configfs_depend

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: <1229601399.9487.218.camel@twins>
Date:	Thu, 18 Dec 2008 12:56:39 +0100
From:	Peter Zijlstra <a.p.zijlstra@...llo.nl>
To:	Joel Becker <Joel.Becker@...cle.com>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	Louis Rilling <louis.rilling@...labs.com>,
	linux-kernel@...r.kernel.org, cluster-devel@...hat.com,
	swhiteho <swhiteho@...hat.com>
Subject: Re: [PATCH] configfs: Silence lockdep on mkdir(), rmdir() and
 configfs_depend_item()

On Thu, 2008-12-18 at 01:27 -0800, Joel Becker wrote:

> 	It's about the default_groups and how they build up and tear
> down small bits of tree.
> 	A simple creation of a config_item, a mkdir(2), is a normal VFS
> lock set and doesn't make lockdep unhappy.  But if the new config_item
> has a default_group or two, they need locking too.  Not so much on
> mkdir(2), but on rmdir(2).

Hohumm,..

So the problem is that mkdir() doesn't just create a single entity but a
whole tree:

configfs:/my_subsystem/$ mkdir foo

might result in:

 foo/
 foo/A/
 foo/B/
 foo/B/C/

which on rmdir foo you'd have to tear down, but only if its that exact
tree and not when say A has any user created directories.

VFS mkdir A/blah only synchronizes on A.i_mutex and checks S_DEAD to
avoid races with rmdir A - which would lock first parent(A).i_mutex and
then A.i_mutex before detaching A and marking it S_DEAD.

So what you're now doing is locking the full foo/ subtree in order to
check there is no user content and block mkdir/creat from generating any
- which is where the trouble comes from, right?

Like said on IRC, the whole populated thing made me think of
mount/umount (steven whitehouse seems to have had a similar notion).

You basically want to synchronize any user mkdir/creat against foo
instead of just the new parent so that rmdir foo can tell if there is
any such content without having to lock the whole subtree.

That would mean them locking both foo and the new parent (when they're
not one and the same). Trouble seems to be that vfs_mkdir() and such
already have their new parent locked, which means you cannot go about
locking foo anymore. But that would have resulted in a 3 deep
lock-chain.

(and I don't see any filesystem hooks in user_path_parent() -- which is
probably a good thing)

Bugger..

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