vfs: take i_mutex on renamed file

A read delegation is used by NFSv4 as a guarantee that a client can
perform local read opens without informing the server.

The open operation takes the last component of the pathname as an
argument, thus is also a lookup operation, and giving the client the
above guarantee means informing the client before we allow anything that
would change the set of names pointing to the inode.

Therefore, we need to break delegations on rename, link, and unlink.

We also need to prevent new delegations from being acquired while one of
these operations is in progress.

We could add some completely new locking for that purpose, but it's
simpler to use the i_mutex, since that's already taken by all the
operations we care about.

The single exception is rename.  So, modify rename to take the i_mutex
on the file that is being renamed.

Also fix up lockdep and Documentation/filesystems/directory-locking to
reflect the change.

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

diff --git a/Documentation/filesystems/directory-locking b/Documentation/filesystems/directory-locking
index ff7b611abf330d11b8a5aef416e06106a39abbca..9edbcd224c938048bdc1cab32202ffa142f2fde5 100644 (file)
--- a/Documentation/filesystems/directory-locking
+++ b/Documentation/filesystems/directory-locking
@@ -12,8 +12,8 @@ kinds of locks - per-inode (->i_mutex) and per-filesystem
 locks victim and calls the method.
 
 4) rename() that is _not_ cross-directory.  Locking rules: caller locks
-the parent, finds source and target, if target already exists - locks it
-and then calls the method.
+the parent, finds source and target, locks source, also locks target if
+it already exists, and then calls the method.
 
 5) link creation.  Locking rules:
        * lock parent
@@ -30,6 +30,7 @@ rules:
                fail with -ENOTEMPTY
        * if new parent is equal to or is a descendent of source
                fail with -ELOOP
+       * lock source if it is not a directory.
        * if target exists - lock it.
        * call the method.
 
@@ -56,9 +57,9 @@ objects - A < B iff A is an ancestor of B.
     renames will be blocked on filesystem lock and we don't start changing
     the order until we had acquired all locks).
 
-(3) any operation holds at most one lock on non-directory object and
-    that lock is acquired after all other locks.  (Proof: see descriptions
-    of operations).
+(3) locks on non-directory objects are acquired only after taking locks
+    on their parents (which remain their parents until all locks are
+    acquired, by (1) and (2)).  (Proof: see descriptions of operations).
 
        Now consider the minimal deadlock.  Each process is blocked on
 attempt to acquire some lock and already holds at least one lock.  Let's

diff --git a/fs/namei.c b/fs/namei.c
index 0062dd17eb55d0f7bc365d0e06064cfb34d43449..e420516b1cc450c00db98e31e504a33ba6450d76 100644 (file)
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -3165,6 +3165,7 @@ static int vfs_rename_other(struct inode *old_dir, struct dentry *old_dentry,
                            struct inode *new_dir, struct dentry *new_dentry)
 {
        struct inode *target = new_dentry->d_inode;
+       struct inode *source = old_dentry->d_inode;
        int error;
 
        error = security_inode_rename(old_dir, old_dentry, new_dir, new_dentry);
@@ -3172,6 +3173,7 @@ static int vfs_rename_other(struct inode *old_dir, struct dentry *old_dentry,
                return error;
 
        dget(new_dentry);
+       mutex_lock_nested(&source->i_mutex, I_MUTEX_RENAME_SOURCE);
        if (target)
                mutex_lock(&target->i_mutex);
 
@@ -3190,6 +3192,7 @@ static int vfs_rename_other(struct inode *old_dir, struct dentry *old_dentry,
 out:
        if (target)
                mutex_unlock(&target->i_mutex);
+       mutex_unlock(&source->i_mutex);
        dput(new_dentry);
        return error;
 }

diff --git a/include/linux/fs.h b/include/linux/fs.h
index 8de675523e464db26c649928610fb4a69bac7a40..6cda56e8eb20e3b7c7a287264b029318c89744d5 100644 (file)
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -860,10 +860,12 @@ static inline int inode_unhashed(struct inode *inode)
  * 0: the object of the current VFS operation
  * 1: parent
  * 2: child/target
- * 3: quota file
+ * 3: xattr
+ * 4: quota file
+ * 5: the file being renamed (used only in rename of a non-directory)
  *
  * The locking order between these classes is
- * parent -> child -> normal -> xattr -> quota
+ * parent -> child -> rename_source -> normal -> xattr -> quota
  */
 enum inode_i_mutex_lock_class
 {
@@ -871,7 +873,8 @@ enum inode_i_mutex_lock_class
        I_MUTEX_PARENT,
        I_MUTEX_CHILD,
        I_MUTEX_XATTR,
-       I_MUTEX_QUOTA
+       I_MUTEX_QUOTA,
+       I_MUTEX_RENAME_SOURCE
 };
 
 /*