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

On 8/14/2026 6:25 PM, [email protected] wrote:
> 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 <[email protected]>
> 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.

This is a false positive. EXT4_STATE_DISKSIZE_GROW_PENDING is set
in patch 28. This patch is just preparing for that functionality.

> 
>> +	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.
> 

You're right. Although I don't quite see how the deadlock would
happen, keeping the original writeback mode is definitely safer.
I'll change it as follows:

/* Keep the caller's sync mode to avoid stalling the background flusher. */
if (wbc->sync_mode == WB_SYNC_ALL)
         filemap_fdatawrite_range(mapping, pstart, pstart + plen - 1);
else
         filemap_flush_range(mapping, pstart, pstart + plen - 1);


>> +}
>> +
> 
> [ ... ]
> 
>> 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.
> 

This is a false positive. This case will be handled in patch 27.


>> +}
>> +
> 
> [ ... ]
> 
>> @@ -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.

This is expected. Even in the original data=ordered mode, a data
writeback failure does not prevent i_disksize from being updated.
The ability to detect data write failures relies on data_err=abort,
which is consistent with the previous behavior.

> 
>> +
>>   	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.
> 

This is a false positive. i_rsv_conversion_work is initialized
in patch 31, where the iomap path is formally enabled.

Thanks,
Yi.