* [PATCH v1 0/2] ext4: Shut down block groups when damage is detected, part 2
@ 2013-09-27 0:03 Darrick J. Wong
2013-09-27 0:03 ` [PATCH 1/2] ext4: Don't count free clusters from a corrupt block group Darrick J. Wong
2013-09-27 0:03 ` [PATCH 2/2] ext4: Spot-check block group sub-table locations Darrick J. Wong
0 siblings, 2 replies; 7+ messages in thread
From: Darrick J. Wong @ 2013-09-27 0:03 UTC (permalink / raw)
To: tytso, darrick.wong; +Cc: linux-ext4
This patchset adds in a few things that weren't included in the original block
group shutdown patches.
The first prevents corrupt block groups from contributing to the free block
summary count, since by definition corrupt block groups have no free blocks.
The second patch performs a quick sanity check of bitmap and inode table block
numbers when loading them in from disk. These are the same checks that are
done at mount time; this patch guards against either deliberate sabotage or
memory corruption. Since we're about to do a slow disk read anyway, we might
as well take the time to double check for obvious brokenness.
This patchset has been tested (albeit lightly) against 3.12-rc2 on x64.
Comments and questions are, as always, welcome.
--D
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] ext4: Don't count free clusters from a corrupt block group
2013-09-27 0:03 [PATCH v1 0/2] ext4: Shut down block groups when damage is detected, part 2 Darrick J. Wong
@ 2013-09-27 0:03 ` Darrick J. Wong
2013-09-27 5:37 ` Zheng Liu
2013-09-27 0:03 ` [PATCH 2/2] ext4: Spot-check block group sub-table locations Darrick J. Wong
1 sibling, 1 reply; 7+ messages in thread
From: Darrick J. Wong @ 2013-09-27 0:03 UTC (permalink / raw)
To: tytso, darrick.wong; +Cc: linux-ext4
A bg that's been flagged "corrupt" by definition has no free blocks, so that
the allocator won't be tempted to use the damaged bg. Therefore, we shouldn't
count the clusters in the damaged group when calculating free counts.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
fs/ext4/balloc.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/fs/ext4/balloc.c b/fs/ext4/balloc.c
index dc5d572..4390f9f 100644
--- a/fs/ext4/balloc.c
+++ b/fs/ext4/balloc.c
@@ -640,6 +640,7 @@ ext4_fsblk_t ext4_count_free_clusters(struct super_block *sb)
struct ext4_group_desc *gdp;
ext4_group_t i;
ext4_group_t ngroups = ext4_get_groups_count(sb);
+ struct ext4_group_info *grp;
#ifdef EXT4FS_DEBUG
struct ext4_super_block *es;
ext4_fsblk_t bitmap_count;
@@ -679,7 +680,11 @@ ext4_fsblk_t ext4_count_free_clusters(struct super_block *sb)
gdp = ext4_get_group_desc(sb, i, NULL);
if (!gdp)
continue;
- desc_count += ext4_free_group_clusters(sb, gdp);
+ grp = NULL;
+ if (EXT4_SB(sb)->s_group_info)
+ grp = ext4_get_group_info(sb, i);
+ if (!grp || !EXT4_MB_GRP_BBITMAP_CORRUPT(grp))
+ desc_count += ext4_free_group_clusters(sb, gdp);
}
return desc_count;
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/2] ext4: Spot-check block group sub-table locations
2013-09-27 0:03 [PATCH v1 0/2] ext4: Shut down block groups when damage is detected, part 2 Darrick J. Wong
2013-09-27 0:03 ` [PATCH 1/2] ext4: Don't count free clusters from a corrupt block group Darrick J. Wong
@ 2013-09-27 0:03 ` Darrick J. Wong
2013-09-27 5:40 ` Zheng Liu
1 sibling, 1 reply; 7+ messages in thread
From: Darrick J. Wong @ 2013-09-27 0:03 UTC (permalink / raw)
To: tytso, darrick.wong; +Cc: linux-ext4
Perform a quick sanity check of bitmap and inode table block numbers when
loading them, and if there's something suspicious, mark the block group
corrupt.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
fs/ext4/balloc.c | 5 +++++
fs/ext4/ext4.h | 2 ++
fs/ext4/ialloc.c | 4 ++++
fs/ext4/inode.c | 6 ++++++
fs/ext4/super.c | 21 +++++++++++++++++++++
5 files changed, 38 insertions(+)
diff --git a/fs/ext4/balloc.c b/fs/ext4/balloc.c
index 4390f9f..3d79bfe 100644
--- a/fs/ext4/balloc.c
+++ b/fs/ext4/balloc.c
@@ -396,11 +396,16 @@ ext4_read_block_bitmap_nowait(struct super_block *sb, ext4_group_t block_group)
struct ext4_group_desc *desc;
struct buffer_head *bh;
ext4_fsblk_t bitmap_blk;
+ struct ext4_group_info *grp;
desc = ext4_get_group_desc(sb, block_group, NULL);
if (!desc)
return NULL;
bitmap_blk = ext4_block_bitmap(sb, desc);
+ if (!ext4_is_sane_bgdata_location(sb, block_group, bitmap_blk)) {
+ grp = ext4_get_group_info(sb, block_group);
+ set_bit(EXT4_GROUP_INFO_BBITMAP_CORRUPT_BIT, &grp->bb_state);
+ }
bh = sb_getblk(sb, bitmap_blk);
if (unlikely(!bh)) {
ext4_error(sb, "Cannot get buffer for block bitmap - "
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index af815ea..afb1bb2 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -2531,6 +2531,8 @@ static inline void ext4_unlock_group(struct super_block *sb,
spin_unlock(ext4_group_lock_ptr(sb, group));
}
+int ext4_is_sane_bgdata_location(struct super_block *sb, ext4_group_t grp,
+ ext4_fsblk_t blk);
/*
* Block validity checking
*/
diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c
index 137193f..242250d 100644
--- a/fs/ext4/ialloc.c
+++ b/fs/ext4/ialloc.c
@@ -122,6 +122,10 @@ ext4_read_inode_bitmap(struct super_block *sb, ext4_group_t block_group)
return NULL;
bitmap_blk = ext4_inode_bitmap(sb, desc);
+ if (!ext4_is_sane_bgdata_location(sb, block_group, bitmap_blk)) {
+ grp = ext4_get_group_info(sb, block_group);
+ set_bit(EXT4_GROUP_INFO_IBITMAP_CORRUPT_BIT, &grp->bb_state);
+ }
bh = sb_getblk(sb, bitmap_blk);
if (unlikely(!bh)) {
ext4_error(sb, "Cannot read inode bitmap - "
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 0d424d7..c5af22e 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -3795,6 +3795,7 @@ static int __ext4_get_inode_loc(struct inode *inode,
struct super_block *sb = inode->i_sb;
ext4_fsblk_t block;
int inodes_per_block, inode_offset;
+ struct ext4_group_info *grp;
iloc->bh = NULL;
if (!ext4_valid_inum(sb, inode->i_ino))
@@ -3814,6 +3815,11 @@ static int __ext4_get_inode_loc(struct inode *inode,
block = ext4_inode_table(sb, gdp) + (inode_offset / inodes_per_block);
iloc->offset = (inode_offset % inodes_per_block) * EXT4_INODE_SIZE(sb);
+ if (!ext4_is_sane_bgdata_location(sb, iloc->block_group, block)) {
+ grp = ext4_get_group_info(sb, iloc->block_group);
+ set_bit(EXT4_GROUP_INFO_IBITMAP_CORRUPT_BIT, &grp->bb_state);
+ return -EIO;
+ }
bh = sb_getblk(sb, block);
if (unlikely(!bh))
return -ENOMEM;
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 2c2e6cb..d22248e 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -2049,6 +2049,27 @@ void ext4_group_desc_csum_set(struct super_block *sb, __u32 block_group,
gdp->bg_checksum = ext4_group_desc_csum(EXT4_SB(sb), block_group, gdp);
}
+/* returns 1 if the location of a blockgroup data item seems sane */
+int ext4_is_sane_bgdata_location(struct super_block *sb, ext4_group_t grp,
+ ext4_fsblk_t blk)
+{
+ struct ext4_sb_info *sbi = EXT4_SB(sb);
+ ext4_fsblk_t first_block = le32_to_cpu(sbi->s_es->s_first_data_block);
+ ext4_fsblk_t last_block;
+
+ if (EXT4_HAS_INCOMPAT_FEATURE(sb, EXT4_FEATURE_INCOMPAT_FLEX_BG))
+ last_block = ext4_blocks_count(sbi->s_es) - 1;
+ else {
+ first_block += grp * sbi->s_blocks_per_group;
+ last_block = first_block + (EXT4_BLOCKS_PER_GROUP(sb) - 1);
+ }
+
+ if (last_block >= ext4_blocks_count(sbi->s_es))
+ last_block = ext4_blocks_count(sbi->s_es) - 1;
+
+ return blk >= first_block && blk <= last_block;
+}
+
/* Called at mount-time, super-block is locked */
static int ext4_check_descriptors(struct super_block *sb,
ext4_group_t *first_not_zeroed)
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] ext4: Don't count free clusters from a corrupt block group
2013-09-27 0:03 ` [PATCH 1/2] ext4: Don't count free clusters from a corrupt block group Darrick J. Wong
@ 2013-09-27 5:37 ` Zheng Liu
2013-09-27 18:03 ` Darrick J. Wong
0 siblings, 1 reply; 7+ messages in thread
From: Zheng Liu @ 2013-09-27 5:37 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: tytso, linux-ext4
On Thu, Sep 26, 2013 at 05:03:28PM -0700, Darrick J. Wong wrote:
> A bg that's been flagged "corrupt" by definition has no free blocks, so that
> the allocator won't be tempted to use the damaged bg. Therefore, we shouldn't
> count the clusters in the damaged group when calculating free counts.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
> fs/ext4/balloc.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
>
> diff --git a/fs/ext4/balloc.c b/fs/ext4/balloc.c
> index dc5d572..4390f9f 100644
> --- a/fs/ext4/balloc.c
> +++ b/fs/ext4/balloc.c
> @@ -640,6 +640,7 @@ ext4_fsblk_t ext4_count_free_clusters(struct super_block *sb)
> struct ext4_group_desc *gdp;
> ext4_group_t i;
> ext4_group_t ngroups = ext4_get_groups_count(sb);
> + struct ext4_group_info *grp;
> #ifdef EXT4FS_DEBUG
It seems that you forgot to add these code in this macro definition.
diff --git a/fs/ext4/balloc.c b/fs/ext4/balloc.c
index dc5d572..3ba2079 100644
--- a/fs/ext4/balloc.c
+++ b/fs/ext4/balloc.c
@@ -655,7 +655,11 @@ ext4_fsblk_t ext4_count_free_clusters(struct
super_block *sb)
gdp = ext4_get_group_desc(sb, i, NULL);
if (!gdp)
continue;
- desc_count += ext4_free_group_clusters(sb, gdp);
+ grp = NULL;
+ if (EXT4_SB(sb)->s_group_info)
+ grp = ext4_get_group_info(sb, i);
+ if (!grp || !EXT4_MB_GRP_BBITMAP_CORRUPT(grp))
+ desc_count += ext4_free_group_clusters(sb, gdp);
brelse(bitmap_bh);
bitmap_bh = ext4_read_block_bitmap(sb, i);
if (bitmap_bh == NULL)
- Zheng
> struct ext4_super_block *es;
> ext4_fsblk_t bitmap_count;
> @@ -679,7 +680,11 @@ ext4_fsblk_t ext4_count_free_clusters(struct super_block *sb)
> gdp = ext4_get_group_desc(sb, i, NULL);
> if (!gdp)
> continue;
> - desc_count += ext4_free_group_clusters(sb, gdp);
> + grp = NULL;
> + if (EXT4_SB(sb)->s_group_info)
> + grp = ext4_get_group_info(sb, i);
> + if (!grp || !EXT4_MB_GRP_BBITMAP_CORRUPT(grp))
> + desc_count += ext4_free_group_clusters(sb, gdp);
> }
>
> return desc_count;
>
> --
> 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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] ext4: Spot-check block group sub-table locations
2013-09-27 0:03 ` [PATCH 2/2] ext4: Spot-check block group sub-table locations Darrick J. Wong
@ 2013-09-27 5:40 ` Zheng Liu
2013-09-27 17:57 ` Darrick J. Wong
0 siblings, 1 reply; 7+ messages in thread
From: Zheng Liu @ 2013-09-27 5:40 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: tytso, linux-ext4
On Thu, Sep 26, 2013 at 05:03:34PM -0700, Darrick J. Wong wrote:
> Perform a quick sanity check of bitmap and inode table block numbers when
> loading them, and if there's something suspicious, mark the block group
> corrupt.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
> fs/ext4/balloc.c | 5 +++++
> fs/ext4/ext4.h | 2 ++
> fs/ext4/ialloc.c | 4 ++++
> fs/ext4/inode.c | 6 ++++++
> fs/ext4/super.c | 21 +++++++++++++++++++++
> 5 files changed, 38 insertions(+)
>
>
> diff --git a/fs/ext4/balloc.c b/fs/ext4/balloc.c
> index 4390f9f..3d79bfe 100644
> --- a/fs/ext4/balloc.c
> +++ b/fs/ext4/balloc.c
> @@ -396,11 +396,16 @@ ext4_read_block_bitmap_nowait(struct super_block *sb, ext4_group_t block_group)
> struct ext4_group_desc *desc;
> struct buffer_head *bh;
> ext4_fsblk_t bitmap_blk;
> + struct ext4_group_info *grp;
>
> desc = ext4_get_group_desc(sb, block_group, NULL);
> if (!desc)
> return NULL;
> bitmap_blk = ext4_block_bitmap(sb, desc);
> + if (!ext4_is_sane_bgdata_location(sb, block_group, bitmap_blk)) {
> + grp = ext4_get_group_info(sb, block_group);
> + set_bit(EXT4_GROUP_INFO_BBITMAP_CORRUPT_BIT, &grp->bb_state);
Why don't return NULL directly? If we found that the location of bitmap
is insane, we shouldn't read this bitmap.
- Zheng
> + }
> bh = sb_getblk(sb, bitmap_blk);
> if (unlikely(!bh)) {
> ext4_error(sb, "Cannot get buffer for block bitmap - "
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index af815ea..afb1bb2 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -2531,6 +2531,8 @@ static inline void ext4_unlock_group(struct super_block *sb,
> spin_unlock(ext4_group_lock_ptr(sb, group));
> }
>
> +int ext4_is_sane_bgdata_location(struct super_block *sb, ext4_group_t grp,
> + ext4_fsblk_t blk);
> /*
> * Block validity checking
> */
> diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c
> index 137193f..242250d 100644
> --- a/fs/ext4/ialloc.c
> +++ b/fs/ext4/ialloc.c
> @@ -122,6 +122,10 @@ ext4_read_inode_bitmap(struct super_block *sb, ext4_group_t block_group)
> return NULL;
>
> bitmap_blk = ext4_inode_bitmap(sb, desc);
> + if (!ext4_is_sane_bgdata_location(sb, block_group, bitmap_blk)) {
> + grp = ext4_get_group_info(sb, block_group);
> + set_bit(EXT4_GROUP_INFO_IBITMAP_CORRUPT_BIT, &grp->bb_state);
> + }
> bh = sb_getblk(sb, bitmap_blk);
> if (unlikely(!bh)) {
> ext4_error(sb, "Cannot read inode bitmap - "
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 0d424d7..c5af22e 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -3795,6 +3795,7 @@ static int __ext4_get_inode_loc(struct inode *inode,
> struct super_block *sb = inode->i_sb;
> ext4_fsblk_t block;
> int inodes_per_block, inode_offset;
> + struct ext4_group_info *grp;
>
> iloc->bh = NULL;
> if (!ext4_valid_inum(sb, inode->i_ino))
> @@ -3814,6 +3815,11 @@ static int __ext4_get_inode_loc(struct inode *inode,
> block = ext4_inode_table(sb, gdp) + (inode_offset / inodes_per_block);
> iloc->offset = (inode_offset % inodes_per_block) * EXT4_INODE_SIZE(sb);
>
> + if (!ext4_is_sane_bgdata_location(sb, iloc->block_group, block)) {
> + grp = ext4_get_group_info(sb, iloc->block_group);
> + set_bit(EXT4_GROUP_INFO_IBITMAP_CORRUPT_BIT, &grp->bb_state);
> + return -EIO;
> + }
> bh = sb_getblk(sb, block);
> if (unlikely(!bh))
> return -ENOMEM;
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index 2c2e6cb..d22248e 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -2049,6 +2049,27 @@ void ext4_group_desc_csum_set(struct super_block *sb, __u32 block_group,
> gdp->bg_checksum = ext4_group_desc_csum(EXT4_SB(sb), block_group, gdp);
> }
>
> +/* returns 1 if the location of a blockgroup data item seems sane */
> +int ext4_is_sane_bgdata_location(struct super_block *sb, ext4_group_t grp,
> + ext4_fsblk_t blk)
> +{
> + struct ext4_sb_info *sbi = EXT4_SB(sb);
> + ext4_fsblk_t first_block = le32_to_cpu(sbi->s_es->s_first_data_block);
> + ext4_fsblk_t last_block;
> +
> + if (EXT4_HAS_INCOMPAT_FEATURE(sb, EXT4_FEATURE_INCOMPAT_FLEX_BG))
> + last_block = ext4_blocks_count(sbi->s_es) - 1;
> + else {
> + first_block += grp * sbi->s_blocks_per_group;
> + last_block = first_block + (EXT4_BLOCKS_PER_GROUP(sb) - 1);
> + }
> +
> + if (last_block >= ext4_blocks_count(sbi->s_es))
> + last_block = ext4_blocks_count(sbi->s_es) - 1;
> +
> + return blk >= first_block && blk <= last_block;
> +}
> +
> /* Called at mount-time, super-block is locked */
> static int ext4_check_descriptors(struct super_block *sb,
> ext4_group_t *first_not_zeroed)
>
> --
> 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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] ext4: Spot-check block group sub-table locations
2013-09-27 5:40 ` Zheng Liu
@ 2013-09-27 17:57 ` Darrick J. Wong
0 siblings, 0 replies; 7+ messages in thread
From: Darrick J. Wong @ 2013-09-27 17:57 UTC (permalink / raw)
To: tytso, linux-ext4
On Fri, Sep 27, 2013 at 01:40:55PM +0800, Zheng Liu wrote:
> On Thu, Sep 26, 2013 at 05:03:34PM -0700, Darrick J. Wong wrote:
> > Perform a quick sanity check of bitmap and inode table block numbers when
> > loading them, and if there's something suspicious, mark the block group
> > corrupt.
> >
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> > fs/ext4/balloc.c | 5 +++++
> > fs/ext4/ext4.h | 2 ++
> > fs/ext4/ialloc.c | 4 ++++
> > fs/ext4/inode.c | 6 ++++++
> > fs/ext4/super.c | 21 +++++++++++++++++++++
> > 5 files changed, 38 insertions(+)
> >
> >
> > diff --git a/fs/ext4/balloc.c b/fs/ext4/balloc.c
> > index 4390f9f..3d79bfe 100644
> > --- a/fs/ext4/balloc.c
> > +++ b/fs/ext4/balloc.c
> > @@ -396,11 +396,16 @@ ext4_read_block_bitmap_nowait(struct super_block *sb, ext4_group_t block_group)
> > struct ext4_group_desc *desc;
> > struct buffer_head *bh;
> > ext4_fsblk_t bitmap_blk;
> > + struct ext4_group_info *grp;
> >
> > desc = ext4_get_group_desc(sb, block_group, NULL);
> > if (!desc)
> > return NULL;
> > bitmap_blk = ext4_block_bitmap(sb, desc);
> > + if (!ext4_is_sane_bgdata_location(sb, block_group, bitmap_blk)) {
> > + grp = ext4_get_group_info(sb, block_group);
> > + set_bit(EXT4_GROUP_INFO_BBITMAP_CORRUPT_BIT, &grp->bb_state);
>
> Why don't return NULL directly? If we found that the location of bitmap
> is insane, we shouldn't read this bitmap.
>
> - Zheng
>
> > + }
> > bh = sb_getblk(sb, bitmap_blk);
> > if (unlikely(!bh)) {
> > ext4_error(sb, "Cannot get buffer for block bitmap - "
I thought it was fine in the event of an insane blockno to fall through to this
error message. I could make that more explicit, though.
--D
> > diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> > index af815ea..afb1bb2 100644
> > --- a/fs/ext4/ext4.h
> > +++ b/fs/ext4/ext4.h
> > @@ -2531,6 +2531,8 @@ static inline void ext4_unlock_group(struct super_block *sb,
> > spin_unlock(ext4_group_lock_ptr(sb, group));
> > }
> >
> > +int ext4_is_sane_bgdata_location(struct super_block *sb, ext4_group_t grp,
> > + ext4_fsblk_t blk);
> > /*
> > * Block validity checking
> > */
> > diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c
> > index 137193f..242250d 100644
> > --- a/fs/ext4/ialloc.c
> > +++ b/fs/ext4/ialloc.c
> > @@ -122,6 +122,10 @@ ext4_read_inode_bitmap(struct super_block *sb, ext4_group_t block_group)
> > return NULL;
> >
> > bitmap_blk = ext4_inode_bitmap(sb, desc);
> > + if (!ext4_is_sane_bgdata_location(sb, block_group, bitmap_blk)) {
> > + grp = ext4_get_group_info(sb, block_group);
> > + set_bit(EXT4_GROUP_INFO_IBITMAP_CORRUPT_BIT, &grp->bb_state);
> > + }
> > bh = sb_getblk(sb, bitmap_blk);
> > if (unlikely(!bh)) {
> > ext4_error(sb, "Cannot read inode bitmap - "
> > diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> > index 0d424d7..c5af22e 100644
> > --- a/fs/ext4/inode.c
> > +++ b/fs/ext4/inode.c
> > @@ -3795,6 +3795,7 @@ static int __ext4_get_inode_loc(struct inode *inode,
> > struct super_block *sb = inode->i_sb;
> > ext4_fsblk_t block;
> > int inodes_per_block, inode_offset;
> > + struct ext4_group_info *grp;
> >
> > iloc->bh = NULL;
> > if (!ext4_valid_inum(sb, inode->i_ino))
> > @@ -3814,6 +3815,11 @@ static int __ext4_get_inode_loc(struct inode *inode,
> > block = ext4_inode_table(sb, gdp) + (inode_offset / inodes_per_block);
> > iloc->offset = (inode_offset % inodes_per_block) * EXT4_INODE_SIZE(sb);
> >
> > + if (!ext4_is_sane_bgdata_location(sb, iloc->block_group, block)) {
> > + grp = ext4_get_group_info(sb, iloc->block_group);
> > + set_bit(EXT4_GROUP_INFO_IBITMAP_CORRUPT_BIT, &grp->bb_state);
> > + return -EIO;
> > + }
> > bh = sb_getblk(sb, block);
> > if (unlikely(!bh))
> > return -ENOMEM;
> > diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> > index 2c2e6cb..d22248e 100644
> > --- a/fs/ext4/super.c
> > +++ b/fs/ext4/super.c
> > @@ -2049,6 +2049,27 @@ void ext4_group_desc_csum_set(struct super_block *sb, __u32 block_group,
> > gdp->bg_checksum = ext4_group_desc_csum(EXT4_SB(sb), block_group, gdp);
> > }
> >
> > +/* returns 1 if the location of a blockgroup data item seems sane */
> > +int ext4_is_sane_bgdata_location(struct super_block *sb, ext4_group_t grp,
> > + ext4_fsblk_t blk)
> > +{
> > + struct ext4_sb_info *sbi = EXT4_SB(sb);
> > + ext4_fsblk_t first_block = le32_to_cpu(sbi->s_es->s_first_data_block);
> > + ext4_fsblk_t last_block;
> > +
> > + if (EXT4_HAS_INCOMPAT_FEATURE(sb, EXT4_FEATURE_INCOMPAT_FLEX_BG))
> > + last_block = ext4_blocks_count(sbi->s_es) - 1;
> > + else {
> > + first_block += grp * sbi->s_blocks_per_group;
> > + last_block = first_block + (EXT4_BLOCKS_PER_GROUP(sb) - 1);
> > + }
> > +
> > + if (last_block >= ext4_blocks_count(sbi->s_es))
> > + last_block = ext4_blocks_count(sbi->s_es) - 1;
> > +
> > + return blk >= first_block && blk <= last_block;
> > +}
> > +
> > /* Called at mount-time, super-block is locked */
> > static int ext4_check_descriptors(struct super_block *sb,
> > ext4_group_t *first_not_zeroed)
> >
> > --
> > 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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] ext4: Don't count free clusters from a corrupt block group
2013-09-27 5:37 ` Zheng Liu
@ 2013-09-27 18:03 ` Darrick J. Wong
0 siblings, 0 replies; 7+ messages in thread
From: Darrick J. Wong @ 2013-09-27 18:03 UTC (permalink / raw)
To: tytso, linux-ext4
On Fri, Sep 27, 2013 at 01:37:24PM +0800, Zheng Liu wrote:
> On Thu, Sep 26, 2013 at 05:03:28PM -0700, Darrick J. Wong wrote:
> > A bg that's been flagged "corrupt" by definition has no free blocks, so that
> > the allocator won't be tempted to use the damaged bg. Therefore, we shouldn't
> > count the clusters in the damaged group when calculating free counts.
> >
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> > fs/ext4/balloc.c | 7 ++++++-
> > 1 file changed, 6 insertions(+), 1 deletion(-)
> >
> >
> > diff --git a/fs/ext4/balloc.c b/fs/ext4/balloc.c
> > index dc5d572..4390f9f 100644
> > --- a/fs/ext4/balloc.c
> > +++ b/fs/ext4/balloc.c
> > @@ -640,6 +640,7 @@ ext4_fsblk_t ext4_count_free_clusters(struct super_block *sb)
> > struct ext4_group_desc *gdp;
> > ext4_group_t i;
> > ext4_group_t ngroups = ext4_get_groups_count(sb);
> > + struct ext4_group_info *grp;
> > #ifdef EXT4FS_DEBUG
>
> It seems that you forgot to add these code in this macro definition.
Oops, you're right, I did neglect the #if EXT4FS_DEBUG case. Thank you for
catching that!
--D
>
> diff --git a/fs/ext4/balloc.c b/fs/ext4/balloc.c
> index dc5d572..3ba2079 100644
> --- a/fs/ext4/balloc.c
> +++ b/fs/ext4/balloc.c
> @@ -655,7 +655,11 @@ ext4_fsblk_t ext4_count_free_clusters(struct
> super_block *sb)
> gdp = ext4_get_group_desc(sb, i, NULL);
> if (!gdp)
> continue;
> - desc_count += ext4_free_group_clusters(sb, gdp);
> + grp = NULL;
> + if (EXT4_SB(sb)->s_group_info)
> + grp = ext4_get_group_info(sb, i);
> + if (!grp || !EXT4_MB_GRP_BBITMAP_CORRUPT(grp))
> + desc_count += ext4_free_group_clusters(sb, gdp);
> brelse(bitmap_bh);
> bitmap_bh = ext4_read_block_bitmap(sb, i);
> if (bitmap_bh == NULL)
>
> - Zheng
>
> > struct ext4_super_block *es;
> > ext4_fsblk_t bitmap_count;
> > @@ -679,7 +680,11 @@ ext4_fsblk_t ext4_count_free_clusters(struct super_block *sb)
> > gdp = ext4_get_group_desc(sb, i, NULL);
> > if (!gdp)
> > continue;
> > - desc_count += ext4_free_group_clusters(sb, gdp);
> > + grp = NULL;
> > + if (EXT4_SB(sb)->s_group_info)
> > + grp = ext4_get_group_info(sb, i);
> > + if (!grp || !EXT4_MB_GRP_BBITMAP_CORRUPT(grp))
> > + desc_count += ext4_free_group_clusters(sb, gdp);
> > }
> >
> > return desc_count;
> >
> > --
> > 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
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2013-09-27 18:03 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-09-27 0:03 [PATCH v1 0/2] ext4: Shut down block groups when damage is detected, part 2 Darrick J. Wong
2013-09-27 0:03 ` [PATCH 1/2] ext4: Don't count free clusters from a corrupt block group Darrick J. Wong
2013-09-27 5:37 ` Zheng Liu
2013-09-27 18:03 ` Darrick J. Wong
2013-09-27 0:03 ` [PATCH 2/2] ext4: Spot-check block group sub-table locations Darrick J. Wong
2013-09-27 5:40 ` Zheng Liu
2013-09-27 17:57 ` Darrick J. Wong
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).