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

[sashiko-bot@kernel.org]

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.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261009103033.2920530-1-yi.zhang@huaweicloud.com?part=27