Re: [PATCH AUTOSEL 6.18-5.10] ext4: check return value of ext4_get_block() in ext4_load_tail_bh()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] ext4: unchecked ext4_block_truncate_page() return in truncate callers
--

commit 26fe59dd3c7737fcb95b3fe2c9732b862d57565e
Author: Zhang Yi <yi.zhang@huawei.com>

ext4: check return value of ext4_get_block() in ext4_load_tail_bh()

This patch checks and propagates the return value of ext4_get_block() in
__ext4_block_zero_page_range() so that I/O or allocation failures are not
silently ignored as holes during zero-range, truncate, or punch-hole
operations.

Link: https://patch.msgid.link/20260714080044.4038124-4-yi.zhang@huaweicloud.com

> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index e99306a8f47ce..234dfe20ef4f3 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -4092,7 +4092,9 @@ static int __ext4_block_zero_page_range(handle_t *handle,
>  	}
>  	if (!buffer_mapped(bh)) {
>  		BUFFER_TRACE(bh, "unmapped");
> -		ext4_get_block(inode, iblock, bh, 0);
> +		err = ext4_get_block(inode, iblock, bh, 0);
> +		if (err < 0)
> +			goto unlock;
>  		/* unmapped? It's a hole - nothing to do */
>  		if (!buffer_mapped(bh)) {
>  			BUFFER_TRACE(bh, "still unmapped");

[Severity: Medium]
While this propagates the negative error from ext4_get_block() out of
__ext4_block_zero_page_range() via ext4_block_zero_page_range(),
ext4_block_truncate_page(), and ext4_zero_partial_blocks(), several
truncate and fallocate callers ignore the returned error.

In fs/ext4/inode.c, a shrinking truncate via
ext4_setattr()->ext4_truncate()->ext4_block_truncate_page() ignores the
return value and proceeds with block truncation:

	if (inode->i_size & (inode->i_sb->s_blocksize - 1))
		ext4_block_truncate_page(handle, mapping, inode->i_size);

Similarly, an extending truncate in ext4_setattr() ignores the error from
ext4_block_truncate_page() and proceeds to update i_disksize and i_size:

	if (!shrink) {
		inode_set_mtime_to_ts(inode,
				      inode_set_ctime_current(inode));
		if (oldsize & (inode->i_sb->s_blocksize - 1))
			ext4_block_truncate_page(handle,
					inode->i_mapping, oldsize);
	}

And in fs/ext4/extents.c, an extending fallocate or zero-range
preallocation in ext4_alloc_file_blocks() ignores the error returned by
ext4_zero_partial_blocks() when zeroing the old EOF tail block:

	if (epos > old_size) {
		pagecache_isize_extended(inode, old_size, epos);
		ext4_zero_partial_blocks(handle, inode,
				     old_size, epos - old_size);
	}

When the tail buffer_head is unmapped and ext4_get_block() fails, can
these paths still silently discard the error, update the inode size, and
return success with stale data left in the partial tail block?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261019135441.1420968-141-sashal@kernel.org?part=1