Re: [PATCH] ext4: skip eof zeroing when the zero range covers the eof block

[Baokun Li <libaokun@linux.alibaba.com>]

On 2026/10/22 15:16, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Critical] ext4: stale data in ext4_alloc_file_blocks() on FALLOC_FL_WRITE_ZEROES
> --
>
> commit 5e26fed6c5f3f56bd5013064a243a3a558e3a73d
> Author: Baokun Li <libaokun@linux.alibaba.com>
>
> ext4: skip eof zeroing when the zero range covers the eof block
>
> This commit skips calling ext4_block_zero_eof() in ext4_alloc_file_blocks()
> when EXT4_GET_BLOCKS_CONVERT_UNWRITTEN is set and the allocation range covers
> the EOF block, preventing the dropped EOF page from being read back into the
> page cache with stale data during FALLOC_FL_ZERO_RANGE.
>
>> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
>> index 7193429a05e1f..8f7ded6e08299 100644
>> --- a/fs/ext4/extents.c
>> +++ b/fs/ext4/extents.c
>> @@ -4639,8 +4639,15 @@ static int ext4_alloc_file_blocks(struct file *file, loff_t offset, loff_t len,
>>  	credits = ext4_chunk_trans_blocks(inode, len_lblk);
>>  	depth = ext_depth(inode);
>>  
>> -	/* Zero to the end of the block containing i_size */
>> -	if (new_size > old_size) {
>> +	/*
>> +	 * Zero to the end of the block containing i_size.
>> +	 * Loading the eof block back before it is converted to unwritten
>> +	 * may leave the page cache inconsistent with the disk, and zeroing
>> +	 * it is redundant, so skip it.
>> +	 */
>> +	if (new_size > old_size &&
>> +	    (!(flags & EXT4_GET_BLOCKS_CONVERT_UNWRITTEN) ||
>> +	     !in_range(old_size >> blkbits, map.m_lblk, map.m_len))) {
>>  		ret = ext4_block_zero_eof(inode, old_size, LLONG_MAX);
>>  		if (ret)
>>  			return ret;
> [Severity: Critical]
> Does this condition also need to check !alloc_zero to cover the
> FALLOC_FL_WRITE_ZEROES path?
>
> In ext4_zero_range(), after ext4_truncate_page_cache_block_range() drops the
> page cache, FALLOC_FL_WRITE_ZEROES calls ext4_alloc_file_blocks() with
> EXT4_GET_BLOCKS_CREATE_ZERO rather than EXT4_GET_BLOCKS_CONVERT_UNWRITTEN:
>
> 	if (align_end > align_start) {
> 		if (mode & FALLOC_FL_WRITE_ZEROES)
> 			flags = EXT4_GET_BLOCKS_CREATE_ZERO | EXT4_EX_NOCACHE;
> 		else
> 			flags |= (EXT4_GET_BLOCKS_CONVERT_UNWRITTEN |
> 				  EXT4_EX_NOCACHE);
> 		ret = ext4_alloc_file_blocks(file, align_start,
> 					     align_end - align_start, new_size,
> 					     flags);
>
> Earlier in ext4_alloc_file_blocks(), EXT4_GET_BLOCKS_ZERO is cleared from
> flags and converted into alloc_zero = true:
>
> 	if (flags & EXT4_GET_BLOCKS_ZERO) {
> 		flags &= ~EXT4_GET_BLOCKS_ZERO;
> 		flags |= EXT4_GET_BLOCKS_UNWRIT_EXT;
> 		alloc_zero = true;
> 	}
>
> When FALLOC_FL_WRITE_ZEROES extends i_size across an unaligned written EOF
> block covered by [align_start, align_end),
> !(flags & EXT4_GET_BLOCKS_CONVERT_UNWRITTEN) is true. As a result,
> ext4_block_zero_eof() is still called and reads the dropped EOF block back
> from disk into the page cache with stale data below old_size, marks the
> buffer or folio dirty, and then the loop zeroes the block on disk via
> ext4_issue_zeroout() and leaves the extent mapped as written.
>
> Wouldn't this leave the dirty page cache folio with stale data that is
> returned on buffered reads and written back over the zeroed disk block on
> writeback or fsync?
>

This is indeed another similar issue that can be addressed
in the current patch. I will send version 2 soon.

Thanks,
Baokun