Re: [PATCH v6 03/31] ext4: skip ordered I/O wait when zeroing beyond i_disksize block

[Zhang Yi <yi.zhang@huaweicloud.com>]

On 9/3/2026 9:14 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Skipping ordered I/O wait based on an unstabilized `i_disksize` allows stale data exposure due to a race with concurrent writeback and mmap.
> --
> 
> commit 9f1f97ff9aa74fccfe7305cebcde6a641f847ab5
> Author: Zhang Yi <yi.zhang@huawei.com>
> 
> ext4: skip ordered I/O wait when zeroing beyond i_disksize block
> 
> ext4_block_zero_eof() zeros the tail of a partial block beyond EOF.
> After zeroing, it waits for ordered I/O completion to prevent stale
> data exposure from concurrent post-EOF mmap writes during folio
> writeback.
> 
> However, if the zeroed range lies entirely beyond the block containing
> i_disksize, no stale data can be exposed because the zeroed region is
> beyond existing on-disk data. The zeroed pages will be written out
> before i_disksize is later extended past i_size, so the ordered I/O
> wait is unnecessary. Add a condition to skip it.
> 
>> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
>> index d0ccafa554f8d..339ebd553959b 100644
>> --- a/fs/ext4/inode.c
>> +++ b/fs/ext4/inode.c
>> @@ -4241,9 +4241,22 @@ int ext4_block_zero_eof(struct inode *inode, loff_t from, loff_t end)
> 
> [ ... ]
> 
>> +	 * Note that it's safe to read i_disksize without holding i_data_sem
>> +	 * here. Since we already hold i_rwsem, the only possible race is with
>> +	 * concurrent writeback that updates i_disksize. And if such a race
>> +	 * occurs, it means the previous unaligned EOF block has already been
>> +	 * zeroed (if needed) and persisted to disk.
>>  	 */
>>  	if (ext4_should_order_data(inode) &&
>> -	    did_zero && zero_written && !IS_DAX(inode)) {
>> +	    did_zero && zero_written && !IS_DAX(inode) &&
>> +	    from < round_up(READ_ONCE(EXT4_I(inode)->i_disksize), blocksize)) {
> 
> [Severity: High]
> Does this condition rely on an unstabilized i_disksize to skip the ordered
> I/O wait, creating a window where in-flight writeback IO can leak stale data
> upon a crash?
> 
> If a file has a delayed allocated block at the end, its on-disk size
> (i_disksize) could be lagging (e.g., 0). If the user mmaps the file and
> writes data past EOF into the page cache, delayed allocation writeback can
> begin, allocating the block and formatting a BIO with the user's stale
> post-EOF data.
> 
> Concurrently, if a truncate operation extends the file (truncate up), it
> calls ext4_block_zero_eof(), which zeroes the page cache tail and marks it
> dirty. However, because READ_ONCE(EXT4_I(inode)->i_disksize) is 0, the
> check evaluates to false.
> 
> This incorrectly skips ext4_jbd2_inode_add_write(), failing to add the
> newly zeroed data to the ordered data list. The truncate operation then
> sets i_disksize to the new size and commits the transaction, waiting only
> for the first BIO (containing the stale data), not the zeroes.
> 
> If a crash occurs, upon recovery, the on-disk block would contain the stale
> data because the transaction committed the new i_disksize without waiting
> for the zeroed page cache to be written.
> 
> Note that this issue does not appear to be resolved by follow-up patches in
> the series. While commit "ext4: set DISKSIZE_GROW_PENDING after zeroing
> unaligned EOF block" introduces ext4_iomap_mark_disksize_pending() which
> safely waits for writeback, it is placed inside the exact same flawed
> condition branch, meaning the wait is still bypassed.

I think the concurrency scenario that Sashiko mentioned does exist, so it
looks like we'll have to abandon this optimization.

Thanks,
Yi.

> 
>>  		handle_t *handle;
>>  
>>  		handle = ext4_journal_start(inode, EXT4_HT_MISC, 1);
>