diff mbox series

[SRU,Trusty,1/1] ext4: fix bitmap position validation

Message ID dd78d0a174b6c2d1560da6a02ea5e16f6eda7c8d.1535473339.git.joseph.salisbury@canonical.com
State New
Headers show
Series [SRU,Trusty,1/1] ext4: fix bitmap position validation | expand

Commit Message

Joseph Salisbury Aug. 31, 2018, 2:04 p.m. UTC 
From: Lukas Czerner <lczerner@redhat.com>

BugLink: https://bugs.launchpad.net/bugs/1789131

Currently in ext4_valid_block_bitmap() we expect the bitmap to be
positioned anywhere between 0 and s_blocksize clusters, but that's
wrong because the bitmap can be placed anywhere in the block group. This
causes false positives when validating bitmaps on perfectly valid file
system layouts. Fix it by checking whether the bitmap is within the group
boundary.

The problem can be reproduced using the following

mkfs -t ext3 -E stride=256 /dev/vdb1
mount /dev/vdb1 /mnt/test
cd /mnt/test
wget https://cdn.kernel.org/pub/linux/kernel/v4.x/linux-4.16.3.tar.xz
tar xf linux-4.16.3.tar.xz

This will result in the warnings in the logs

EXT4-fs error (device vdb1): ext4_validate_block_bitmap:399: comm tar: bg 84: block 2774529: invalid block bitmap

[ Changed slightly for clarity and to not drop a overflow test -- TYT ]

Signed-off-by: Lukas Czerner <lczerner@redhat.com>
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
Reported-by: Ilya Dryomov <idryomov@gmail.com>
Fixes: 7dac4a1726a9 ("ext4: add validity checks for bitmap block numbers")
Cc: stable@vger.kernel.org
(cherry picked from commit 22be37acce25d66ecf6403fc8f44df9c5ded2372)
Signed-off-by: Joseph Salisbury <joseph.salisbury@canonical.com>
Reviewed-by: Tyler Hicks <tyhicks@canonical.com>
---
 fs/ext4/balloc.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

Comments

Stefan Bader Sept. 5, 2018, 12:53 p.m. UTC | #1 
On 31.08.2018 16:04, Joseph Salisbury wrote:
> From: Lukas Czerner <lczerner@redhat.com>
> 
> BugLink: https://bugs.launchpad.net/bugs/1789131
> 
> Currently in ext4_valid_block_bitmap() we expect the bitmap to be
> positioned anywhere between 0 and s_blocksize clusters, but that's
> wrong because the bitmap can be placed anywhere in the block group. This
> causes false positives when validating bitmaps on perfectly valid file
> system layouts. Fix it by checking whether the bitmap is within the group
> boundary.
> 
> The problem can be reproduced using the following
> 
> mkfs -t ext3 -E stride=256 /dev/vdb1
> mount /dev/vdb1 /mnt/test
> cd /mnt/test
> wget https://cdn.kernel.org/pub/linux/kernel/v4.x/linux-4.16.3.tar.xz
> tar xf linux-4.16.3.tar.xz
> 
> This will result in the warnings in the logs
> 
> EXT4-fs error (device vdb1): ext4_validate_block_bitmap:399: comm tar: bg 84: block 2774529: invalid block bitmap
> 
> [ Changed slightly for clarity and to not drop a overflow test -- TYT ]
> 
> Signed-off-by: Lukas Czerner <lczerner@redhat.com>
> Signed-off-by: Theodore Ts'o <tytso@mit.edu>
> Reported-by: Ilya Dryomov <idryomov@gmail.com>
> Fixes: 7dac4a1726a9 ("ext4: add validity checks for bitmap block numbers")
> Cc: stable@vger.kernel.org
> (cherry picked from commit 22be37acce25d66ecf6403fc8f44df9c5ded2372)
Acked-by: Stefan Bader <stefan.bader@canonical.com>
> Signed-off-by: Joseph Salisbury <joseph.salisbury@canonical.com>
> Reviewed-by: Tyler Hicks <tyhicks@canonical.com>
> ---
>  fs/ext4/balloc.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/ext4/balloc.c b/fs/ext4/balloc.c
> index c19e367..7f0300f 100644
> --- a/fs/ext4/balloc.c
> +++ b/fs/ext4/balloc.c
> @@ -311,6 +311,7 @@ static ext4_fsblk_t ext4_valid_block_bitmap(struct super_block *sb,
>  	struct ext4_sb_info *sbi = EXT4_SB(sb);
>  	ext4_grpblk_t offset;
>  	ext4_grpblk_t next_zero_bit;
> +	ext4_grpblk_t max_bit = EXT4_CLUSTERS_PER_GROUP(sb);
>  	ext4_fsblk_t blk;
>  	ext4_fsblk_t group_first_block;
>  
> @@ -328,7 +329,7 @@ static ext4_fsblk_t ext4_valid_block_bitmap(struct super_block *sb,
>  	/* check whether block bitmap block number is set */
>  	blk = ext4_block_bitmap(sb, desc);
>  	offset = blk - group_first_block;
> -	if (offset < 0 || EXT4_B2C(sbi, offset) >= sb->s_blocksize ||
> +	if (offset < 0 || EXT4_B2C(sbi, offset) >= max_bit ||
>  	    !ext4_test_bit(EXT4_B2C(sbi, offset), bh->b_data))
>  		/* bad block bitmap */
>  		return blk;
> @@ -336,7 +337,7 @@ static ext4_fsblk_t ext4_valid_block_bitmap(struct super_block *sb,
>  	/* check whether the inode bitmap block number is set */
>  	blk = ext4_inode_bitmap(sb, desc);
>  	offset = blk - group_first_block;
> -	if (offset < 0 || EXT4_B2C(sbi, offset) >= sb->s_blocksize ||
> +	if (offset < 0 || EXT4_B2C(sbi, offset) >= max_bit ||
>  	    !ext4_test_bit(EXT4_B2C(sbi, offset), bh->b_data))
>  		/* bad block bitmap */
>  		return blk;
> @@ -344,8 +345,8 @@ static ext4_fsblk_t ext4_valid_block_bitmap(struct super_block *sb,
>  	/* check whether the inode table block number is set */
>  	blk = ext4_inode_table(sb, desc);
>  	offset = blk - group_first_block;
> -	if (offset < 0 || EXT4_B2C(sbi, offset) >= sb->s_blocksize ||
> -	    EXT4_B2C(sbi, offset + sbi->s_itb_per_group) >= sb->s_blocksize)
> +	if (offset < 0 || EXT4_B2C(sbi, offset) >= max_bit ||
> +	    EXT4_B2C(sbi, offset + sbi->s_itb_per_group) >= max_bit)
>  		return blk;
>  	next_zero_bit = ext4_find_next_zero_bit(bh->b_data,
>  			EXT4_B2C(sbi, offset + EXT4_SB(sb)->s_itb_per_group),
>
Kleber Sacilotto de Souza Sept. 5, 2018, 2:33 p.m. UTC | #2 
On 08/31/18 16:04, Joseph Salisbury wrote:
> From: Lukas Czerner <lczerner@redhat.com>
> 
> BugLink: https://bugs.launchpad.net/bugs/1789131
> 
> Currently in ext4_valid_block_bitmap() we expect the bitmap to be
> positioned anywhere between 0 and s_blocksize clusters, but that's
> wrong because the bitmap can be placed anywhere in the block group. This
> causes false positives when validating bitmaps on perfectly valid file
> system layouts. Fix it by checking whether the bitmap is within the group
> boundary.
> 
> The problem can be reproduced using the following
> 
> mkfs -t ext3 -E stride=256 /dev/vdb1
> mount /dev/vdb1 /mnt/test
> cd /mnt/test
> wget https://cdn.kernel.org/pub/linux/kernel/v4.x/linux-4.16.3.tar.xz
> tar xf linux-4.16.3.tar.xz
> 
> This will result in the warnings in the logs
> 
> EXT4-fs error (device vdb1): ext4_validate_block_bitmap:399: comm tar: bg 84: block 2774529: invalid block bitmap
> 
> [ Changed slightly for clarity and to not drop a overflow test -- TYT ]
> 
> Signed-off-by: Lukas Czerner <lczerner@redhat.com>
> Signed-off-by: Theodore Ts'o <tytso@mit.edu>
> Reported-by: Ilya Dryomov <idryomov@gmail.com>
> Fixes: 7dac4a1726a9 ("ext4: add validity checks for bitmap block numbers")
> Cc: stable@vger.kernel.org
> (cherry picked from commit 22be37acce25d66ecf6403fc8f44df9c5ded2372)
> Signed-off-by: Joseph Salisbury <joseph.salisbury@canonical.com>
> Reviewed-by: Tyler Hicks <tyhicks@canonical.com>

Acked-by: Kleber Sacilotto de Souza <kleber.souza@canonical.com>

> ---
>  fs/ext4/balloc.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/ext4/balloc.c b/fs/ext4/balloc.c
> index c19e367..7f0300f 100644
> --- a/fs/ext4/balloc.c
> +++ b/fs/ext4/balloc.c
> @@ -311,6 +311,7 @@ static ext4_fsblk_t ext4_valid_block_bitmap(struct super_block *sb,
>  	struct ext4_sb_info *sbi = EXT4_SB(sb);
>  	ext4_grpblk_t offset;
>  	ext4_grpblk_t next_zero_bit;
> +	ext4_grpblk_t max_bit = EXT4_CLUSTERS_PER_GROUP(sb);
>  	ext4_fsblk_t blk;
>  	ext4_fsblk_t group_first_block;
>  
> @@ -328,7 +329,7 @@ static ext4_fsblk_t ext4_valid_block_bitmap(struct super_block *sb,
>  	/* check whether block bitmap block number is set */
>  	blk = ext4_block_bitmap(sb, desc);
>  	offset = blk - group_first_block;
> -	if (offset < 0 || EXT4_B2C(sbi, offset) >= sb->s_blocksize ||
> +	if (offset < 0 || EXT4_B2C(sbi, offset) >= max_bit ||
>  	    !ext4_test_bit(EXT4_B2C(sbi, offset), bh->b_data))
>  		/* bad block bitmap */
>  		return blk;
> @@ -336,7 +337,7 @@ static ext4_fsblk_t ext4_valid_block_bitmap(struct super_block *sb,
>  	/* check whether the inode bitmap block number is set */
>  	blk = ext4_inode_bitmap(sb, desc);
>  	offset = blk - group_first_block;
> -	if (offset < 0 || EXT4_B2C(sbi, offset) >= sb->s_blocksize ||
> +	if (offset < 0 || EXT4_B2C(sbi, offset) >= max_bit ||
>  	    !ext4_test_bit(EXT4_B2C(sbi, offset), bh->b_data))
>  		/* bad block bitmap */
>  		return blk;
> @@ -344,8 +345,8 @@ static ext4_fsblk_t ext4_valid_block_bitmap(struct super_block *sb,
>  	/* check whether the inode table block number is set */
>  	blk = ext4_inode_table(sb, desc);
>  	offset = blk - group_first_block;
> -	if (offset < 0 || EXT4_B2C(sbi, offset) >= sb->s_blocksize ||
> -	    EXT4_B2C(sbi, offset + sbi->s_itb_per_group) >= sb->s_blocksize)
> +	if (offset < 0 || EXT4_B2C(sbi, offset) >= max_bit ||
> +	    EXT4_B2C(sbi, offset + sbi->s_itb_per_group) >= max_bit)
>  		return blk;
>  	next_zero_bit = ext4_find_next_zero_bit(bh->b_data,
>  			EXT4_B2C(sbi, offset + EXT4_SB(sb)->s_itb_per_group),
>
diff mbox series

Patch

diff --git a/fs/ext4/balloc.c b/fs/ext4/balloc.c
index c19e367..7f0300f 100644
--- a/fs/ext4/balloc.c
+++ b/fs/ext4/balloc.c
@@ -311,6 +311,7 @@  static ext4_fsblk_t ext4_valid_block_bitmap(struct super_block *sb,
 	struct ext4_sb_info *sbi = EXT4_SB(sb);
 	ext4_grpblk_t offset;
 	ext4_grpblk_t next_zero_bit;
+	ext4_grpblk_t max_bit = EXT4_CLUSTERS_PER_GROUP(sb);
 	ext4_fsblk_t blk;
 	ext4_fsblk_t group_first_block;
 
@@ -328,7 +329,7 @@  static ext4_fsblk_t ext4_valid_block_bitmap(struct super_block *sb,
 	/* check whether block bitmap block number is set */
 	blk = ext4_block_bitmap(sb, desc);
 	offset = blk - group_first_block;
-	if (offset < 0 || EXT4_B2C(sbi, offset) >= sb->s_blocksize ||
+	if (offset < 0 || EXT4_B2C(sbi, offset) >= max_bit ||
 	    !ext4_test_bit(EXT4_B2C(sbi, offset), bh->b_data))
 		/* bad block bitmap */
 		return blk;
@@ -336,7 +337,7 @@  static ext4_fsblk_t ext4_valid_block_bitmap(struct super_block *sb,
 	/* check whether the inode bitmap block number is set */
 	blk = ext4_inode_bitmap(sb, desc);
 	offset = blk - group_first_block;
-	if (offset < 0 || EXT4_B2C(sbi, offset) >= sb->s_blocksize ||
+	if (offset < 0 || EXT4_B2C(sbi, offset) >= max_bit ||
 	    !ext4_test_bit(EXT4_B2C(sbi, offset), bh->b_data))
 		/* bad block bitmap */
 		return blk;
@@ -344,8 +345,8 @@  static ext4_fsblk_t ext4_valid_block_bitmap(struct super_block *sb,
 	/* check whether the inode table block number is set */
 	blk = ext4_inode_table(sb, desc);
 	offset = blk - group_first_block;
-	if (offset < 0 || EXT4_B2C(sbi, offset) >= sb->s_blocksize ||
-	    EXT4_B2C(sbi, offset + sbi->s_itb_per_group) >= sb->s_blocksize)
+	if (offset < 0 || EXT4_B2C(sbi, offset) >= max_bit ||
+	    EXT4_B2C(sbi, offset + sbi->s_itb_per_group) >= max_bit)
 		return blk;
 	next_zero_bit = ext4_find_next_zero_bit(bh->b_data,
 			EXT4_B2C(sbi, offset + EXT4_SB(sb)->s_itb_per_group),