* [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; 12+ 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] 12+ 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; 12+ 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] 12+ 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; 12+ 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] 12+ 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; 12+ 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] 12+ 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; 12+ 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] 12+ 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; 12+ 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] 12+ 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; 12+ 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] 12+ messages in thread
* [PATCH v1.1 0/2] ext4: Shut down block groups when damage is detected, part 2 @ 2013-09-28 0:13 Darrick J. Wong 2013-09-28 0:14 ` [PATCH 2/2] ext4: Spot-check block group sub-table locations Darrick J. Wong 0 siblings, 1 reply; 12+ messages in thread From: Darrick J. Wong @ 2013-09-28 0:13 UTC (permalink / raw) To: tytso, darrick.wong; +Cc: linux-ext4, gnehzuil.liu 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. Zheng Liu contributed some comments. Comments and questions are, as always, welcome. --D ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 2/2] ext4: Spot-check block group sub-table locations 2013-09-28 0:13 [PATCH v1.1 0/2] ext4: Shut down block groups when damage is detected, part 2 Darrick J. Wong @ 2013-09-28 0:14 ` Darrick J. Wong 2013-09-29 6:44 ` Zheng Liu 0 siblings, 1 reply; 12+ messages in thread From: Darrick J. Wong @ 2013-09-28 0:14 UTC (permalink / raw) To: tytso, darrick.wong; +Cc: linux-ext4, gnehzuil.liu 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 | 7 +++++++ fs/ext4/ext4.h | 2 ++ fs/ext4/ialloc.c | 6 ++++++ fs/ext4/inode.c | 9 +++++++++ fs/ext4/super.c | 21 +++++++++++++++++++++ 5 files changed, 45 insertions(+) diff --git a/fs/ext4/balloc.c b/fs/ext4/balloc.c index 6ea7b14..2f35689 100644 --- a/fs/ext4/balloc.c +++ b/fs/ext4/balloc.c @@ -396,13 +396,20 @@ 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); + goto no_bitmap; + } bh = sb_getblk(sb, bitmap_blk); if (unlikely(!bh)) { +no_bitmap: ext4_error(sb, "Cannot get buffer for block bitmap - " "block_group = %u, block_bitmap = %llu", block_group, bitmap_blk); 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..4e7994b 100644 --- a/fs/ext4/ialloc.c +++ b/fs/ext4/ialloc.c @@ -122,8 +122,14 @@ 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); + goto no_bitmap; + } bh = sb_getblk(sb, bitmap_blk); if (unlikely(!bh)) { +no_bitmap: ext4_error(sb, "Cannot read inode bitmap - " "block_group = %u, inode_bitmap = %llu", block_group, bitmap_blk); diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c index 0d424d7..0a54e13 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,14 @@ 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); + ext4_error(sb, "Cannot read inode table - " + "block_group = %u, inode_table_block = %llu", + iloc->block_group, block); + 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] 12+ messages in thread
* Re: [PATCH 2/2] ext4: Spot-check block group sub-table locations 2013-09-28 0:14 ` [PATCH 2/2] ext4: Spot-check block group sub-table locations Darrick J. Wong @ 2013-09-29 6:44 ` Zheng Liu 2013-10-09 22:08 ` Darrick J. Wong 0 siblings, 1 reply; 12+ messages in thread From: Zheng Liu @ 2013-09-29 6:44 UTC (permalink / raw) To: Darrick J. Wong; +Cc: tytso, linux-ext4 On Fri, Sep 27, 2013 at 05:14:04PM -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> One comment below. Otherwise the patch looks good to me. Reviewed-by: Zheng Liu <wenqing.lz@taobao.com> > --- > fs/ext4/balloc.c | 7 +++++++ > fs/ext4/ext4.h | 2 ++ > fs/ext4/ialloc.c | 6 ++++++ > fs/ext4/inode.c | 9 +++++++++ > fs/ext4/super.c | 21 +++++++++++++++++++++ > 5 files changed, 45 insertions(+) [...] > 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) FWIW, I notice that in ext4_check_descriptors() we also check the location of bitmap. So maybe we could use ext4_is_sane_bgdata_location in that function. - Zheng ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] ext4: Spot-check block group sub-table locations 2013-09-29 6:44 ` Zheng Liu @ 2013-10-09 22:08 ` Darrick J. Wong 2013-10-30 16:45 ` Theodore Ts'o 0 siblings, 1 reply; 12+ messages in thread From: Darrick J. Wong @ 2013-10-09 22:08 UTC (permalink / raw) To: tytso, linux-ext4 On Sun, Sep 29, 2013 at 02:44:01PM +0800, Zheng Liu wrote: > On Fri, Sep 27, 2013 at 05:14:04PM -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> > > One comment below. Otherwise the patch looks good to me. > Reviewed-by: Zheng Liu <wenqing.lz@taobao.com> > > > --- > > fs/ext4/balloc.c | 7 +++++++ > > fs/ext4/ext4.h | 2 ++ > > fs/ext4/ialloc.c | 6 ++++++ > > fs/ext4/inode.c | 9 +++++++++ > > fs/ext4/super.c | 21 +++++++++++++++++++++ > > 5 files changed, 45 insertions(+) > [...] > > 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) > > FWIW, I notice that in ext4_check_descriptors() we also check the > location of bitmap. So maybe we could use ext4_is_sane_bgdata_location > in that function. That could be done, but all we'd be doing is trading this: block_bitmap = ext4_block_bitmap(sb, gdp); if (block_bitmap < first_block || block_bitmap > last_block) { for this slightly slower code: if (ext4_is_sane_bgdata_location(sb, gdp, ext4_block_bitmap(sb, gdb))) { I don't know if that's worth the code churn or not. It's an easy change, but I don't think it adds much. Ted -- if you want me to make the change I'll do it, but if you're content to leave the patch as it is, then so am I. --D > > - Zheng ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] ext4: Spot-check block group sub-table locations 2013-10-09 22:08 ` Darrick J. Wong @ 2013-10-30 16:45 ` Theodore Ts'o 2013-10-30 17:33 ` Darrick J. Wong 0 siblings, 1 reply; 12+ messages in thread From: Theodore Ts'o @ 2013-10-30 16:45 UTC (permalink / raw) To: Darrick J. Wong; +Cc: linux-ext4 On Wed, Oct 09, 2013 at 03:08:24PM -0700, Darrick J. Wong wrote: > > FWIW, I notice that in ext4_check_descriptors() we also check the > > location of bitmap. So maybe we could use ext4_is_sane_bgdata_location > > in that function. > > That could be done, but all we'd be doing is trading this: > > block_bitmap = ext4_block_bitmap(sb, gdp); > if (block_bitmap < first_block || block_bitmap > last_block) { > > for this slightly slower code: > > if (ext4_is_sane_bgdata_location(sb, gdp, > ext4_block_bitmap(sb, gdb))) { > > I don't know if that's worth the code churn or not. It's an easy change, but I > don't think it adds much. Ted -- if you want me to make the change I'll do it, > but if you're content to leave the patch as it is, then so am I. The question I have is if we are checking to make sure all of the block and inode bitmaps are sane at mount time, how much is it worth it to check to make sure that they are sane each time we need to load the bitmap? This is especially true since for ext4 file systems with flex_bg, the only real check we can do is to is whether the block number is out of range, which will almost certainly cause a device error. - Ted ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] ext4: Spot-check block group sub-table locations 2013-10-30 16:45 ` Theodore Ts'o @ 2013-10-30 17:33 ` Darrick J. Wong 0 siblings, 0 replies; 12+ messages in thread From: Darrick J. Wong @ 2013-10-30 17:33 UTC (permalink / raw) To: Theodore Ts'o; +Cc: linux-ext4 On Wed, Oct 30, 2013 at 12:45:07PM -0400, Theodore Ts'o wrote: > On Wed, Oct 09, 2013 at 03:08:24PM -0700, Darrick J. Wong wrote: > > > FWIW, I notice that in ext4_check_descriptors() we also check the > > > location of bitmap. So maybe we could use ext4_is_sane_bgdata_location > > > in that function. > > > > That could be done, but all we'd be doing is trading this: > > > > block_bitmap = ext4_block_bitmap(sb, gdp); > > if (block_bitmap < first_block || block_bitmap > last_block) { > > > > for this slightly slower code: > > > > if (ext4_is_sane_bgdata_location(sb, gdp, > > ext4_block_bitmap(sb, gdb))) { > > > > I don't know if that's worth the code churn or not. It's an easy change, but I > > don't think it adds much. Ted -- if you want me to make the change I'll do it, > > but if you're content to leave the patch as it is, then so am I. > > The question I have is if we are checking to make sure all of the > block and inode bitmaps are sane at mount time, how much is it worth > it to check to make sure that they are sane each time we need to load > the bitmap? This is especially true since for ext4 file systems with > flex_bg, the only real check we can do is to is whether the block > number is out of range, which will almost certainly cause a device > error. Probably not worth much, but I thought it was cheap compared to making and bouncing an IO. Also, if the check fails, we can mark the bg offline and let fsck sort out the mess. The regular error path doesn't do this... though we could change that too. --D > > - 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2013-10-30 17:33 UTC | newest] Thread overview: 12+ 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 -- strict thread matches above, loose matches on Subject: below -- 2013-09-28 0:13 [PATCH v1.1 0/2] ext4: Shut down block groups when damage is detected, part 2 Darrick J. Wong 2013-09-28 0:14 ` [PATCH 2/2] ext4: Spot-check block group sub-table locations Darrick J. Wong 2013-09-29 6:44 ` Zheng Liu 2013-10-09 22:08 ` Darrick J. Wong 2013-10-30 16:45 ` Theodore Ts'o 2013-10-30 17:33 ` 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).