Re: [PATCH v6 09/31] ext4: skip block allocation for holes in the data submission path

[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 1 potential issue(s) to consider:
> - [Critical] mpage_map_one_extent() fails to check for err == 0 from ext4_map_blocks(), leading to dirty buffers being mapped to block 0 and silently overwriting the filesystem superblock on inconsistencies.
> --
> 
> commit 0c0032b57a5cee4a0ac28f30d5e6fc9a53e4e923
> Author: Zhang Yi <yi.zhang@huawei.com>
> 
> ext4: skip block allocation for holes in the data submission path
> 
> This commit modifies ext4_map_blocks() to return 0 without allocating blocks
> when encountering a hole during the data submission path. It aims to discard
> dirty bits for holes without submitting I/O.
> 
>> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
>> index 1473263dd82f1..81c4625de752a 100644
>> --- a/fs/ext4/inode.c
>> +++ b/fs/ext4/inode.c
>> @@ -825,6 +825,13 @@ int ext4_map_blocks(handle_t *handle, struct inode *inode,
>>   			map->m_flags |= EXT4_MAP_MAPPED;
>>   			goto out_handle;
>>   		}
>> +	} else if (retval == 0) {
>> +		/*
>> +		 * Do not allocate blocks for holes in the context of
>> +		 * data submission path.
>> +		 */
>> +		if (!map->m_flags && (flags & EXT4_GET_BLOCKS_IO_SUBMIT))
>> +			goto out_handle;
> 
> [Severity: Critical]
> Does this new return path create an unhandled condition in
> mpage_map_one_extent() for the buffer_head writeback path?
> 
> When ext4_map_blocks() is called by mpage_map_one_extent() and returns 0 due
> to this new logic, the caller only checks for err < 0:
> 
> fs/ext4/inode.c:mpage_map_one_extent() {
> 	...
> 	err = ext4_map_blocks(handle, inode, map, get_blocks_flags);
> 	if (err < 0)
> 		return err;
> 	...
> }
> 
> Because err == 0 is ignored, mpage_map_one_extent() proceeds as if blocks
> were successfully allocated. The zero m_pblk is then used by
> mpage_map_and_submit_buffers(), which sets bh->b_blocknr = 0 and clears the
> delay flag.
> 
> Could this result in ext4_bio_write_folio() submitting I/O to the mapped
> physical block 0 and silently overwriting the filesystem superblock on
> inconsistencies?
> 

This should not happen in practice. mpage_add_bh_to_extent() only
collects delayed or unwritten buffers into the extent to map. Hole
buffers are filtered out there and never reach ext4_map_blocks(), so
when mpage_map_one_extent() runs, ext4_map_blocks() can only return a
positive number on allocation or a negative error, never zero.

That said, I agree a defensive check is better than relying on that
invariant alone. I'll add the following to guard against an unexpected
hole.

	/*
	 * A hole? This should never happen since mpage_add_bh_to_extent()
	 * has filtered it out.
	 */
	if (WARN_ON_ONCE(!err))
			return -EINVAL;

Thanks,
Yi

>>   	}
>>   
>>   	if (!handle) {
>