[PATCH 0/2] ext2: Refuse filesystems with invalid block size

[PATCH 0/2] ext2: Refuse filesystems with invalid block size

[Jan Kara]

Hello,

these two patches make sure to validate log of filesystem block size stored in
the superblock before it is used to avoid undefined shifts. I plan to merge
the patches through my tree.

								Honza

[PATCH 1/2] ext2: Correct maximum ext2 filesystem block size

[Jan Kara]

Ext2 has traditionally supported filesystem block sizes upto page size
or upto 65536. Macro EXT2_MAX_BLOCK_SIZE is set to 4096, however that is
never used in ext2 so practically we always allowed whatever
sb_set_blocksize() accepted. Fix value of EXT2_MAX_BLOCK_SIZE because it
will be used in the next patch.

Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/ext2/ext2.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/ext2/ext2.h b/fs/ext2/ext2.h
index cb78d7dcfb95..6c8e838bb278 100644
--- a/fs/ext2/ext2.h
+++ b/fs/ext2/ext2.h
@@ -178,7 +178,7 @@ static inline struct ext2_sb_info *EXT2_SB(struct super_block *sb)
  * Macro-instructions used to manage several block sizes
  */
 #define EXT2_MIN_BLOCK_SIZE		1024
-#define	EXT2_MAX_BLOCK_SIZE		4096
+#define	EXT2_MAX_BLOCK_SIZE		65536
 #define EXT2_MIN_BLOCK_LOG_SIZE		  10
 #define EXT2_BLOCK_SIZE(s)		((s)->s_blocksize)
 #define	EXT2_ADDR_PER_BLOCK(s)		(EXT2_BLOCK_SIZE(s) / sizeof (__u32))
-- 
2.35.3

[PATCH 2/2] ext2: Check block size validity during mount

[Jan Kara]

Check that log of block size stored in the superblock has sensible
value. Otherwise the shift computing the block size can overflow leading
to undefined behavior.

Reported-by: syzbot+4fec412f59eba8c01b77@syzkaller.appspotmail.com
Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/ext2/ext2.h  | 1 +
 fs/ext2/super.c | 7 +++++++
 2 files changed, 8 insertions(+)

diff --git a/fs/ext2/ext2.h b/fs/ext2/ext2.h
index 6c8e838bb278..8244366862e4 100644
--- a/fs/ext2/ext2.h
+++ b/fs/ext2/ext2.h
@@ -180,6 +180,7 @@ static inline struct ext2_sb_info *EXT2_SB(struct super_block *sb)
 #define EXT2_MIN_BLOCK_SIZE		1024
 #define	EXT2_MAX_BLOCK_SIZE		65536
 #define EXT2_MIN_BLOCK_LOG_SIZE		  10
+#define EXT2_MAX_BLOCK_LOG_SIZE		  16
 #define EXT2_BLOCK_SIZE(s)		((s)->s_blocksize)
 #define	EXT2_ADDR_PER_BLOCK(s)		(EXT2_BLOCK_SIZE(s) / sizeof (__u32))
 #define EXT2_BLOCK_SIZE_BITS(s)		((s)->s_blocksize_bits)
diff --git a/fs/ext2/super.c b/fs/ext2/super.c
index 69c88facfe90..f342f347a695 100644
--- a/fs/ext2/super.c
+++ b/fs/ext2/super.c
@@ -945,6 +945,13 @@ static int ext2_fill_super(struct super_block *sb, void *data, int silent)
 		goto failed_mount;
 	}
 
+	if (le32_to_cpu(es->s_log_block_size) >
+	    (EXT2_MAX_BLOCK_LOG_SIZE - BLOCK_SIZE_BITS)) {
+		ext2_msg(sb, KERN_ERR,
+			 "Invalid log block size: %u",
+			 le32_to_cpu(es->s_log_block_size));
+		goto failed_mount;
+	}
 	blocksize = BLOCK_SIZE << le32_to_cpu(sbi->s_es->s_log_block_size);
 
 	if (test_opt(sb, DAX)) {
-- 
2.35.3

Re: [PATCH 2/2] ext2: Check block size validity during mount

[Tudor Ambarus]

Hi!

On 3/1/23 11:12, Jan Kara wrote:
> Check that log of block size stored in the superblock has sensible
> value. Otherwise the shift computing the block size can overflow leading
> to undefined behavior.
> 
> Reported-by: syzbot+4fec412f59eba8c01b77@syzkaller.appspotmail.com

Would be helpful to also have:
LINK: https://syzkaller.appspot.com/bug?extid=4fec412f59eba8c01b77
a "Fixes:" tag and
Cc: stable@vger.kernel.org

Cheers,
ta

> Signed-off-by: Jan Kara <jack@suse.cz>
> ---
>   fs/ext2/ext2.h  | 1 +
>   fs/ext2/super.c | 7 +++++++
>   2 files changed, 8 insertions(+)
> 
> diff --git a/fs/ext2/ext2.h b/fs/ext2/ext2.h
> index 6c8e838bb278..8244366862e4 100644
> --- a/fs/ext2/ext2.h
> +++ b/fs/ext2/ext2.h
> @@ -180,6 +180,7 @@ static inline struct ext2_sb_info *EXT2_SB(struct super_block *sb)
>   #define EXT2_MIN_BLOCK_SIZE		1024
>   #define	EXT2_MAX_BLOCK_SIZE		65536
>   #define EXT2_MIN_BLOCK_LOG_SIZE		  10
> +#define EXT2_MAX_BLOCK_LOG_SIZE		  16
>   #define EXT2_BLOCK_SIZE(s)		((s)->s_blocksize)
>   #define	EXT2_ADDR_PER_BLOCK(s)		(EXT2_BLOCK_SIZE(s) / sizeof (__u32))
>   #define EXT2_BLOCK_SIZE_BITS(s)		((s)->s_blocksize_bits)
> diff --git a/fs/ext2/super.c b/fs/ext2/super.c
> index 69c88facfe90..f342f347a695 100644
> --- a/fs/ext2/super.c
> +++ b/fs/ext2/super.c
> @@ -945,6 +945,13 @@ static int ext2_fill_super(struct super_block *sb, void *data, int silent)
>   		goto failed_mount;
>   	}
>   
> +	if (le32_to_cpu(es->s_log_block_size) >
> +	    (EXT2_MAX_BLOCK_LOG_SIZE - BLOCK_SIZE_BITS)) {
> +		ext2_msg(sb, KERN_ERR,
> +			 "Invalid log block size: %u",
> +			 le32_to_cpu(es->s_log_block_size));
> +		goto failed_mount;
> +	}
>   	blocksize = BLOCK_SIZE << le32_to_cpu(sbi->s_es->s_log_block_size);
>   
>   	if (test_opt(sb, DAX)) {

Re: [PATCH 2/2] ext2: Check block size validity during mount

[Jan Kara]

On Wed 01-03-23 11:37:20, Tudor Ambarus wrote:
> Hi!
> 
> On 3/1/23 11:12, Jan Kara wrote:
> > Check that log of block size stored in the superblock has sensible
> > value. Otherwise the shift computing the block size can overflow leading
> > to undefined behavior.
> > 
> > Reported-by: syzbot+4fec412f59eba8c01b77@syzkaller.appspotmail.com
> 
> Would be helpful to also have:
> LINK: https://syzkaller.appspot.com/bug?extid=4fec412f59eba8c01b77
> a "Fixes:" tag and
> Cc: stable@vger.kernel.org

Well, Fixes tag doesn't really make sense here. The behavior is there since
forever... I've added the Link and CC tags. Thanks for the suggestion.

								Honza

-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR