GIT: unionfs2-2.6.27.y: Unionfs: fix readlink/follow_link to add locking

Erez Zadok ezk at fsl.cs.sunysb.edu
Thu Aug 12 23:18:10 EDT 2010


commit 076397b222c0f8bd8325aada247c72dc7733f7ec
Author: Erez Zadok <ezk at cs.sunysb.edu>
Date:   Wed Sep 17 03:27:35 2008 -0400

    Unionfs: fix readlink/follow_link to add locking
    
    Signed-off-by: Erez Zadok <ezk at cs.sunysb.edu>

diff --git a/fs/unionfs/inode.c b/fs/unionfs/inode.c
index 15e1e33..cfe2088 100644
--- 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,11 +653,32 @@ 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);
 	unionfs_unlock_parent(dentry, parent);
@@ -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);
 }
 


More information about the unionfs-cvs mailing list