Re: [PATCH v6 26/31] ext4: clear DISKSIZE_GROW_PENDING on truncate or error

[Zhang Yi <yizhang089@gmail.com>]

On 9/3/2026 9:46 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] Unconditional advancement of i_disksize in ext4_truncate_down exposes unwritten data and corrupts the disksize pending state.
> - [Medium] EXT4_STATE_DISKSIZE_GROW_PENDING is not cleared when evicting an inode with i_nlink > 0 or a bad inode, leading to a spurious WARN_ON_ONCE in ext4_destroy_inode().
> --
> 
> commit ce48fe2572983c4095fed45d85327a228bcb48f0
> Author: Zhang Yi <yi.zhang@huawei.com>
> 
> ext4: clear DISKSIZE_GROW_PENDING on truncate or error
> 
> This commit clears the DISKSIZE_GROW_PENDING state on error paths, such
> as folio discard and unlinked inode eviction, as well as during truncation.
> It aims to prevent writeback from blocking indefinitely when a zeroed EOF
> block is discarded, and adds a warning if an inode is destroyed with the
> bit still set.
> 
>> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
>> index 173ac69e9b953..f38437fa91064 100644
>> --- a/fs/ext4/inode.c
>> +++ b/fs/ext4/inode.c
> 
> [ ... ]
> 
>> @@ -273,6 +273,8 @@ void ext4_evict_inode(struct inode *inode)
>>   
>>   	if (ext4_should_order_data(inode))
>>   		ext4_begin_ordered_truncate(inode, 0);
>> +	if (ext4_inode_buffered_iomap(inode))
>> +		ext4_iomap_clear_disksize_pending(inode);
>>   	truncate_inode_pages_final(&inode->i_data);
>>   
> 
> [Severity: Medium]
> Does this miss clearing the pending bit for linked inodes (i_nlink > 0)
> or bad inodes?
> 
> If writeback fails for an inode with a pending zeroed EOF block, and the
> filesystem is unmounted, evict_inodes() unconditionally evicts all inodes.
> Earlier in ext4_evict_inode(), the code truncates pages for linked inodes
> and then jumps to the no_delete label:
> 
> 	if (inode->i_nlink) {
> 		...
> 		truncate_inode_pages_final(&inode->i_data);
> 		...
> 		goto no_delete;
> 	}
> 
> This jump bypasses the new ext4_iomap_clear_disksize_pending() cleanup here.
> 
> Could this result in the pending bit remaining set and triggering the new
> WARN_ON_ONCE in ext4_destroy_inode() when the inode is finally destroyed?
> 

This is false positive, the pending bit is always cleared before an
inode can be destroyed:

- ext4_iomap_end_bio() clears it when the pending I/O completes,
   regardless of whether that I/O succeeded, and a failure before
   submission goes through ext4_iomap_discard_folio(), which clears it
   as well.
- An inode with i_nlink > 0 is only evicted after its dirty pages have
   been written out.
- The only case where the pending block is not written back is an I/O
   error or a forced shutdown, and both set EXT4_ERROR_FS / the
   emergency state, which the WARN_ON_ONCE() already skips.

A bad inode can never have the bit set since the bit is set only from
ext4_block_zero_eof() on the buffered I/O path, while make_bad_inode()
is only reached from __ext4_iget() and from the mount-time journal
inode load, both before the inode can be used for any I/O.

> [ ... ]
> 
>> @@ -6765,7 +6776,18 @@ static int ext4_truncate_down(struct inode *inode, loff_t oldsize,
>>   	start_lblk = newsize > 0 ? (newsize - 1) >> inode->i_blkbits : 0;
>>   	ext4_fc_track_range(handle, inode, start_lblk, EXT_MAX_BLOCKS - 1);
>>   
>> -	ext4_set_inode_size(inode, newsize);
>> +	down_write(&EXT4_I(inode)->i_data_sem);
>> +	/*
>> +	 * Truncate the zeroed EOF block invalidates the pending disksize
>> +	 * update, so clear the disksize-grow-pending state.
>> +	 */
>> +	if (ext4_test_inode_state(inode, EXT4_STATE_DISKSIZE_GROW_PENDING) &&
>> +	    (newsize <= EXT4_I(inode)->i_disksize))
>> +		ext4_iomap_clear_disksize_pending(inode);
>> +
>> +	i_size_write(inode, newsize);
>> +	__ext4_set_i_disksize(inode, newsize);
>> +	up_write(&EXT4_I(inode)->i_data_sem);
>>   
>>   	ret = ext4_mark_inode_dirty(handle, inode);
>>   	ext4_journal_stop(handle);
> 
> [Severity: High]
> Could unconditionally advancing i_disksize here expose unwritten data or leak
> the pending bit?
> 
> If ftruncate() is called with a newsize that is smaller than i_size but
> strictly greater than i_disksize, the condition
> (newsize <= EXT4_I(inode)->i_disksize) evaluates to false. The pending bit is
> not cleared.
> 
> However, __ext4_set_i_disksize(inode, newsize) then unconditionally advances
> i_disksize past the safely written data.
> 
> If a crash occurs before the dirty pages up to newsize are written back, will
> this expose stale or uninitialized data on disk since data ordering is not
> guaranteed for the iomap path?
> 
> Additionally, if newsize is block-aligned, would the writeback thread later
> hitting ext4_iomap_get_disksize_pending_range() encounter a WARN_ON_ONCE
> because i_disksize was updated to a block-aligned value, preventing the ioend
> from ever clearing the pending bit?
> 

This is also false positive, __ext4_set_i_disksize() does not advance
i_disksize while the GROW_PENDING bit is set. So with
newsize > i_disksize, i_disksize is left untouched and stays unaligned,
and the WARN_ON_ONCE() in ext4_iomap_get_disksize_pending_range()
cannot trigger.

Keeping the bit in that case is intentional. The new EOF is still inside
the pending zeroed block, so the zeroed data must reach the disk before
i_disksize may be advanced past it.

Thanks,
Yi.