Re: [PATCH -next v5 23/32] ext4: submit and wait for pending disksize-grow I/O on writeback

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [Critical] Unconditional clearing of the disksize-grow-pending state on I/O error exposes stale data.
- [Critical] The iomap writeback path queues `struct iomap_ioend` to `ei->i_rsv_conversion_list`, but the work item executes `ext4_end_io_rsv_work` expecting `ext4_io_end_t`, causing memory corruption.
- [High] Proactive submission of the EOF block issues a WB_SYNC_ALL writeback pass, violating WB_SYNC_NONE non-blocking guarantees.
- [High] The EXT4_STATE_DISKSIZE_GROW_PENDING bit is never cleared when a file is truncated down or punched, leading to a permanent deadlock in ext4_iomap_wb_disksize_pending_wait on subsequent writes.
- [Medium] The EXT4_STATE_DISKSIZE_GROW_PENDING flag is never set anywhere in the codebase.
--

--- Patch [23]: [PATCH -next v5 23/32] ext4: submit and wait for pending disksize-grow I/O on writeback ---
commit 8550774845865922b3c12e81e62fba6b64136065
Author: Zhang Yi <yi.zhang@huawei.com>
Subject: ext4: submit and wait for pending disksize-grow I/O on writeback

This patch introduces logic to submit and wait for pending zeroed EOF blocks
that straddle i_disksize during writeback to prevent exposing stale data.
It tags ioends with EXT4_IOMAP_IOEND_DISKSIZE_GROW_IO and proactively submits
the pending ranges.

> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index e4a4396eaf879..a0707310b464c 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -4379,6 +4382,21 @@ static int ext4_iomap_writeback_submit(struct iomap_writepage_ctx *wpc,
>  	    test_opt(ioend->io_inode->i_sb, DATA_ERR_ABORT))
>  		ioend->io_bio.bi_end_io = ext4_iomap_end_bio;
>  
> +	/*
> +	 * Mark the I/O as DISKSIZE_GROW_IO by setting io_private to
> +	 * EXT4_IOMAP_IOEND_DISKSIZE_GROW_IO if it covers the pending range.
> +	 * Such I/O will allow or trigger i_disksize advancement in the
> +	 * ioend worker.
> +	 */
> +	plen = ext4_iomap_get_disksize_pending_range(inode, &pstart);

[Severity: Medium]
Does this actually get triggered? A codebase search shows that the flag
EXT4_STATE_DISKSIZE_GROW_PENDING is never actually set anywhere (for example,
when zeroing the tail of the EOF block). Because the flag is never set,
ext4_iomap_get_disksize_pending_range() will always return 0, which seems to
completely bypass the newly added logic.

> +	if (plen &&
> +	    round_down(ioend->io_offset, blocksize) <= pstart &&
> +	    round_up(ioend->io_offset + ioend->io_size, blocksize) >=
> +			pstart + plen) {
> +		ioend->io_bio.bi_end_io = ext4_iomap_end_bio;
> +		ioend->io_private = (void *)EXT4_IOMAP_IOEND_DISKSIZE_GROW_IO;
> +	}
> +

[ ... ]

> @@ -4398,6 +4416,29 @@ static const struct iomap_writeback_ops ext4_writeback_ops = {
>  	.writeback_submit = ext4_iomap_writeback_submit,
>  };
>  
> +/*
> + * If the current writeback range begins after the pending zeroed EOF
> + * block range which straddles i_disksize, issue a separate writeback to
> + * flush it first, so as to avoid prolonged waiting.
> + */
> +static void ext4_iomap_wb_submit_zeroed_eof(struct inode *inode,
> +					    struct writeback_control *wbc)
> +{
> +	struct address_space *mapping = inode->i_mapping;
> +	loff_t pstart, plen, range_start;
> +
> +	if (wbc->range_cyclic)
> +		range_start = (loff_t)mapping->writeback_index << PAGE_SHIFT;
> +	else
> +		range_start = wbc->range_start;
> +
> +	plen = ext4_iomap_get_disksize_pending_range(inode, &pstart);
> +	if (!plen || range_start < pstart + plen)
> +		return;
> +
> +	filemap_fdatawrite_range(mapping, pstart, pstart + plen - 1);

[Severity: High]
Does this violate the WB_SYNC_NONE guarantees when called by the background
flusher? 

filemap_fdatawrite_range() initiates a WB_SYNC_ALL writeback pass. If
ext4_iomap_writepages() is invoked with wbc->sync_mode == WB_SYNC_NONE,
this nested WB_SYNC_ALL call will synchronously block waiting on page locks
and in-flight I/O. This could stall the background flusher and potentially
cause deadlocks if the thread holding the lock is blocked in memory reclaim
waiting for the flusher to free pages.

> +}
> +

[ ... ]

> diff --git a/fs/ext4/page-io.c b/fs/ext4/page-io.c
> index 2888e00575615..955ff88045db6 100644
> --- a/fs/ext4/page-io.c
> +++ b/fs/ext4/page-io.c
> @@ -549,6 +549,34 @@ void ext4_bio_write_folio(struct ext4_io_submit *io, struct folio *folio,
>  	} while ((bh = bh->b_this_page) != head);
>  }
>  
> +/*
> + * If the current writeback range starts beyond the zeroed EOF pending
> + * range that straddles i_disksize, wait for the zeroed data from
> + * ext4_block_zero_eof() to be written out first. Otherwise, extending
> + * i_disksize may expose stale data in the old EOF block.
> + */
> +static void ext4_iomap_wb_disksize_pending_wait(struct inode *inode,
> +						loff_t pos, size_t size)
> +{
> +	loff_t disksize = READ_ONCE(EXT4_I(inode)->i_disksize);
> +	loff_t pstart, plen;
> +
> +	/*
> +	 * Overwrite I/Os and I/Os covering the EOF block do not need to
> +	 * wait: the former do not advance i_disksize past the pending
> +	 * boundary, and the latter are the pending I/O itself (cleared in
> +	 * the bio completion path).
> +	 */
> +	if (pos < round_up(disksize, i_blocksize(inode)))
> +		return;
> +
> +	plen = ext4_iomap_get_disksize_pending_range(inode, &pstart);
> +	if (!plen || pos < pstart + plen)
> +		return;
> +
> +	ext4_iomap_wait_disksize_pending(inode);

[Severity: High]
Could this wait indefinitely if the file is truncated down or punched?

If EXT4_STATE_DISKSIZE_GROW_PENDING is set, but no code clears it during
ext4_truncate() or ext4_punch_hole(), the zeroed EOF page will be removed
from the page cache while the bit remains set. If new data is subsequently
written past the stale pending range, the writeback completion might call
ext4_iomap_wb_disksize_pending_wait(). 

Since the original page was dropped, no I/O will ever complete to clear the
bit, causing the worker thread to hang forever in the wait_on_bit call and
permanently blocking the rsv_conversion_wq.

> +}
> +

[ ... ]

> @@ -660,8 +691,17 @@ void ext4_iomap_end_bio(struct bio *bio)
>  {
>  	struct iomap_ioend *ioend = iomap_ioend_from_bio(bio);
>  	struct ext4_inode_info *ei = EXT4_I(ioend->io_inode);
> +	unsigned long io_mode = (unsigned long)ioend->io_private;
>  	unsigned long flags;
>  
> +	/*
> +	 * This is a disksize-pending I/O: clear the disksize-pending
> +	 * state set in ext4_block_zero_eof() and wake up all waiters
> +	 * that will update the inode i_disksize.
> +	 */
> +	if (io_mode == EXT4_IOMAP_IOEND_DISKSIZE_GROW_IO)
> +		ext4_iomap_clear_disksize_pending(ioend->io_inode);

[Severity: Critical]
Does clearing the pending state here expose stale data on I/O errors?

If the writeback fails, the pending state is unconditionally cleared here,
waking up waiters in ext4_iomap_wb_disksize_pending_wait(). Because those
waiters only check their own bio status, they could proceed to advance
i_disksize past the failed EOF block in ext4_iomap_finish_ioend(). Advancing
i_disksize over the failed zeroing operation exposes the stale data previously
present in the block's tail on disk.

> +
>  	spin_lock_irqsave(&ei->i_completed_io_lock, flags);
>  	if (list_empty(&ei->i_rsv_conversion_list))
>  		queue_work(EXT4_SB(ioend->io_inode->i_sb)->rsv_conversion_wq,
>  			   &ei->i_rsv_conversion_work);
>  	list_add_tail(&ioend->io_list, &ei->i_rsv_conversion_list);

[Severity: Critical]
Can this queueing lead to memory corruption due to type confusion?

A struct iomap_ioend is added to ei->i_rsv_conversion_list here and
ei->i_rsv_conversion_work is scheduled. However, this work item is
permanently bound in ext4_alloc_inode() to execute ext4_end_io_rsv_work().

When the workqueue executes ext4_end_io_rsv_work(), it iterates over the
list and improperly casts the struct iomap_ioend entries to ext4_io_end_t.
Accessing fields from this miscast pointer will retrieve garbage data,
leading to a panic or use-after-free.

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