diff mbox

vfs: avoid hang caused by attempting to rmdir an invalid file system

Message ID 20120528210511.GA5610@thunk.org
State Accepted, archived
Headers show

Commit Message

Theodore Ts'o May 28, 2012, 9:05 p.m. UTC 
On Mon, May 28, 2012 at 02:29:05PM -0600, Andreas Dilger wrote:
> This patch is good from the POV of covering all filesystems, and
> avoiding the deadlock at the dcache level.  It would be possible to
> detect this problem in the filesystem itself during lookup, before
> the bad link got into the dcache itself.  Something like:

I like that as a solution for detecting the problem in ext4.  As you
say, it's still an issue for other file systems, and so the patch I
proposed is still probably a good idea for the VFS.  But this way ext4
(and ext3 when Jan backports it) will be able to detect the problem
and mark the file system as being corrupted.

Andreas, are you ok with the Signed-off-by?  I gave you credit since
the patch is yours...  (and do you want me to use the dilger.ca or the
whamcloud.com domain)?

Regards,

						- Ted

commit bfd0ca03af12fa1dc439b57f65828dde2e7530e2
Author: Andreas Dilger <adilger@dilger.ca>
Date:   Mon May 28 17:02:25 2012 -0400

    ext4: disallow hard-linked directory in ext4_lookup
    
    A hard-linked directory to its parent can cause the VFS to deadlock,
    and is a sign of a corrupted file system.  So detect this case in
    ext4_lookup(), before the rmdir() lockup scenario can take place.
    
    Signed-off-by: Andreas Dilger <adilger@dilger.ca>
    Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>

--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Jan Kara May 29, 2012, 7:50 p.m. UTC | #1 
On Mon 28-05-12 17:05:11, Ted Tso wrote:
> On Mon, May 28, 2012 at 02:29:05PM -0600, Andreas Dilger wrote:
> > This patch is good from the POV of covering all filesystems, and
> > avoiding the deadlock at the dcache level.  It would be possible to
> > detect this problem in the filesystem itself during lookup, before
> > the bad link got into the dcache itself.  Something like:
> 
> I like that as a solution for detecting the problem in ext4.  As you
> say, it's still an issue for other file systems, and so the patch I
> proposed is still probably a good idea for the VFS.  But this way ext4
> (and ext3 when Jan backports it) will be able to detect the problem
> and mark the file system as being corrupted.
  Actually, I think there's even better way. d_splice_alias() can rather
easily detect the problem and report it to filesystem. The advantage is
that the check in d_splice_alias() can catch any "hardlinks" to
directories, not just self loops. The patch is attached, I also have
corresponding handling written for ext? filesystems but that's trivial.
I'll post the whole series to Al to have a look.

								Honza

--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
index a9fd5f4..5f4a030 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -1330,6 +1330,12 @@  static struct dentry *ext4_lookup(struct inode *dir, struct dentry *dentry, stru
 			EXT4_ERROR_INODE(dir, "bad inode number: %u", ino);
 			return ERR_PTR(-EIO);
 		}
+		if (ino == dir->i_ino) {
+			EXT4_ERROR_INODE(dir, "'%.*s' linked to parent dir",
+					 dentry->d_name.len,
+					 dentry->d_name.name);
+			return ERR_PTR(-EIO);
+		}
 		inode = ext4_iget(dir->i_sb, ino);
 		if (inode == ERR_PTR(-ESTALE)) {
 			EXT4_ERROR_INODE(dir,