[Ocfs2-devel] [PATCH] ocfs2: fix the wrong directory passed to ocfs2_lookup_ino_from_name() when link file

[Ocfs2-devel] [PATCH] ocfs2: fix the wrong directory passed to ocfs2_lookup_ino_from_name() when link file

[Xue jiufei]

In function ocfs2_link(), parent directory inode passed to function
ocfs2_lookup_ino_from_name() is wrong. Parameter dir is the parent
of new_dentry not old_dentry. We should get old_dir from old_dentry
and lookup old_dentry in old_dir in case another node remove the old dentry.

Reported-by: Szabo Aron - UBIT <aron@ubit.hu>
Signed-off-by: joyce.xue <xuejiufei@huawei.com>
---
 fs/ocfs2/namei.c | 43 +++++++++++++++++++++++++++++++++++--------
 1 file changed, 35 insertions(+), 8 deletions(-)

diff --git a/fs/ocfs2/namei.c b/fs/ocfs2/namei.c
index b931e04..914c121 100644
--- a/fs/ocfs2/namei.c
+++ b/fs/ocfs2/namei.c
@@ -94,6 +94,14 @@ static int ocfs2_create_symlink_data(struct ocfs2_super *osb,
 				     struct inode *inode,
 				     const char *symname);
 
+static int ocfs2_double_lock(struct ocfs2_super *osb,
+			     struct buffer_head **bh1,
+			     struct inode *inode1,
+			     struct buffer_head **bh2,
+			     struct inode *inode2,
+			     int rename);
+
+static void ocfs2_double_unlock(struct inode *inode1, struct inode *inode2);
 /* An orphan dir name is an 8 byte value, printed as a hex string */
 #define OCFS2_ORPHAN_NAMELEN ((int)(2 * sizeof(u64)))
 
@@ -678,8 +686,10 @@ static int ocfs2_link(struct dentry *old_dentry,
 {
 	handle_t *handle;
 	struct inode *inode = old_dentry->d_inode;
+	struct inode *old_dir = old_dentry->d_parent->d_inode;
 	int err;
 	struct buffer_head *fe_bh = NULL;
+	struct buffer_head *old_dir_bh = NULL;
 	struct buffer_head *parent_fe_bh = NULL;
 	struct ocfs2_dinode *fe = NULL;
 	struct ocfs2_super *osb = OCFS2_SB(dir->i_sb);
@@ -696,19 +706,33 @@ static int ocfs2_link(struct dentry *old_dentry,
 
 	dquot_initialize(dir);
 
-	err = ocfs2_inode_lock_nested(dir, &parent_fe_bh, 1, OI_LS_PARENT);
+	err = ocfs2_double_lock(osb, &old_dir_bh, old_dir,
+			&parent_fe_bh, dir, 0);
 	if (err < 0) {
 		if (err != -ENOENT)
 			mlog_errno(err);
 		return err;
 	}
 
+	/* make sure both dirs have bhs
+	 * get an extra ref on old_dir_bh if old==new */
+	if (!parent_fe_bh) {
+		if (old_dir_bh) {
+			parent_fe_bh = old_dir_bh;
+			get_bh(parent_fe_bh);
+		} else {
+			mlog(ML_ERROR, "%s: no old_dir_bh!\n", osb->uuid_str);
+			err = -EIO;
+			goto out;
+		}
+	}
+
 	if (!dir->i_nlink) {
 		err = -ENOENT;
 		goto out;
 	}
 
-	err = ocfs2_lookup_ino_from_name(dir, old_dentry->d_name.name,
+	err = ocfs2_lookup_ino_from_name(old_dir, old_dentry->d_name.name,
 			old_dentry->d_name.len, &old_de_ino);
 	if (err) {
 		err = -ENOENT;
@@ -801,10 +825,11 @@ out_unlock_inode:
 	ocfs2_inode_unlock(inode, 1);
 
 out:
-	ocfs2_inode_unlock(dir, 1);
+	ocfs2_double_unlock(old_dir, dir);
 
 	brelse(fe_bh);
 	brelse(parent_fe_bh);
+	brelse(old_dir_bh);
 
 	ocfs2_free_dir_lookup_result(&lookup);
 
@@ -1072,14 +1097,15 @@ static int ocfs2_check_if_ancestor(struct ocfs2_super *osb,
 }
 
 /*
- * The only place this should be used is rename!
+ * The only place this should be used is rename and link!
  * if they have the same id, then the 1st one is the only one locked.
  */
 static int ocfs2_double_lock(struct ocfs2_super *osb,
 			     struct buffer_head **bh1,
 			     struct inode *inode1,
 			     struct buffer_head **bh2,
-			     struct inode *inode2)
+			     struct inode *inode2,
+			     int rename)
 {
 	int status;
 	int inode1_is_ancestor, inode2_is_ancestor;
@@ -1127,7 +1153,7 @@ static int ocfs2_double_lock(struct ocfs2_super *osb,
 		}
 		/* lock id2 */
 		status = ocfs2_inode_lock_nested(inode2, bh2, 1,
-						 OI_LS_RENAME1);
+				rename == 1 ? OI_LS_RENAME1 : OI_LS_PARENT);
 		if (status < 0) {
 			if (status != -ENOENT)
 				mlog_errno(status);
@@ -1136,7 +1162,8 @@ static int ocfs2_double_lock(struct ocfs2_super *osb,
 	}
 
 	/* lock id1 */
-	status = ocfs2_inode_lock_nested(inode1, bh1, 1, OI_LS_RENAME2);
+	status = ocfs2_inode_lock_nested(inode1, bh1, 1,
+			rename == 1 ?  OI_LS_RENAME2 : OI_LS_PARENT);
 	if (status < 0) {
 		/*
 		 * An error return must mean that no cluster locks
@@ -1252,7 +1279,7 @@ static int ocfs2_rename(struct inode *old_dir,
 
 	/* if old and new are the same, this'll just do one lock. */
 	status = ocfs2_double_lock(osb, &old_dir_bh, old_dir,
-				   &new_dir_bh, new_dir);
+				   &new_dir_bh, new_dir, 1);
 	if (status < 0) {
 		mlog_errno(status);
 		goto bail;
-- 
1.8.4.3

[Ocfs2-devel] [PATCH] ocfs2: fix the wrong directory passed to ocfs2_lookup_ino_from_name() when link file

[Andrew Morton]

On Fri, 19 Dec 2014 18:07:45 +0800 Xue jiufei <xuejiufei@huawei.com> wrote:

> In function ocfs2_link(), parent directory inode passed to function
> ocfs2_lookup_ino_from_name() is wrong. Parameter dir is the parent
> of new_dentry not old_dentry. We should get old_dir from old_dentry
> and lookup old_dentry in old_dir in case another node remove the old dentry.

What are the user-visible effects of this change?

[Ocfs2-devel] [PATCH] ocfs2: fix the wrong directory passed to ocfs2_lookup_ino_from_name() when link file

[Aron Szabo]

Hi Andew!

Hard linking works again, when paths are relative with at least one 
subdirectory. This is how the problem was reproducable:

# mkdir a
# mkdir b
# touch a/test
# ln a/test b/test
ln: failed to create hard link `b/test' => `a/test': No such file or 
directory

However when creating links in the same dir, it worked well.

Now the link gets created.

Thanks for the quick fix Xue!

Yours,
Aron

12/19/2014 11:15 PM keltez?ssel, Andrew Morton ?rta:
> On Fri, 19 Dec 2014 18:07:45 +0800 Xue jiufei <xuejiufei@huawei.com> wrote:
>
>> In function ocfs2_link(), parent directory inode passed to function
>> ocfs2_lookup_ino_from_name() is wrong. Parameter dir is the parent
>> of new_dentry not old_dentry. We should get old_dir from old_dentry
>> and lookup old_dentry in old_dir in case another node remove the old dentry.
> What are the user-visible effects of this change?

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://oss.oracle.com/pipermail/ocfs2-devel/attachments/20141222/0cf52b56/attachment.html 

[Ocfs2-devel] [PATCH] ocfs2: fix the wrong directory passed to ocfs2_lookup_ino_from_name() when link file

[Andrew Morton]

On Mon, 22 Dec 2014 09:52:14 +0100 Aron Szabo <aron@ubit.hu> wrote:

> 12/19/2014 11:15 PM keltez__ssel, Andrew Morton __rta:
> > On Fri, 19 Dec 2014 18:07:45 +0800 Xue jiufei <xuejiufei@huawei.com> wrote:
> >
> >> In function ocfs2_link(), parent directory inode passed to function
> >> ocfs2_lookup_ino_from_name() is wrong. Parameter dir is the parent
> >> of new_dentry not old_dentry. We should get old_dir from old_dentry
> >> and lookup old_dentry in old_dir in case another node remove the old dentry.
> > What are the user-visible effects of this change?
> 
> Hi Andew!
> 
> Hard linking works again, when paths are relative with at least one 
> subdirectory. This is how the problem was reproducable:
> 
> # mkdir a
> # mkdir b
> # touch a/test
> # ln a/test b/test
> ln: failed to create hard link `b/test' => `a/test': No such file or 
> directory
> 
> However when creating links in the same dir, it worked well.
> 
> Now the link gets created.
> 
> Thanks for the quick fix Xue!

(top-posting untangled)

When you say "works again", you mean that we broke it?  This patch
fixes a regression?  If so, do we know what caused that regression?  Or
at least when it occurred?

[Ocfs2-devel] [PATCH] ocfs2: fix the wrong directory passed to ocfs2_lookup_ino_from_name() when link file

[Joseph Qi]

On 2015/1/6 6:16, Andrew Morton wrote:
> On Mon, 22 Dec 2014 09:52:14 +0100 Aron Szabo <aron@ubit.hu> wrote:
> 
>> 12/19/2014 11:15 PM keltez__ssel, Andrew Morton __rta:
>>> On Fri, 19 Dec 2014 18:07:45 +0800 Xue jiufei <xuejiufei@huawei.com> wrote:
>>>
>>>> In function ocfs2_link(), parent directory inode passed to function
>>>> ocfs2_lookup_ino_from_name() is wrong. Parameter dir is the parent
>>>> of new_dentry not old_dentry. We should get old_dir from old_dentry
>>>> and lookup old_dentry in old_dir in case another node remove the old dentry.
>>> What are the user-visible effects of this change?
>>
>> Hi Andew!
>>
>> Hard linking works again, when paths are relative with at least one 
>> subdirectory. This is how the problem was reproducable:
>>
>> # mkdir a
>> # mkdir b
>> # touch a/test
>> # ln a/test b/test
>> ln: failed to create hard link `b/test' => `a/test': No such file or 
>> directory
>>
>> However when creating links in the same dir, it worked well.
>>
>> Now the link gets created.
>>
>> Thanks for the quick fix Xue!
> 
> (top-posting untangled)
> 
> When you say "works again", you mean that we broke it?  This patch
> fixes a regression?  If so, do we know what caused that regression?  Or
> at least when it occurred?

Yes? it fixes commit 0e048316ff57 (ocfs2: check existence of old dentry
in ocfs2_link()).
> 
> 
> 
> _______________________________________________
> Ocfs2-devel mailing list
> Ocfs2-devel at oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/ocfs2-devel
> 
>