2008-07-08 14:42:06

by Duane Griffin

[permalink] [raw]
Subject: [PATCH] ext4: handle corrupted orphan list at mount

If the orphan node list includes valid, untruncatable nodes with nlink > 0
the ext4_orphan_cleanup loop which attempts to delete them will not do so,
causing it to loop forever. Fix by checking for such nodes in the
ext4_orphan_get function.

This patch fixes the second case (image hdb.20000009.softlockup.gz)
reported in http://bugzilla.kernel.org/show_bug.cgi?id=10882.

Signed-off-by: Duane Griffin <[email protected]>
--

This is an ext4 version of an ext3 patch queued in -mm.
---

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 8158083..f7a0758 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -1039,6 +1039,7 @@ extern void ext4_discard_reservation (struct inode *);
extern void ext4_dirty_inode(struct inode *);
extern int ext4_change_inode_journal_flag(struct inode *, int);
extern int ext4_get_inode_loc(struct inode *, struct ext4_iloc *);
+extern int ext4_can_truncate(struct inode *inode);
extern void ext4_truncate (struct inode *);
extern void ext4_set_inode_flags(struct inode *);
extern void ext4_get_inode_flags(struct ext4_inode_info *);
diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c
index c6efbab..11cafe1 100644
--- a/fs/ext4/ialloc.c
+++ b/fs/ext4/ialloc.c
@@ -817,6 +817,14 @@ struct inode *ext4_orphan_get(struct super_block *sb, unsigned long ino)
if (IS_ERR(inode))
goto iget_failed;

+ /*
+ * If the orphans has i_nlinks > 0 then it should be able to be
+ * truncated, otherwise it won't be removed from the orphan list
+ * during processing and an infinite loop will result.
+ */
+ if (inode->i_nlink && !ext4_can_truncate(inode))
+ goto bad_orphan;
+
if (NEXT_ORPHAN(inode) > max_ino)
goto bad_orphan;
brelse(bitmap_bh);
@@ -838,6 +846,7 @@ bad_orphan:
printk(KERN_NOTICE "NEXT_ORPHAN(inode)=%u\n",
NEXT_ORPHAN(inode));
printk(KERN_NOTICE "max_ino=%lu\n", max_ino);
+ printk(KERN_NOTICE "i_nlink=%u\n", inode->i_nlink);
/* Avoid freeing blocks if we got a bad deleted inode */
if (inode->i_nlink == 0)
inode->i_blocks = 0;
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 8d97077..269763b 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -2305,6 +2305,19 @@ static void ext4_free_branches(handle_t *handle, struct inode *inode,
}
}

+int ext4_can_truncate(struct inode *inode)
+{
+ if (IS_APPEND(inode) || IS_IMMUTABLE(inode))
+ return 0;
+ if (S_ISREG(inode->i_mode))
+ return 1;
+ if (S_ISDIR(inode->i_mode))
+ return 1;
+ if (S_ISLNK(inode->i_mode))
+ return !ext4_inode_is_fast_symlink(inode);
+ return 0;
+}
+
/*
* ext4_truncate()
*
@@ -2349,12 +2362,7 @@ void ext4_truncate(struct inode *inode)
unsigned blocksize = inode->i_sb->s_blocksize;
struct page *page;

- if (!(S_ISREG(inode->i_mode) || S_ISDIR(inode->i_mode) ||
- S_ISLNK(inode->i_mode)))
- return;
- if (ext4_inode_is_fast_symlink(inode))
- return;
- if (IS_APPEND(inode) || IS_IMMUTABLE(inode))
+ if (!ext4_can_truncate(inode))
return;

/*


2008-07-08 14:42:07

by Duane Griffin

[permalink] [raw]
Subject: [PATCH] ext4: handle deleting corrupted indirect blocks

While freeing indirect blocks we attach a journal head to the parent buffer
head, free the blocks, then journal the parent. If the indirect block list
is corrupted and points to the parent the journal head will be detached
when the block is cleared, causing an OOPS.

Check for that explicitly and handle it gracefully.

This patch fixes the third case (image hdb.20000057.nullderef.gz)
reported in http://bugzilla.kernel.org/show_bug.cgi?id=10882.

Signed-off-by: Duane Griffin <[email protected]>
--

This is the ext4 version of an ext3 patch queued in -mm.
---

diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 8d97077..f3cd914 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -2179,7 +2179,20 @@ static void ext4_free_data(handle_t *handle, struct inode *inode,

if (this_bh) {
BUFFER_TRACE(this_bh, "call ext4_journal_dirty_metadata");
- ext4_journal_dirty_metadata(handle, this_bh);
+
+ /*
+ * The buffer head should have an attached journal head at this
+ * point. However, if the data is corrupted and an indirect
+ * block pointed to itself, it would have been detached when
+ * the block was cleared. Check for this instead of OOPSing.
+ */
+ if (bh2jh(this_bh))
+ ext4_journal_dirty_metadata(handle, this_bh);
+ else
+ ext4_error(inode->i_sb, __func__,
+ "circular indirect block detected, "
+ "inode=%lu, block=%lu",
+ inode->i_ino, this_bh->b_blocknr);
}
}


2008-07-08 14:42:09

by Duane Griffin

[permalink] [raw]
Subject: [PATCH] ext4: validate directory entry data before use

ext4_dx_find_entry uses ext4_next_entry without verifying that the entry is
valid. If its rec_len == 0 this causes an infinite loop. Refactor the loop
to check the validity of entries before checking whether they match and
moving onto the next one.

There are other uses of ext4_next_entry in this file which also look
problematic. They should be reviewed and fixed if/when we have a test-case
that triggers them.

This patch fixes the first case (image hdb.25.softlockup.gz) reported in
http://bugzilla.kernel.org/show_bug.cgi?id=10882.

Signed-off-by: Duane Griffin <[email protected]>
--

This is an ext4 version of an ext3 patch queued in -mm.
---

diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
index ab16bea..384f122 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -993,19 +993,21 @@ static struct buffer_head * ext4_dx_find_entry(struct dentry *dentry,
de = (struct ext4_dir_entry_2 *) bh->b_data;
top = (struct ext4_dir_entry_2 *) ((char *) de + sb->s_blocksize -
EXT4_DIR_REC_LEN(0));
- for (; de < top; de = ext4_next_entry(de))
- if (ext4_match (namelen, name, de)) {
- if (!ext4_check_dir_entry("ext4_find_entry",
- dir, de, bh,
- (block<<EXT4_BLOCK_SIZE_BITS(sb))
- +((char *)de - bh->b_data))) {
- brelse (bh);
+ for (; de < top; de = ext4_next_entry(de)) {
+ int off = (block << EXT4_BLOCK_SIZE_BITS(sb))
+ + ((char *) de - bh->b_data);
+
+ if (!ext4_check_dir_entry(__func__, dir, de, bh, off)) {
+ brelse(bh);
*err = ERR_BAD_DX_DIR;
goto errout;
}
- *res_dir = de;
- dx_release (frames);
- return bh;
+
+ if (ext4_match(namelen, name, de)) {
+ *res_dir = de;
+ dx_release(frames);
+ return bh;
+ }
}
brelse (bh);
/* Check to see if we should continue to search */

2008-07-09 02:53:04

by Theodore Ts'o

[permalink] [raw]
Subject: Re: [PATCH] ext4: handle deleting corrupted indirect blocks

On Tue, Jul 08, 2008 at 03:42:00PM +0100, Duane Griffin wrote:
> + if (bh2jh(this_bh))
> + ext4_journal_dirty_metadata(handle, this_bh);
> + else
> + ext4_error(inode->i_sb, __func__,
> + "circular indirect block detected, "
> + "inode=%lu, block=%lu",
> + inode->i_ino, this_bh->b_blocknr);

this_bh->b_blocknr is a sector_t, which could be 64-bits. So this
should be:

ext4_error(inode->i_sb, __func__,
"circular indirect block detected, "
"inode=%lu, block=%llu",
inode->i_ino,
(unsigned long long) this_bh->b_blocknr);

I think.

With this change, I've included your three patches into the ext4 patch
queue.

- Ted