ext2/3/4 block bitmap validation patches
Subsystems (3)
This is the updated ext2/3/4 block bitmap validation patches
Changes from the last post
a) moved the bh_uptodate_or_lock and bh_submit_read to fs/buffer.c and added EXPORT_SYMBOL
b) Updated bh_submit_read not to release buffer on failure. This handles one reference handling bug in the error patch.
PATCH 1/1 ext2: add block bitmap validation
When a new block bitmap is read from disk in read_block_bitmap() there are a few bits that should ALWAYS be set. In particular, the blocks given corresponding to block bitmap, inode bitmap and inode tables. Validate the block bitmap against these blocks.
--- fs/ext2/balloc.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++++------ 1 files changed, 72 insertions(+), 9 deletions(-)
fs/ext2/balloc.c +72 -9
@@ -69,9 +69,53 @@ struct ext2_group_desc * ext2_get_group_desc(struct super_block * sb,return desc + offset; } +static int ext2_valid_block_bitmap (struct super_block *sb, +struct ext2_group_desc *desc, +unsigned int block_group, +struct buffer_head *bh) +{ +ext2_grpblk_t offset; +ext2_grpblk_t next_zero_bit; +ext2_fsblk_t bitmap_blk; +ext2_fsblk_t group_first_block; + + group_first_block =ext2_group_first_block_no (sb, block_group); + +/* check whether block bitmap block number is set */ + bitmap_blk =le32_to_cpu (desc->bg_block_bitmap); + offset = bitmap_blk - group_first_block; +if (!ext2_test_bit (offset, bh->b_data)) +/* bad block bitmap */ + goto err_out; + +/* check whether the inode bitmap block number is set */ + bitmap_blk =le32_to_cpu (desc->bg_inode_bitmap); + offset = bitmap_blk - group_first_block; +if (!ext2_test_bit (offset, bh->b_data)) +/* bad block bitmap */ + goto err_out; + +/* check whether the inode table block number is set */ + bitmap_blk =le32_to_cpu (desc->bg_inode_table); + offset = bitmap_blk - group_first_block; + next_zero_bit =ext2_find_next_zero_bit (bh->b_data, + offset +EXT2_SB (sb)->s_itb_per_group, + offset); +if (next_zero_bit >= offset +EXT2_SB (sb)->s_itb_per_group) +/* good bitmap for inode tables */ +return 1 ; + +err_out: +ext2_error (sb, __FUNCTION__, +"Invalid block bitmap - " +"block_group = %d, block = %lu" , + block_group, bitmap_blk); +return 0 ; +} +/* -* Read the bitmap for a given block_group, reading into the specified -* slot in the superblock's bitmap cache. +* Read the bitmap for a given block_group,and validate the +* bits for block/inode/inode tables are set in the bitmaps * * Return buffer_head on success or NULL in case of failure. */
@@ -80,17 +124,36 @@ read_block_bitmap(struct super_block *sb, unsigned int block_group) {struct ext2_group_desc * desc;struct buffer_head * bh =NULL ; - - desc =ext2_get_group_desc (sb, block_group,NULL ); +ext2_fsblk_t bitmap_blk; + + desc =ext2_get_group_desc (sb, block_group,NULL );if (!desc) - goto error_out; - bh =sb_bread (sb,le32_to_cpu (desc->bg_block_bitmap)); -if (!bh) -ext2_error (sb,"read_block_bitmap" , +return NULL ; + bitmap_blk =le32_to_cpu (desc->bg_block_bitmap); + bh =sb_getblk (sb, bitmap_blk); +if (unlikely (!bh)) { +ext2_error (sb, __FUNCTION__,"Cannot read block bitmap - " "block_group = %d, block_bitmap = %u" , block_group,le32_to_cpu (desc->bg_block_bitmap)); -error_out: +return NULL ; + } +if (likely (bh_uptodate_or_lock (bh))) +return bh; + +if (bh_submit_read (bh) <0 ) { +brelse (bh); +ext2_error (sb, __FUNCTION__, +"Cannot read block bitmap - " +"block_group = %d, block_bitmap = %u" , + block_group,le32_to_cpu (desc->bg_block_bitmap)); +return NULL ; + } +if (!ext2_valid_block_bitmap (sb, desc, block_group, bh)) { +brelse (bh); +return NULL ; + } +return bh; }
Signature · 1 line
1.5.3.5.652.gf192c-dirty
PATCH 1/1 ext3: add block bitmap validation
In reply to Aneesh Kumar K.VWhen a new block bitmap is read from disk in read_block_bitmap() there are a few bits that should ALWAYS be set. In particular, the blocks given corresponding to block bitmap, inode bitmap and inode tables. Validate the block bitmap against these blocks.
--- fs/ext3/balloc.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++++----- 1 files changed, 70 insertions(+), 8 deletions(-)
fs/ext3/balloc.c +70 -8
@@ -80,13 +80,57 @@ struct ext3_group_desc * ext3_get_group_desc(struct super_block * sb,return desc + offset; } +static int ext3_valid_block_bitmap (struct super_block *sb, +struct ext3_group_desc *desc, +unsigned int block_group, +struct buffer_head *bh) +{ +ext3_grpblk_t offset; +ext3_grpblk_t next_zero_bit; +ext3_fsblk_t bitmap_blk; +ext3_fsblk_t group_first_block; + + group_first_block =ext3_group_first_block_no (sb, block_group); + +/* check whether block bitmap block number is set */ + bitmap_blk =le32_to_cpu (desc->bg_block_bitmap); + offset = bitmap_blk - group_first_block; +if (!ext3_test_bit (offset, bh->b_data)) +/* bad block bitmap */ + goto err_out; + +/* check whether the inode bitmap block number is set */ + bitmap_blk =le32_to_cpu (desc->bg_inode_bitmap); + offset = bitmap_blk - group_first_block; +if (!ext3_test_bit (offset, bh->b_data)) +/* bad block bitmap */ + goto err_out; + +/* check whether the inode table block number is set */ + bitmap_blk =le32_to_cpu (desc->bg_inode_table); + offset = bitmap_blk - group_first_block; + next_zero_bit =ext3_find_next_zero_bit (bh->b_data, + offset +EXT3_SB (sb)->s_itb_per_group, + offset); +if (next_zero_bit >= offset +EXT3_SB (sb)->s_itb_per_group) +/* good bitmap for inode tables */ +return 1 ; + +err_out: +ext3_error (sb, __FUNCTION__, +"Invalid block bitmap - " +"block_group = %d, block = %lu" , + block_group, bitmap_blk); +return 0 ; +} +/** * read_block_bitmap() * @sb: super block * @block_group: given block group * -* Read the bitmap for a given block_group, reading into the specified -* slot in the superblock's bitmap cache. +* Read the bitmap for a given block_group,and validate the +* bits for block/inode/inode tables are set in the bitmaps * * Return buffer_head on success or NULL in case of failure. */
@@ -95,17 +139,35 @@ read_block_bitmap(struct super_block *sb, unsigned int block_group) {struct ext3_group_desc * desc;struct buffer_head * bh =NULL ; +ext3_fsblk_t bitmap_blk; - desc =ext3_get_group_desc (sb, block_group,NULL ); + desc =ext3_get_group_desc (sb, block_group,NULL );if (!desc) - goto error_out; - bh =sb_bread (sb,le32_to_cpu (desc->bg_block_bitmap)); -if (!bh) -ext3_error (sb,"read_block_bitmap" , +return NULL ; + bitmap_blk =le32_to_cpu (desc->bg_block_bitmap); + bh =sb_getblk (sb, bitmap_blk); +if (unlikely (!bh)) { +ext3_error (sb, __FUNCTION__,"Cannot read block bitmap - " "block_group = %d, block_bitmap = %u" , block_group,le32_to_cpu (desc->bg_block_bitmap)); -error_out: +return NULL ; + } +if (likely (bh_uptodate_or_lock (bh))) +return bh; + +if (bh_submit_read (bh) <0 ) { +brelse (bh); +ext3_error (sb, __FUNCTION__, +"Cannot read block bitmap - " +"block_group = %d, block_bitmap = %u" , + block_group,le32_to_cpu (desc->bg_block_bitmap)); +return NULL ; + } +if (!ext3_valid_block_bitmap (sb, desc, block_group, bh)) { +brelse (bh); +return NULL ; + }return bh; }/*
Signature · 1 line
1.5.3.5.652.gf192c-dirty
PATCH 1/1 ext4: add block bitmap validation
In reply to Aneesh Kumar K.VWhen a new block bitmap is read from disk in read_block_bitmap() there are a few bits that should ALWAYS be set. In particular, the blocks given corresponding to block bitmap, inode bitmap and inode tables. Validate the block bitmap against these blocks.
--- fs/ext4/balloc.c | 97 ++++++++++++++++++++++++++++++++++++++++++++--------- 1 files changed, 80 insertions(+), 17 deletions(-)
fs/ext4/balloc.c +80 -17
@@ -189,13 +189,65 @@ struct ext4_group_desc * ext4_get_group_desc(struct super_block * sb,return desc; } +static int ext4_valid_block_bitmap (struct super_block *sb, +struct ext4_group_desc *desc, +unsigned int block_group, +struct buffer_head *bh) +{ +ext4_grpblk_t offset; +ext4_grpblk_t next_zero_bit; +ext4_fsblk_t bitmap_blk; +ext4_fsblk_t group_first_block; + +if (EXT4_HAS_INCOMPAT_FEATURE (sb,EXT4_FEATURE_INCOMPAT_FLEX_BG )) { +/* with FLEX_BG, the inode/block bitmaps and itable +* blocks may not be in the group at all +* so the bitmap validation will be skipped for those groups +* or it has to also read the block group where the bitmaps +* are located to verify they are set. +*/ +return 1 ; + } + group_first_block =ext4_group_first_block_no (sb, block_group); + +/* check whether block bitmap block number is set */ + bitmap_blk =ext4_block_bitmap (sb, desc); + offset = bitmap_blk - group_first_block; +if (!ext4_test_bit (offset, bh->b_data)) +/* bad block bitmap */ + goto err_out; + +/* check whether the inode bitmap block number is set */ + bitmap_blk =ext4_inode_bitmap (sb, desc); + offset = bitmap_blk - group_first_block; +if (!ext4_test_bit (offset, bh->b_data)) +/* bad block bitmap */ + goto err_out; + +/* check whether the inode table block number is set */ + bitmap_blk =ext4_inode_table (sb, desc); + offset = bitmap_blk - group_first_block; + next_zero_bit =ext4_find_next_zero_bit (bh->b_data, + offset +EXT4_SB (sb)->s_itb_per_group, + offset); +if (next_zero_bit >= offset +EXT4_SB (sb)->s_itb_per_group) +/* good bitmap for inode tables */ +return 1 ; + +err_out: +ext4_error (sb, __FUNCTION__, +"Invalid block bitmap - " +"block_group = %d, block = %llu" , + block_group, bitmap_blk); +return 0 ; +}/** * read_block_bitmap() * @sb: super block * @block_group: given block group * -* Read the bitmap for a given block_group, reading into the specified -* slot in the superblock's bitmap cache. +* Read the bitmap for a given block_group,and validate the +* bits for block/inode/inode tables are set in the bitmaps * * Return buffer_head on success or NULL in case of failure. */
@@ -210,25 +262,36 @@ read_block_bitmap(struct super_block *sb, ext4_group_t block_group)if (!desc)return NULL ; bitmap_blk =ext4_block_bitmap (sb, desc); + bh =sb_getblk (sb, bitmap_blk); +if (unlikely (!bh)) { +ext4_error (sb, __FUNCTION__, +"Cannot read block bitmap - " +"block_group = %d, block_bitmap = %llu" , + block_group, bitmap_blk); +return NULL ; + } +if (bh_uptodate_or_lock (bh)) +return bh; +if (desc->bg_flags &cpu_to_le16 (EXT4_BG_BLOCK_UNINIT )) { - bh =sb_getblk (sb, bitmap_blk); -if (!buffer_uptodate (bh)) { -lock_buffer (bh); -if (!buffer_uptodate (bh)) { -ext4_init_block_bitmap (sb, bh, block_group, - desc); -set_buffer_uptodate (bh); - } -unlock_buffer (bh); - } - }else { - bh =sb_bread (sb, bitmap_blk); +ext4_init_block_bitmap (sb, bh, block_group, desc); +set_buffer_uptodate (bh); +unlock_buffer (bh); +return bh; } -if (!bh) -ext4_error (sb, __FUNCTION__, +if (bh_submit_read (bh) <0 ) { +brelse (bh); +ext4_error (sb, __FUNCTION__,"Cannot read block bitmap - " -"block_group = %lu, block_bitmap = %llu" , +"block_group = %d, block_bitmap = %llu" , block_group, bitmap_blk); +return NULL ; + } +if (!ext4_valid_block_bitmap (sb, desc, block_group, bh)) { +brelse (bh); +return NULL ; + } +return bh; }/*
Signature · 1 line
1.5.3.5.652.gf192c-dirty
Re: [PATCH] ext4: add block bitmap validation
Quoted 4 lines · Aneesh Kumar K.V
fs/ext4/balloc.c +4
-0+static int ext4_valid_block_bitmap (struct super_block *sb, +struct ext4_group_desc *desc, +unsigned int block_group, +struct buffer_head *bh)
fs/ext4/balloc.c struct ext4_group_desc * ext4_get_group_desc(struct super_block * sb, · line 210
Quoted 10 lines
fs/ext4/balloc.c +10
-0+{ +if (EXT4_HAS_INCOMPAT_FEATURE (sb,EXT4_FEATURE_INCOMPAT_FLEX_BG )) { +/* with FLEX_BG, the inode/block bitmaps and itable +* blocks may not be in the group at all +* so the bitmap validation will be skipped for those groups +* or it has to also read the block group where the bitmaps +* are located to verify they are set. +*/ +return 1 ; + }
Rather than skipping the bitmap check entirely when FLEX_BG is enabled, it is probably better to just validate whether each of the bitmap/itable offsets are within the bitmap being read, like:
offset = ... if (offset < EXT4_BLOCKS_PER_GROUP(sb) && ext4_test_bit(offset, bh->b_data)) /* bad block bitmap */ goto err_out;
Cheers, Andreas
--
Andreas Dilger
Sr. Software Engineer, Lustre Group
Sun Microsystems of Canada, Inc.
PATCH 1/1 Add buffer head related helper functions
In reply to Aneesh Kumar K.VAdd buffer head related helper functionbh_uptodate_or_lock and bh_submit_read
which can be used by file system
--- fs/buffer.c | 41 +++++++++++++++++++++++++++++++++++++++++ include/linux/buffer_head.h | 2 ++ 2 files changed, 43 insertions(+), 0 deletions(-)
fs/buffer.c +41 -0
@@ -3213,6 +3213,47 @@ static int buffer_cpu_notify(struct notifier_block *self,return NOTIFY_OK ; } +/** +* bh_uptodate_or_lock: Test whether the buffer is uptodate +* @bh: struct buffer_head +* +* Return true if the buffer is up-to-date and false, +* with the buffer locked, if not. +*/ +int bh_uptodate_or_lock (struct buffer_head *bh) +{ +if (!buffer_uptodate (bh)) { +lock_buffer (bh); +if (!buffer_uptodate (bh)) +return 0 ; +unlock_buffer (bh); + } +return 1 ; +} +EXPORT_SYMBOL (bh_uptodate_or_lock); +/** +* bh_submit_read: Submit a locked buffer for reading +* @bh: struct buffer_head +* +* Returns a negative error +*/ +int bh_submit_read (struct buffer_head *bh) +{ +if (!buffer_locked (bh)) +lock_buffer (bh); + +if (buffer_uptodate (bh)) +return 0 ; + +get_bh (bh); + bh->b_end_io = end_buffer_read_sync; +submit_bh (READ , bh); +wait_on_buffer (bh); +if (buffer_uptodate (bh)) +return 0 ; +return -EIO ; +} +EXPORT_SYMBOL (bh_submit_read);void __init buffer_init (void ) {int nrpages;
include/linux/buffer_head.h +2 -0
@@ -192,6 +192,8 @@ int sync_dirty_buffer(struct buffer_head *bh);int submit_bh (int ,struct buffer_head *);void write_boundary_block (struct block_device *bdev,sector_t bblock,unsigned blocksize); +int bh_uptodate_or_lock (struct buffer_head *bh); +int bh_submit_read (struct buffer_head *bh);extern int buffer_heads_over_limit;
Signature · 1 line
1.5.3.5.652.gf192c-dirty
Quoted 12 lines · Aneesh Kumar K.V
This is the updated ext2/3/4 block bitmap validation patches
Changes from the last post
a) moved the
bh_uptodate_or_lockandbh_submit_readtofs/buffer.cand addedEXPORT_SYMBOLb) Updated
bh_submit_readnot to release buffer on failure. This handles one reference handling bug in the error patch.
I forgot to pass --numbered to git format-patch The order is
[PATCH 1/4] Add buffer head related helper functions [PATCH 2/4] ext2: add block bitmap validation [PATCH 3/4] ext3: add block bitmap validation [PATCH 4/4] ext4: add block bitmap validation
it is against patch queue and applies as the last set of patches in the stable series
....
buffer_head_helper.patch
ext2_bitmap_validation.patch
ext3_bitmap_validation.patch
ext4_bitmap_validation.patch
stable-boundary
-aneesh