GFS2: check NULL return value in gfs2_ok_to_move

PATCH New Abhi Das 2 participants 4 messages Mar 12, 2014 GFS2 file system

Share & tools

Abhi Das

author

2014-03-12 08:41 UTC #0 lore raw

PATCH 1/1 GFS2: check NULL return value in gfs2_ok_to_move

gfs2_lookupi() can return NULL if the path to the root is broken by another rename/rmdir. In this case gfs2_ok_to_move() must check for this NULL pointer and return error.

Resolves: rhbz#1060246

Signed-off-by Abhi Das <[email protected]>

---
 fs/gfs2/inode.c | 4 ++++
 1 file changed, 4 insertions(+)

fs/gfs2/inode.c +4 -0

index ec455b9..b52ebf8 100644
@@ -1299,6 +1299,10 @@ static int gfs2_ok_to_move(struct gfs2_inode *this, struct gfs2_inode *to)
 		}
 
 		tmp = gfs2_lookupi(dir, &gfs2_qdotdot, 1);
+		if (!tmp) {
+			error = -ENOENT;
+			break;
+		}
 		if (IS_ERR(tmp)) {
 			error = PTR_ERR(tmp);
 			break;

Signature · 1 line

1.8.1.4

Steven Whitehouse

2014-03-12 10:03 UTC #1 lore raw

PATCH 1/1 GFS2: check NULL return value in gfs2_ok_to_move

Hi,

Quoted 6 lines · Abhi Das

gfs2_lookupi() can return NULL if the path to the root is broken by another rename/rmdir. In this case gfs2_ok_to_move() must check for this NULL pointer and return error.
Resolves: rhbz#1060246
Signed-off-by Abhi Das <[email protected]>

Thanks for the patch. I've added it to the -nmw tree,

Steve.

Quoted 19 lines

---
 fs/gfs2/inode.c | 4 ++++
 1 file changed, 4 insertions(+)

fs/gfs2/inode.c +4 -0

index ec455b9..b52ebf8 100644
@@ -1299,6 +1299,10 @@ static int gfs2_ok_to_move(struct gfs2_inode *this, struct gfs2_inode *to)
 		}
 
 		tmp = gfs2_lookupi(dir, &gfs2_qdotdot, 1);
+		if (!tmp) {
+			error = -ENOENT;
+			break;
+		}
 		if (IS_ERR(tmp)) {
 			error = PTR_ERR(tmp);
 			break;

Abhijith Das

author

2014-03-12 10:59 UTC #2 lore raw

PATCH 1/1 GFS2: check NULL return value in gfs2_ok_to_move

Err... a heads up: We might have to revise this patch. I just ran fsck on the fs after cleanly unmounting it and it came back with a bunch of errors complaining about inodes missing '.' and '..' directories. I haven't looked at the fsck log in detail, but I'll know more when I investigate it later today.

Holding off on posting RHEL6/7 versions of this patch.

Cheers!
--Abhi

----- Original Message -----

Quoted 43 lines

From: "Steven Whitehouse" <[email protected]>
To: "Abhi Das" <[email protected]>
Cc: cluster-devel at redhat.com
Sent: Wednesday, March 12, 2014 5:03:20 AM
Subject: Re: [Cluster-devel] [GFS2 PATCH] GFS2: check NULL return value in gfs2_ok_to_move
Hi,
Quoted 6 lines · Abhi Das
gfs2_lookupi() can return NULL if the path to the root is broken by another rename/rmdir. In this case gfs2_ok_to_move() must check for this NULL pointer and return error.
Resolves: rhbz#1060246
Signed-off-by Abhi Das <[email protected]>
Thanks for the patch. I've added it to the -nmw tree,
Steve.
Quoted 20 lines
---
 fs/gfs2/inode.c | 4 ++++
 1 file changed, 4 insertions(+)
fs/gfs2/inode.c +4 -0
index ec455b9..b52ebf8 100644
@@ -1299,6 +1299,10 @@ static int gfs2_ok_to_move(struct gfs2_inode *this,
 struct gfs2_inode *to)
 		}
 
 		tmp = gfs2_lookupi(dir, &gfs2_qdotdot, 1);
+		if (!tmp) {
+			error = -ENOENT;
+			break;
+		}
 		if (IS_ERR(tmp)) {
 			error = PTR_ERR(tmp);
break;

Steven Whitehouse

2014-03-12 13:08 UTC #3 lore raw

PATCH 1/1 GFS2: check NULL return value in gfs2_ok_to_move

Hi,

Quoted 10 lines · Abhijith Das

Err... a heads up: We might have to revise this patch. I just ran fsck on the fs after cleanly unmounting it and it came back with a bunch of errors complaining about inodes missing '.' and '..' directories. I haven't looked at the fsck log in detail, but I'll know more when I investigate it later today.
Holding off on posting RHEL6/7 versions of this patch.
Cheers!
--Abhi

Ok... I'd be surprised if they were related though, since the new test just aborts the rename without doing anything in that one particular case,

Steve.

Quoted 44 lines

----- Original Message -----
Quoted 43 lines
From: "Steven Whitehouse" <[email protected]>
To: "Abhi Das" <[email protected]>
Cc: cluster-devel at redhat.com
Sent: Wednesday, March 12, 2014 5:03:20 AM
Subject: Re: [Cluster-devel] [GFS2 PATCH] GFS2: check NULL return value in gfs2_ok_to_move
Hi,
Quoted 6 lines · Abhi Das
gfs2_lookupi() can return NULL if the path to the root is broken by another rename/rmdir. In this case gfs2_ok_to_move() must check for this NULL pointer and return error.
Resolves: rhbz#1060246
Signed-off-by Abhi Das <[email protected]>
Thanks for the patch. I've added it to the -nmw tree,
Steve.
Quoted 20 lines
---
 fs/gfs2/inode.c | 4 ++++
 1 file changed, 4 insertions(+)
fs/gfs2/inode.c +4 -0
index ec455b9..b52ebf8 100644
@@ -1299,6 +1299,10 @@ static int gfs2_ok_to_move(struct gfs2_inode *this,
 struct gfs2_inode *to)
 		}
 
 		tmp = gfs2_lookupi(dir, &gfs2_qdotdot, 1);
+		if (!tmp) {
+			error = -ENOENT;
+			break;
+		}
 		if (IS_ERR(tmp)) {
 			error = PTR_ERR(tmp);
break;