Re: [PATCH v7 27/31] ext4: set DISKSIZE_GROW_PENDING after zeroing unaligned EOF block

[Zhang Yi <yizhang089@gmail.com>]

On 10/9/2026 7:21 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] ext4: torn lockless read of i_disksize causes missing DISKSIZE_GROW_PENDING
> --
> 
> commit 4a76a3a815e11b477d4aaf7fd5a0eb69623ddb31
> Author: Zhang Yi <yi.zhang@huawei.com>
> 
> ext4: set DISKSIZE_GROW_PENDING after zeroing unaligned EOF block
> 
> This commit adds the ext4_iomap_mark_disksize_pending() function to set
> EXT4_STATE_DISKSIZE_GROW_PENDING after zeroing an unaligned EOF block.
> It ensures that the zeroed block is written before any subsequent data
> updates i_disksize, preventing the exposure of stale data.
> 
>> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
>> index 9b6916609000c..3bdb5f3b119ab 100644
>> --- a/fs/ext4/inode.c
>> +++ b/fs/ext4/inode.c
>> @@ -4847,6 +4847,89 @@ static int ext4_block_zero_range(struct inode *inode,
> 
> [ ... ]
> 
>> +	/*
>> +	 * If the zeroed range does not overlap the existing on-disk tail
>> +	 * block, the zeroed data lies beyond the currently on-disk data
>> +	 * and will be written back before i_disksize is advanced past it,
>> +	 * so no stale data can be exposed.
>> +	 *
>> +	 * Checking i_disksize here (after folio_wait_writeback()) is
>> +	 * necessary and safe. If a delalloc writeback of this folio was
>> +	 * in-flight, it could be raced by a concurrent mmap write which
>> +	 * corrupts the tail block beyond i_size but the i_disksize is not
>> +	 * advanced. folio_wait_writeback() has waited for its completion
>> +	 * and the ioend has advanced i_disksize accordingly. If writeback
>> +	 * had not started, we don't need to mark any pending state because
>> +	 * any future writeback will carry the pagecache content that now
>> +	 * includes the zeroed data, so no stale data can appear on disk
>> +	 * even without the pending tracking.
>> +	 */
>> +	if (from >= round_up(READ_ONCE(EXT4_I(inode)->i_disksize),
>> +			     i_blocksize(inode)))
>> +		goto out;
> 
> [Severity: Medium]
> Could this lockless read of a 64-bit integer suffer from a torn read on
> 32-bit architectures?
> 
> On a 32-bit system, READ_ONCE() on a 64-bit variable like i_disksize compiles
> to two 32-bit loads. If a concurrent thread updates i_disksize via
> WRITE_ONCE() (for example, during I/O completion while holding i_data_sem)
> and the value crosses a 32-bit boundary, this read can tear.
> 
> For instance, if i_disksize changes from 0x00000000_FFFFFFFF to
> 0x00000001_00000000, the torn read could yield 0x00000000_00000000.
> 
> This would cause the condition to evaluate to true, skipping the pending
> tracking and failing to set EXT4_STATE_DISKSIZE_GROW_PENDING.
> 
> If the system crashes, this could allow i_disksize to be advanced past the
> unwritten zeroed block, exposing stale, uninitialized disk data to the
> file tail and defeating the data-ordering mechanism.
> 
> While i_size is protected by i_size_seqcount on 32-bit platforms, i_disksize
> does not seem to have seqcount protection here.
> 

I don't think a torn read is possible here, because there is no
concurrent writer of i_disksize in this window.

ext4_iomap_mark_disksize_pending() is called under i_rwsem, so the
i_disksize read can only be raced by the worker in
ext4_iomap_wb_update_disksize().  But it cannot make i_disksize larger
than what we account for here.  It writes min(end, i_size), and all
callers of ext4_block_zero_eof() zero the tail block before i_size is
advanced, so the new value is at most 'from'.  To change the outcome of
the check, 'end' would have to fall inside the block containing 'from',
and the only ioend that can do that is the one covering the folio we
have already waited for with folio_wait_writeback().  So the value we
read is already settled.

Thanks,
Yi.