Unionfs: fix readlink/follow_link to add locking

Signed-off-by: Erez Zadok <ezk@cs.sunysb.edu>

diff --git a/fs/unionfs/inode.c b/fs/unionfs/inode.c
index 15e1e331d28d4d79458f4a435446178f1881dcec..cfe2088cc099a2f0880ac9f11a62b3ad55608210 100644 (file)
--- a/fs/unionfs/inode.c
+++ b/fs/unionfs/inode.c
@@ -636,21 +636,12 @@ out:
        return err;
 }
 
-static int unionfs_readlink(struct dentry *dentry, char __user *buf,
-                           int bufsiz)
+/* requires sb, dentry, and parent to already be locked */
+static int __unionfs_readlink(struct dentry *dentry, char __user *buf,
+                             int bufsiz)
 {
        int err;
        struct dentry *lower_dentry;
-       struct dentry *parent;
-
-       unionfs_read_lock(dentry->d_sb, UNIONFS_SMUTEX_CHILD);
-       parent = unionfs_lock_parent(dentry, UNIONFS_DMUTEX_PARENT);
-       unionfs_lock_dentry(dentry, UNIONFS_DMUTEX_CHILD);
-
-       if (unlikely(!__unionfs_d_revalidate(dentry, parent, NULL, false))) {
-               err = -ESTALE;
-               goto out;
-       }
 
        lower_dentry = unionfs_lower_dentry(dentry);
 
@@ -662,10 +653,31 @@ static int unionfs_readlink(struct dentry *dentry, char __user *buf,
 
        err = lower_dentry->d_inode->i_op->readlink(lower_dentry,
                                                    buf, bufsiz);
-       if (err > 0)
+       if (err >= 0)
                fsstack_copy_attr_atime(dentry->d_inode,
                                        lower_dentry->d_inode);
 
+out:
+       return err;
+}
+
+static int unionfs_readlink(struct dentry *dentry, char __user *buf,
+                           int bufsiz)
+{
+       int err;
+       struct dentry *parent;
+
+       unionfs_read_lock(dentry->d_sb, UNIONFS_SMUTEX_CHILD);
+       parent = unionfs_lock_parent(dentry, UNIONFS_DMUTEX_PARENT);
+       unionfs_lock_dentry(dentry, UNIONFS_DMUTEX_CHILD);
+
+       if (unlikely(!__unionfs_d_revalidate(dentry, parent, NULL, false))) {
+               err = -ESTALE;
+               goto out;
+       }
+
+       err = __unionfs_readlink(dentry, buf, bufsiz);
+
 out:
        unionfs_check_dentry(dentry);
        unionfs_unlock_dentry(dentry);
@@ -675,22 +687,16 @@ out:
        return err;
 }
 
-/*
- * unionfs_follow_link takes a dentry, but it is simple.  It only needs to
- * allocate some memory and then call our ->readlink method.  Our
- * unionfs_readlink *does* lock our dentry and revalidate the dentry.
- * Therefore, we do not have to lock our dentry here, to prevent a deadlock;
- * nor do we need to revalidate it either.  It is safe to not lock our
- * dentry here, nor revalidate it, because unionfs_follow_link does not do
- * anything (prior to calling ->readlink) which could become inconsistent
- * due to branch management.  We also don't need to lock our super because
- * this function isn't affected by branch-management.
- */
 static void *unionfs_follow_link(struct dentry *dentry, struct nameidata *nd)
 {
        char *buf;
        int len = PAGE_SIZE, err;
        mm_segment_t old_fs;
+       struct dentry *parent;
+
+       unionfs_read_lock(dentry->d_sb, UNIONFS_SMUTEX_CHILD);
+       parent = unionfs_lock_parent(dentry, UNIONFS_DMUTEX_PARENT);
+       unionfs_lock_dentry(dentry, UNIONFS_DMUTEX_CHILD);
 
        /* This is freed by the put_link method assuming a successful call. */
        buf = kmalloc(len, GFP_KERNEL);
@@ -702,7 +708,7 @@ static void *unionfs_follow_link(struct dentry *dentry, struct nameidata *nd)
        /* read the symlink, and then we will follow it */
        old_fs = get_fs();
        set_fs(KERNEL_DS);
-       err = dentry->d_inode->i_op->readlink(dentry, (char __user *)buf, len);
+       err = __unionfs_readlink(dentry, buf, len);
        set_fs(old_fs);
        if (err < 0) {
                kfree(buf);
@@ -714,12 +720,15 @@ static void *unionfs_follow_link(struct dentry *dentry, struct nameidata *nd)
        err = 0;
 
 out:
-       if (!err) {
-               unionfs_lock_dentry(dentry, UNIONFS_DMUTEX_CHILD);
+       if (err >= 0) {
+               unionfs_check_nd(nd);
                unionfs_check_dentry(dentry);
-               unionfs_unlock_dentry(dentry);
        }
-       unionfs_check_nd(nd);
+
+       unionfs_unlock_dentry(dentry);
+       unionfs_unlock_parent(dentry, parent);
+       unionfs_read_unlock(dentry->d_sb);
+
        return ERR_PTR(err);
 }