linux-ext4 - Re: [PATCH v3] ext4: increase i_disksize to offset + len in ext4_update_disksize_before

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]

Message-ID: <d82a3e6f-25a2-4943-9e97-73337b33cfcc@huawei.com>
Date: Thu, 11 Sep 2025 09:10:07 +0800
From: Sun Yongjian <sunyongjian1@...wei.com>
To: Jan Kara <jack@...e.cz>
CC: <linux-ext4@...r.kernel.org>, <linux-fsdevel@...r.kernel.org>,
	<tytso@....edu>, <yangerkun@...wei.com>, <yi.zhang@...wei.com>,
	<libaokun1@...wei.com>, <chengzhihao1@...wei.com>
Subject: Re: [PATCH v3] ext4: increase i_disksize to offset + len in
 ext4_update_disksize_before_punch()



在 2025/9/10 16:11, Jan Kara 写道:
> On Wed 10-09-25 12:25:16, Yongjian Sun wrote:
>> From: Yongjian Sun <sunyongjian1@...wei.com>
>>
>> After running a stress test combined with fault injection,
>> we performed fsck -a followed by fsck -fn on the filesystem
>> image. During the second pass, fsck -fn reported:
>>
>> Inode 131512, end of extent exceeds allowed value
>> 	(logical block 405, physical block 1180540, len 2)
>>
>> This inode was not in the orphan list. Analysis revealed the
>> following call chain that leads to the inconsistency:
>>
>>                               ext4_da_write_end()
>>                                //does not update i_disksize
>>                               ext4_punch_hole()
>>                                //truncate folio, keep size
>> ext4_page_mkwrite()
>>   ext4_block_page_mkwrite()
>>    ext4_block_write_begin()
>>      ext4_get_block()
>>       //insert written extent without update i_disksize
>> journal commit
>> echo 1 > /sys/block/xxx/device/delete
>>
>> da-write path updates i_size but does not update i_disksize. Then
>> ext4_punch_hole truncates the da-folio yet still leaves i_disksize
>> unchanged(in the ext4_update_disksize_before_punch function, the
>> condition offset + len < size is met). Then ext4_page_mkwrite sees
>> ext4_nonda_switch return 1 and takes the nodioread_nolock path, the
>> folio about to be written has just been punched out, and it’s offset
>> sits beyond the current i_disksize. This may result in a written
>> extent being inserted, but again does not update i_disksize. If the
>> journal gets committed and then the block device is yanked, we might
>> run into this. It should be noted that replacing ext4_punch_hole with
>> ext4_zero_range in the call sequence may also trigger this issue, as
>> neither will update i_disksize under these circumstances.
>>
>> To fix this, we can modify ext4_update_disksize_before_punch to
>> increase i_disksize to min(offset + len) when both i_size and
>> (offset + len) are greater than i_disksize.
>>
>> Signed-off-by: Yongjian Sun <sunyongjian1@...wei.com>
>> ---
>> Changes in v3:
>> - Add a condition to avoid increasing i_disksize and include some comments.
>> - Link to v2: https://lore.kernel.org/all/20250908063355.3149491-1-sunyongjian@huaweicloud.com/
>> Changes in v2:
>> - The modification of i_disksize should be moved into ext4_update_disksize_before_punch,
>>    rather than being done in ext4_page_mkwrite.
>> - Link to v1: https://lore.kernel.org/all/20250731140528.1554917-1-sunyongjian@huaweicloud.com/
> 
> Very nice! Just some language improvements below but otherwise feel free to
> add:
> 
> Reviewed-by: Jan Kara <jack@...e.cz>
> 
>> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
>> index 5b7a15db4953..3df03469d405 100644
>> --- a/fs/ext4/inode.c
>> +++ b/fs/ext4/inode.c
>> @@ -4287,7 +4287,10 @@ int ext4_can_truncate(struct inode *inode)
>>    * We have to make sure i_disksize gets properly updated before we truncate
>>    * page cache due to hole punching or zero range. Otherwise i_disksize update
>>    * can get lost as it may have been postponed to submission of writeback but
>> - * that will never happen after we truncate page cache.
>> + * 1) that will never happen after we truncate page cache to the end of i_size;
>> + * 2) that will get deferred after we truncate page cache in i_size but beyond
>> + *    i_disksize, another concurrent write page fault can allocate written
>> + *    blocks in the range and lead to filesystem inconsistency.
> 
> I'd phrase this:
>   ... that will never happen if we remove the folio containing i_size from
> the page cache. Also if we punch hole within i_size but above i_disksize,
> following ext4_page_mkwrite() may mistakenly allocate written blocks over
> the hole and thus introduce allocated blocks beyond i_disksize which is not
> allowed (e2fsck would complain in case of crash).
> 
> 								Honza
> 
Thank you, Jan! This does make it simpler and easier to understand. I'll 
improve it.

>>    */
>>   int ext4_update_disksize_before_punch(struct inode *inode, loff_t offset,
>>   				      loff_t len)
>> @@ -4298,9 +4301,11 @@ int ext4_update_disksize_before_punch(struct inode *inode, loff_t offset,
>>   	loff_t size = i_size_read(inode);
>>   
>>   	WARN_ON(!inode_is_locked(inode));
>> -	if (offset > size || offset + len < size)
>> +	if (offset > size)
>>   		return 0;
>>   
>> +	if (offset + len < size)
>> +		size = offset + len;
>>   	if (EXT4_I(inode)->i_disksize >= size)
>>   		return 0;
>>   
>> -- 
>> 2.39.2
>>