Re: [PATCH -next v5 07/32] ext4: fix orig_mlen initialization in ext4_map_blocks()

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] If ext4_issue_zeroout() fails during block allocation with EXT4_GET_BLOCKS_ZERO (e.g. via DAX hole-filling), the newly allocated WRITTEN extent is not reverted, exposing uninitialized disk data.
--

--- Patch [7]: [PATCH -next v5 07/32] ext4: fix orig_mlen initialization in ext4_map_blocks() ---
commit 547294dd62d03f6ea5caef0d90e340b0af933eae
Author: Zhang Yi <[email protected]>

ext4: fix orig_mlen initialization in ext4_map_blocks()

This commit fixes an overflow protection bypass by ensuring orig_mlen
is saved only after clamping map->m_len to INT_MAX.

> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c

[ ... ]

> @@ -719,6 +719,7 @@ int ext4_map_blocks(handle_t *handle, struct inode *inode,
>  	 */
>  	if (unlikely(map->m_len > INT_MAX))
>  		map->m_len = INT_MAX;
> +	orig_mlen = map->m_len;

[Severity: High]
This is a pre-existing issue, but while reviewing the block mapping code in
fs/ext4/inode.c, I noticed a potential data exposure when DAX hole-filling
writes fail to zero blocks.

If ext4_issue_zeroout() fails in ext4_map_create_blocks(), the error is
returned immediately without reverting the extent tree modification:

fs/ext4/inode.c:ext4_map_create_blocks() {
    ...
    if (flags & EXT4_GET_BLOCKS_ZERO &&
        map->m_flags & EXT4_MAP_MAPPED && map->m_flags & EXT4_MAP_NEW) {
        err = ext4_issue_zeroout(inode, map->m_lblk, map->m_pblk, map->m_len);
        if (err)
            return err;
    }
    ...
}

This error propagates back to ext4_iomap_alloc(), which aborts the operation
but proceeds to commit the transaction anyway:

fs/ext4/inode.c:ext4_iomap_alloc() {
    ...
    if (ret < 0) {
        ext4_journal_stop(handle);
        return ret;
    }
    ...
}

For a DAX write to a hole within EOF, the extend flag evaluates to false in
ext4_dax_write_iter(). This completely bypasses the cleanup path that would
otherwise drop the failed extent:

fs/ext4/file.c:ext4_dax_write_iter() {
    ...
    if (extend) {
        ret = ext4_handle_inode_extension(inode, offset, ret, count);
        ext4_inode_extension_cleanup(inode, ret < (ssize_t)count);
    }
    ...
}

Because the newly allocated extent was already inserted into the extent tree
as WRITTEN by ext4_ext_map_blocks() earlier in the call chain, could this
sequence leave uninitialized disk data mapped and exposed to userspace?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=7