dcache: Reduce the scope of i_lock in d_splice_alias
authorEric W. Biederman <ebiederm@xmission.com>
Sat, 15 Aug 2015 18:36:41 +0000 (13:36 -0500)
committerAl Viro <viro@zeniv.linux.org.uk>
Fri, 21 Aug 2015 06:34:37 +0000 (02:34 -0400)
i_lock is only needed until __d_find_any_alias calls dget on the alias
dentry.  After that the reference to new ensures that dentry_kill and
d_delete will not remove the inode from the dentry, and remove the
dentry from the inode->d_entry list.

The inode i_lock came to be held over the the __d_move calls in
d_splice_alias through a series of introduction of locks with
increasing smaller scope.  First it was the dcache_lock, then
it was the dcache_inode_lock, and finally inode->i_lock.

Furthermore inode->i_lock is not held over any other calls
to d_move or __d_move so it can not provide any meaningful
rename protection.

Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
fs/dcache.c

index e3b44ca75a1b3ab2a1099197c46c604d169b37d5..5c33aeb0f68febdd03e6f478c7949fd847e9fdcd 100644 (file)
@@ -2718,7 +2718,7 @@ struct dentry *d_ancestor(struct dentry *p1, struct dentry *p2)
  * This helper attempts to cope with remotely renamed directories
  *
  * It assumes that the caller is already holding
- * dentry->d_parent->d_inode->i_mutex, inode->i_lock and rename_lock
+ * dentry->d_parent->d_inode->i_mutex, and rename_lock
  *
  * Note: If ever the locking in lock_rename() changes, then please
  * remember to update this too...
@@ -2744,7 +2744,6 @@ out_unalias:
        __d_move(alias, dentry, false);
        ret = 0;
 out_err:
-       spin_unlock(&inode->i_lock);
        if (m2)
                mutex_unlock(m2);
        if (m1)
@@ -2790,10 +2789,11 @@ struct dentry *d_splice_alias(struct inode *inode, struct dentry *dentry)
        if (S_ISDIR(inode->i_mode)) {
                struct dentry *new = __d_find_any_alias(inode);
                if (unlikely(new)) {
+                       /* The reference to new ensures it remains an alias */
+                       spin_unlock(&inode->i_lock);
                        write_seqlock(&rename_lock);
                        if (unlikely(d_ancestor(new, dentry))) {
                                write_sequnlock(&rename_lock);
-                               spin_unlock(&inode->i_lock);
                                dput(new);
                                new = ERR_PTR(-ELOOP);
                                pr_warn_ratelimited(
@@ -2812,7 +2812,6 @@ struct dentry *d_splice_alias(struct inode *inode, struct dentry *dentry)
                        } else {
                                __d_move(new, dentry, false);
                                write_sequnlock(&rename_lock);
-                               spin_unlock(&inode->i_lock);
                                security_d_instantiate(new, inode);
                        }
                        iput(inode);