2023-03-01 11:12:55

by Jan Kara

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

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

2023-03-01 11:12:57

by Jan Kara

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

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: [email protected]
Signed-off-by: Jan Kara <[email protected]>
---
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

2023-03-01 11:37:26

by Tudor Ambarus

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

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: [email protected]

Would be helpful to also have:
LINK: https://syzkaller.appspot.com/bug?extid=4fec412f59eba8c01b77
a "Fixes:" tag and
Cc: [email protected]

Cheers,
ta

> Signed-off-by: Jan Kara <[email protected]>
> ---
> 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)) {

2023-03-01 13:05:17

by Jan Kara

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

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: [email protected]
>
> Would be helpful to also have:
> LINK: https://syzkaller.appspot.com/bug?extid=4fec412f59eba8c01b77
> a "Fixes:" tag and
> Cc: [email protected]

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 <[email protected]>
SUSE Labs, CR