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

[sashiko-bot@kernel.org]

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.

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

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