Re: [PATCH v4] ext4: rewrite ext4_convert_inline_data_nolock to make it safer

[shuo chen <1289151713@qq.com>]

On Mon, Aug 31, 2026 at 02:34:54AM +0000, sashiko-bot@kernel.org wrote:
> [Severity: High]
> Does this error path leak a reference to is.iloc.bh?
> 
> If kmalloc() for xattr_buf fails, the code jumps to the error label which does
> not call brelse(is.iloc.bh). Since ext4_get_inode_loc() pins a reference
> on is.iloc.bh, could this result in a permanent buffer head reference leak?
Got it.
> 
> [Severity: High]
> Does hardcoding the extent length to 1 leak extra allocated blocks?
> 
> In ext4_convert_inline_data_nolock(), ext4_mb_new_blocks() may return a
> larger extent due to bigalloc or normalization, passing the total allocated
> blocks as len:
> 
> fs/ext4/inline.c:ext4_convert_inline_data_nolock() {
> 	...
> 	newblock = ext4_mb_new_blocks(handle, &ar, &error);
> 	if (error < 0)
> 		goto out;
> 	allocated_block = EXT4_C2B(sbi, ar.len);
> 	...
> 		error = ext4_set_inline_data_block(handle, inode,
> 				newblock, allocated_block, bh);
> 	...
> }
> 
> If multiple blocks were allocated, the remaining blocks might be permanently
> leaked since they are marked as used but not mapped to the file.
The remaining blocks are stored in i_prealloc_node.
> 
> [Severity: Critical]
> Is it possible this overwrites the error code from ext4_ext_insert_extent()?
> 
> If ext4_ext_insert_extent() fails with an error other than -EDQUOT or
> -ENOSPC, execution falls through to the ext4_xattr_ibody_find() call, which
> could succeed (returning 0) and overwrite the failure. This would cause the
> function to proceed to the success path, clearing EXT4_INODE_INLINE_DATA
> without actually inserting the extent, potentially leading to data loss and
> a block leak.
Got it.
> 
> [Severity: Critical]
> Does this recovery path miss a call to ext4_mark_iloc_dirty()?
> 
> The code restores the inline data into raw_inode->i_block and clears the
> EXT4_INODE_EXTENTS flag, but doesn't mark the inode dirty. Since
> ext4_ext_tree_init() previously dirtied the inode, it might be written to
> disk with an invalid checksum and the EXT4_EXTENTS_FL flag still set in
> raw_inode->i_flags, which could cause metadata corruption.
Got it.
I'll fix these issues and send v5.