[PATCH] don't allocate/free blocks in system areas

From: Andreas Dilger (adilger@clusterfs.com)
Date: Mon Apr 07 2003 - 17:21:33 EST


In case of on-disk corruption, the current ext2/3 code will happily reallocate
or mark free blocks in the system area (bitmaps, inode table, etc) instead of
ignoring the bad alloc/free and leaving the system blocks alone. This will
cause further corruption of the filesystem as metadata gets overwritten by
file data and in turn causes more bad alloc/free requests.

This patch was (I thought at least) in -ac and scheduled to go into mainline
a long time ago (over a year ago, looking at the dates in the diff), but it
seems to have been lost. The patch still applies to 2.4.21-pre7.

A similar patch is needed for 2.5, but there are subtle differences in the
2.5 ext2 code and I haven't been using 2.5...

Cheers, Andreas
======================= ext2-2.4.18-badalloc.diff ===========================
diff -ru linux-2.4.18.orig/fs/ext2/balloc.c linux-2.4.18-aed/fs/ext2/balloc.c
--- linux-2.4.18.orig/fs/ext2/balloc.c Wed Feb 27 10:31:58 2002
+++ linux-2.4.18-aed/fs/ext2/balloc.c Mon Mar 18 17:07:55 2002
@@ -269,7 +269,8 @@
         }
         lock_super (sb);
         es = sb->u.ext2_sb.s_es;
- if (block < le32_to_cpu(es->s_first_data_block) ||
+ if (block < le32_to_cpu(es->s_first_data_block) ||
+ block + count < block ||
             (block + count) > le32_to_cpu(es->s_blocks_count)) {
                 ext2_error (sb, "ext2_free_blocks",
                             "Freeing blocks not in datazone - "
@@ -302,22 +303,20 @@
         if (!gdp)
                 goto error_return;
 
- if (in_range (le32_to_cpu(gdp->bg_block_bitmap), block, count) ||
- in_range (le32_to_cpu(gdp->bg_inode_bitmap), block, count) ||
- in_range (block, le32_to_cpu(gdp->bg_inode_table),
- sb->u.ext2_sb.s_itb_per_group) ||
- in_range (block + count - 1, le32_to_cpu(gdp->bg_inode_table),
- sb->u.ext2_sb.s_itb_per_group))
- ext2_error (sb, "ext2_free_blocks",
- "Freeing blocks in system zones - "
- "Block = %lu, count = %lu",
- block, count);
+ for (i = 0; i < count; i++, block++) {
+ if (block == le32_to_cpu(gdp->bg_block_bitmap) ||
+ block == le32_to_cpu(gdp->bg_inode_bitmap) ||
+ in_range(block, le32_to_cpu(gdp->bg_inode_table),
+ EXT2_SB(sb)->s_itb_per_group)) {
+ ext2_error(sb, __FUNCTION__,
+ "Freeing block in system zone - block = %lu",
+ block);
+ continue;
+ }
 
- for (i = 0; i < count; i++) {
                 if (!ext2_clear_bit (bit + i, bh->b_data))
- ext2_error (sb, "ext2_free_blocks",
- "bit already cleared for block %lu",
- block + i);
+ ext2_error(sb, __FUNCTION__,
+ "bit already cleared for block %lu", block);
                 else {
                         DQUOT_FREE_BLOCK(inode, 1);
                         gdp->bg_free_blocks_count =
@@ -336,7 +335,6 @@
                 wait_on_buffer (bh);
         }
         if (overflow) {
- block += count;
                 count = overflow;
                 goto do_more;
         }
@@ -522,10 +522,14 @@
        if (tmp == le32_to_cpu(gdp->bg_block_bitmap) ||
            tmp == le32_to_cpu(gdp->bg_inode_bitmap) ||
             in_range (tmp, le32_to_cpu(gdp->bg_inode_table),
- sb->u.ext2_sb.s_itb_per_group))
+ EXT2_SB(sb)->s_itb_per_group)) {
                 ext2_error (sb, "ext2_new_block",
- "Allocating block in system zone - "
- "block = %u", tmp);
+ "Allocating block in system zone - block = %lu",
+ tmp);
+ ext2_set_bit(j, bh->b_data);
+ DQUOT_FREE_BLOCK(inode, 1);
+ goto repeat;
+ }
 
         if (ext2_set_bit (j, bh->b_data)) {
                 ext2_warning (sb, "ext2_new_block",
--- linux-2.4.18.orig/fs/ext3/balloc.c Wed Feb 27 10:31:59 2002
+++ linux-2.4.18-aed/fs/ext3/balloc.c Mon Mar 18 17:15:46 2002
@@ -276,7 +273,8 @@
         }
         lock_super (sb);
         es = sb->u.ext3_sb.s_es;
- if (block < le32_to_cpu(es->s_first_data_block) ||
+ if (block < le32_to_cpu(es->s_first_data_block) ||
+ block + count < block ||
             (block + count) > le32_to_cpu(es->s_blocks_count)) {
                 ext3_error (sb, "ext3_free_blocks",
                             "Freeing blocks not in datazone - "
@@ -309,17 +307,6 @@
         if (!gdp)
                 goto error_return;
 
- if (in_range (le32_to_cpu(gdp->bg_block_bitmap), block, count) ||
- in_range (le32_to_cpu(gdp->bg_inode_bitmap), block, count) ||
- in_range (block, le32_to_cpu(gdp->bg_inode_table),
- sb->u.ext3_sb.s_itb_per_group) ||
- in_range (block + count - 1, le32_to_cpu(gdp->bg_inode_table),
- sb->u.ext3_sb.s_itb_per_group))
- ext3_error (sb, "ext3_free_blocks",
- "Freeing blocks in system zones - "
- "Block = %lu, count = %lu",
- block, count);
-
         /*
          * We are about to start releasing blocks in the bitmap,
          * so we need undo access.
@@ -345,14 +332,24 @@
         if (err)
                 goto error_return;
 
- for (i = 0; i < count; i++) {
+ for (i = 0; i < count; i++, block++) {
+ if (block == le32_to_cpu(gdp->bg_block_bitmap) ||
+ block == le32_to_cpu(gdp->bg_inode_bitmap) ||
+ in_range(block, le32_to_cpu(gdp->bg_inode_table),
+ EXT3_SB(sb)->s_itb_per_group)) {
+ ext3_error(sb, __FUNCTION__,
+ "Freeing block in system zone - block = %lu",
+ block);
+ continue;
+ }
+
                 /*
                  * An HJ special. This is expensive...
                  */
 #ifdef CONFIG_JBD_DEBUG
                 {
                         struct buffer_head *debug_bh;
- debug_bh = sb_get_hash_table(sb, block + i);
+ debug_bh = sb_get_hash_table(sb, block);
                         if (debug_bh) {
                                 BUFFER_TRACE(debug_bh, "Deleted!");
                                 if (!bh2jh(bitmap_bh)->b_committed_data)
@@ -365,9 +362,8 @@
 #endif
                 BUFFER_TRACE(bitmap_bh, "clear bit");
                 if (!ext3_clear_bit (bit + i, bitmap_bh->b_data)) {
- ext3_error (sb, __FUNCTION__,
- "bit already cleared for block %lu",
- block + i);
+ ext3_error(sb, __FUNCTION__,
+ "bit already cleared for block %lu", block);
                         BUFFER_TRACE(bitmap_bh, "bit already cleared");
                 } else {
                         dquot_freed_blocks++;
@@ -415,7 +411,6 @@
         if (!err) err = ret;
 
         if (overflow && !err) {
- block += count;
                 count = overflow;
                 goto do_more;
         }
@@ -575,6 +574,7 @@
 
         ext3_debug ("goal=%lu.\n", goal);
 
+repeat:
         /*
          * First, test whether the goal block is free.
          */
@@ -684,10 +684,21 @@
         if (tmp == le32_to_cpu(gdp->bg_block_bitmap) ||
             tmp == le32_to_cpu(gdp->bg_inode_bitmap) ||
             in_range (tmp, le32_to_cpu(gdp->bg_inode_table),
- sb->u.ext3_sb.s_itb_per_group))
- ext3_error (sb, "ext3_new_block",
- "Allocating block in system zone - "
- "block = %u", tmp);
+ EXT3_SB(sb)->s_itb_per_group)) {
+ ext3_error(sb, __FUNCTION__,
+ "Allocating block in system zone - block = %u", tmp);
+
+ /* Note: This will potentially use up one of the handle's
+ * buffer credits. Normally we have way too many credits,
+ * so that is OK. In _very_ rare cases it might not be OK.
+ * We will trigger an assertion if we run out of credits,
+ * and we will have to do a full fsck of the filesystem -
+ * better than randomly corrupting filesystem metadata.
+ */
+ ext3_set_bit(j, bh->b_data);
+ goto repeat;
+ }
+
 
         /* The superblock lock should guard against anybody else beating
          * us to this point! */

--
Andreas Dilger
http://sourceforge.net/projects/ext2resize/
http://www-mddsp.enel.ucalgary.ca/People/adilger/

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



This archive was generated by hypermail 2b29 : Mon Apr 07 2003 - 22:00:32 EST