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

Zhang Yi <[email protected]>
Newsgroups org.kernel.vger.linux-ext4,dev.linux.lists.sashiko-reviews
Message-ID <[email protected]>
On 8/14/2026 6:08 PM, [email protected] wrote:
> 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?
> 

Yeah, this is an pre-existing issue. In DAX mode, if issue zero fails,
we should clean up the allocated blocks on the error path.

Thanks,
Yi.
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.