Re: [Ecryptfs-devel] [PATCH] ecryptfs: some inode attrs, and a question

[hooanon05]

> Yes, testing by d_unhashed() is best, I think.
> But all inode attributes are maintained by ecryptfs_interpose(), so you
> don't need to handle them.
> 
> And I found another bug in ecryptfs_link().
> Here is a patch including that.

I forgot i_count.
Here is the fixed one.

J. R. Okajima

----------------------------------------------------------------------
refine ecryptfs_link()

ecryptfs_link() doesn't need to,,,
- copy i_nlink and i_size for the target inode since they are already
  handled by ecryptfs_interpose(). (I don't know why i_size was
  necessary)
- maintain dir attributes based upon the lower dir, instead of
  lower_new_dentry. (another bug)
- several d_drop()s. do it only when the lower_new_dentry is d_drop()ed
  by the lower fs or vfs_link() failed.
- testing lower_new_dentry->d_inode after vfs_link() succeeded.

Signed-off-by: J. R. Okajima <hooanon05@xxxxxxxxxxx>

diff -u -p -r1.1 inode.c
--- fs/ecryptfs/inode.c	19 Dec 2008 03:05:27 -0000	1.1
+++ fs/ecryptfs/inode.c	18 Jan 2009 05:22:35 -0000
@@ -405,10 +407,8 @@ static int ecryptfs_link(struct dentry *
 	struct dentry *lower_old_dentry;
 	struct dentry *lower_new_dentry;
 	struct dentry *lower_dir_dentry;
-	u64 file_size_save;
 	int rc;
 
-	file_size_save = i_size_read(old_dentry->d_inode);
 	lower_old_dentry = ecryptfs_dentry_to_lower(old_dentry);
 	lower_new_dentry = ecryptfs_dentry_to_lower(new_dentry);
 	dget(lower_old_dentry);
@@ -416,23 +416,26 @@ static int ecryptfs_link(struct dentry *
 	lower_dir_dentry = lock_parent(lower_new_dentry);
 	rc = vfs_link(lower_old_dentry, lower_dir_dentry->d_inode,
 		      lower_new_dentry);
-	if (rc || !lower_new_dentry->d_inode)
-		goto out_lock;
-	rc = ecryptfs_interpose(lower_new_dentry, new_dentry, dir->i_sb, 0);
-	if (rc)
-		goto out_lock;
-	fsstack_copy_attr_times(dir, lower_new_dentry->d_inode);
-	fsstack_copy_inode_size(dir, lower_new_dentry->d_inode);
-	old_dentry->d_inode->i_nlink =
-		ecryptfs_inode_to_lower(old_dentry->d_inode)->i_nlink;
-	i_size_write(new_dentry->d_inode, file_size_save);
-out_lock:
+	if (!rc) {
+		fsstack_copy_attr_times(dir, lower_dir_dentry->d_inode);
+		fsstack_copy_inode_size(dir, lower_dir_dentry->d_inode);
+		if (!d_unhashed(lower_new_dentry)
+		    /* && lower_new_dentry->d_inode */) {
+			rc = ecryptfs_interpose(lower_new_dentry, new_dentry,
+						dir->i_sb, 0);
+			if (!rc) {
+				/* ecryptfs_interpose() doesn't increment */
+				atomic_inc(&inode->i_count);
+				goto out_lock; /* success */
+			}
+		}
+	}
+ out_drop:
+	d_drop(new_dentry);
+ out_lock:
 	unlock_dir(lower_dir_dentry);
 	dput(lower_new_dentry);
 	dput(lower_old_dentry);
-	d_drop(lower_old_dentry);
-	d_drop(new_dentry);
-	d_drop(old_dentry);
 	return rc;
 }
 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/