linux-ext4 - Re: [PATCH] ext2: add check for blocksize in ext2_fill

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Tue, 22 Jan 2019 20:29:39 +0800
From:   yangerkun <yangerkun@...wei.com>
To:     Jan Kara <jack@...e.cz>
CC:     <linux-ext4@...r.kernel.org>, <miaoxie@...wei.com>,
        <houtao1@...wei.com>, <yi.zhang@...wei.com>
Subject: Re: [PATCH] ext2: add check for blocksize in ext2_fill_super



Jan Kara wrote on 2019/1/22 19:17:
> Hello!
> 
> On Tue 15-01-19 20:46:01, yangerkun wrote:
>> Could you please help me to check this patch?
>>
>> Besides, i have notice that EXT2_MAX_BLOCK_SIZE has never been used before.
>> So there maybe someone using ext2fs which blocksize exceed
>> EXT2_MAX_BLOCK_SIZE in the os that page size exceed 4K? And if it's true,
>> this patch will lead to a unused ext2fs.
>>
>> Also, we can fix the problem by modify ext2_max_size, but it's difficult to
>> get the accurate result, it may not a good idea too...
> 
> Thanks for your report and the patch! Ext2 is supposed to be usable for
> block sizes between 1k and min(PAGE_SIZE, 64k). So rather than limiting
> block size to 4k, we should fix ext2_max_size(). Will you send a new patch
> or should I fix the problem? I agree it is difficult to come up with the
> exact limit for a particular block size. But the computation does not have
> to be exact - even the current one is not. It just has to be approximately
> right and not allow any overflow. And BTW ext4 seems to have the same
> overflow problem in its ext4_max_bitmap_size().
> 
Hi,

Thanks for your reply!

I am working on this problem now, and the patch will coming soon.

Thanks,
Kun.

> 								Honza
> 
>> yangerkun wrote on 2019/1/10 13:31:
>>> While mkfs with '-b 65536' and then mount this fs with arm 64KB page
>>> size, function mount_fs will trigger WARNING because that ext2_max_size
>>> will return value less than 0. Also, we cannot write any file in this
>>> fs since the sb->s_maxbytes is less than 0. Avoid this by check blocksize
>>> in ext2_fill_super like ext4_fill_super.
>>>
>>> Signed-off-by: yangerkun <yangerkun@...wei.com>
>>> ---
>>>    fs/ext2/ext2.h  |  1 +
>>>    fs/ext2/super.c | 15 +++++++++++++++
>>>    2 files changed, 16 insertions(+)
>>>
>>> diff --git a/fs/ext2/ext2.h b/fs/ext2/ext2.h
>>> index e770cd1..ba95316 100644
>>> --- a/fs/ext2/ext2.h
>>> +++ b/fs/ext2/ext2.h
>>> @@ -177,6 +177,7 @@ static inline struct ext2_sb_info *EXT2_SB(struct super_block *sb)
>>>    #define EXT2_MIN_BLOCK_SIZE		1024
>>>    #define	EXT2_MAX_BLOCK_SIZE		4096
>>>    #define EXT2_MIN_BLOCK_LOG_SIZE		  10
>>> +#define EXT2_MAX_BLOCK_LOG_SIZE		  12
>>>    #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 73b2d52..029945e 100644
>>> --- a/fs/ext2/super.c
>>> +++ b/fs/ext2/super.c
>>> @@ -960,6 +960,21 @@ static int ext2_fill_super(struct super_block *sb, void *data, int silent)
>>>    	}
>>>    	blocksize = BLOCK_SIZE << le32_to_cpu(sbi->s_es->s_log_block_size);
>>> +	if (blocksize < EXT2_MIN_BLOCK_SIZE ||
>>> +		blocksize > EXT2_MAX_BLOCK_SIZE) {
>>> +		ext2_msg(sb, KERN_ERR,
>>> +				"Unsupported filesystem blocksize %d (%d log_block_size)",
>>> +				blocksize, le32_to_cpu(es->s_log_block_size));
>>> +		goto failed_mount;
>>> +	}
>>> +
>>> +	if (le32_to_cpu(es->s_log_block_size) >
>>> +		(EXT2_MAX_BLOCK_LOG_SIZE - EXT2_MIN_BLOCK_LOG_SIZE)) {
>>> +			ext2_msg(sb, KERN_ERR,
>>> +				"Invalid log block size: %u",
>>> +				le32_to_cpu(es->s_log_block_size));
>>> +			goto failed_mount;
>>> +		}
>>>    	if (sbi->s_mount_opt & EXT2_MOUNT_DAX) {
>>>    		if (!bdev_dax_supported(sb->s_bdev, blocksize)) {
>>>
>>