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

[sashiko-bot@kernel.org]

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?

[ ... ]

> @@ -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?

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