diff mbox

[1/5] ext4: Error out if verifying the block bitmap fails

Message ID 20130719235539.24017.94739.stgit@blackbox.djwong.org
State Accepted, archived
Headers show

Commit Message

Darrick Wong July 19, 2013, 11:55 p.m. UTC
The block bitmap verification code assumes that calling ext4_error() either
panics the system or makes the fs readonly.  However, this is not always true:
when 'errors=continue' is specified, an error is printed but we don't return
any indication of error to the caller, which is (probably) the block allocator,
which pretends that the crud we read in off the disk is a usable bitmap.  Yuck.

A block bitmap that fails the check should at least return no bitmap to the
caller.  The block allocator should be told to go look in a different group,
but that's a separate issue.

The easiest way to reproduce this is to modify bg_block_bitmap (on a ^flex_bg
fs) to point to a block outside the block group; or you can create a
metadata_csum filesystem and zero out the block bitmaps.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/ext4/balloc.c |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)



--
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

Theodore Ts'o Aug. 28, 2013, 7:36 p.m. UTC | #1
On Fri, Jul 19, 2013 at 04:55:39PM -0700, Darrick J. Wong wrote:
> The block bitmap verification code assumes that calling ext4_error() either
> panics the system or makes the fs readonly.  However, this is not always true:
> when 'errors=continue' is specified, an error is printed but we don't return
> any indication of error to the caller, which is (probably) the block allocator,
> which pretends that the crud we read in off the disk is a usable bitmap.  Yuck.
> 
> A block bitmap that fails the check should at least return no bitmap to the
> caller.  The block allocator should be told to go look in a different group,
> but that's a separate issue.
> 
> The easiest way to reproduce this is to modify bg_block_bitmap (on a ^flex_bg
> fs) to point to a block outside the block group; or you can create a
> metadata_csum filesystem and zero out the block bitmaps.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

Thanks, applied.

					- Ted
--
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/balloc.c b/fs/ext4/balloc.c
index ddd715e..b430afe 100644
--- a/fs/ext4/balloc.c
+++ b/fs/ext4/balloc.c
@@ -445,7 +445,10 @@  ext4_read_block_bitmap_nowait(struct super_block *sb, ext4_group_t block_group)
 	return bh;
 verify:
 	ext4_validate_block_bitmap(sb, desc, block_group, bh);
-	return bh;
+	if (buffer_verified(bh))
+		return bh;
+	put_bh(bh);
+	return NULL;
 }
 
 /* Returns 0 on success, 1 on error */
@@ -469,7 +472,8 @@  int ext4_wait_block_bitmap(struct super_block *sb, ext4_group_t block_group,
 	clear_buffer_new(bh);
 	/* Panic or remount fs read-only if block bitmap is invalid */
 	ext4_validate_block_bitmap(sb, desc, block_group, bh);
-	return 0;
+	/* ...but check for error just in case errors=continue. */
+	return !buffer_verified(bh);
 }
 
 struct buffer_head *