fix loop checks in d_materialise_unique()
Both __d_unalias() and __d_materialise_dentry() need loop prevention.
Grab rename_lock in caller, check for loops there...
As a side benefit, we have dentry_lock_for_move() called only under
rename_lock, which seriously reduces deadlock potential of the
execrable "locking order" used for ->d_lock.
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
diff --git a/fs/dcache.c b/fs/dcache.c
index 37f72ee..6e4ea6d 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -2213,14 +2213,15 @@
* The hash value has to match the hash queue that the dentry is on..
*/
/*
- * d_move - move a dentry
+ * __d_move - move a dentry
* @dentry: entry to move
* @target: new dentry
*
* Update the dcache to reflect the move of a file name. Negative
- * dcache entries should not be moved in this way.
+ * dcache entries should not be moved in this way. Caller hold
+ * rename_lock.
*/
-void d_move(struct dentry * dentry, struct dentry * target)
+static void __d_move(struct dentry * dentry, struct dentry * target)
{
if (!dentry->d_inode)
printk(KERN_WARNING "VFS: moving negative dcache entry\n");
@@ -2228,8 +2229,6 @@
BUG_ON(d_ancestor(dentry, target));
BUG_ON(d_ancestor(target, dentry));
- write_seqlock(&rename_lock);
-
dentry_lock_for_move(dentry, target);
write_seqcount_begin(&dentry->d_seq);
@@ -2275,6 +2274,20 @@
spin_unlock(&target->d_lock);
fsnotify_d_move(dentry);
spin_unlock(&dentry->d_lock);
+}
+
+/*
+ * d_move - move a dentry
+ * @dentry: entry to move
+ * @target: new dentry
+ *
+ * Update the dcache to reflect the move of a file name. Negative
+ * dcache entries should not be moved in this way.
+ */
+void d_move(struct dentry *dentry, struct dentry *target)
+{
+ write_seqlock(&rename_lock);
+ __d_move(dentry, target);
write_sequnlock(&rename_lock);
}
EXPORT_SYMBOL(d_move);
@@ -2302,7 +2315,7 @@
* This helper attempts to cope with remotely renamed directories
*
* It assumes that the caller is already holding
- * dentry->d_parent->d_inode->i_mutex and the inode->i_lock
+ * dentry->d_parent->d_inode->i_mutex, inode->i_lock and rename_lock
*
* Note: If ever the locking in lock_rename() changes, then please
* remember to update this too...
@@ -2317,11 +2330,6 @@
if (alias->d_parent == dentry->d_parent)
goto out_unalias;
- /* Check for loops */
- ret = ERR_PTR(-ELOOP);
- if (d_ancestor(alias, dentry))
- goto out_err;
-
/* See lock_rename() */
ret = ERR_PTR(-EBUSY);
if (!mutex_trylock(&dentry->d_sb->s_vfs_rename_mutex))
@@ -2331,7 +2339,7 @@
goto out_err;
m2 = &alias->d_parent->d_inode->i_mutex;
out_unalias:
- d_move(alias, dentry);
+ __d_move(alias, dentry);
ret = alias;
out_err:
spin_unlock(&inode->i_lock);
@@ -2416,15 +2424,24 @@
alias = __d_find_alias(inode, 0);
if (alias) {
actual = alias;
- /* Is this an anonymous mountpoint that we could splice
- * into our tree? */
- if (IS_ROOT(alias)) {
+ write_seqlock(&rename_lock);
+
+ if (d_ancestor(alias, dentry)) {
+ /* Check for loops */
+ actual = ERR_PTR(-ELOOP);
+ } else if (IS_ROOT(alias)) {
+ /* Is this an anonymous mountpoint that we
+ * could splice into our tree? */
__d_materialise_dentry(dentry, alias);
+ write_sequnlock(&rename_lock);
__d_drop(alias);
goto found;
+ } else {
+ /* Nope, but we must(!) avoid directory
+ * aliasing */
+ actual = __d_unalias(inode, dentry, alias);
}
- /* Nope, but we must(!) avoid directory aliasing */
- actual = __d_unalias(inode, dentry, alias);
+ write_sequnlock(&rename_lock);
if (IS_ERR(actual))
dput(alias);
goto out_nolock;