[PATCH] ext4: don't free the tmp_inode tree after a partial migration swap

[PATCH] ext4: don't free the tmp_inode tree after a partial migration swap

[Yichong Chen]

ext4_ext_migrate() cleans up the temporary inode when
ext4_ext_swap_inode_data() fails:

	retval = ext4_ext_swap_inode_data(handle, inode, tmp_inode);
	if (retval)
		free_ext_block(handle, tmp_inode);

ext4_ext_swap_inode_data() switches the inode to extents and copies the
tree root into it before it frees the old indirect block map, so it can
return an error after the swap has already happened.  The copied root
points at index blocks that tmp_inode allocated, and free_ext_block()
walks that same tree and frees them, leaving the migrated inode with a
tree node that the block bitmap considers free.

Only free the tmp_inode tree when the inode has not been switched to
extents, which is exactly the case where the swap did not happen.

Fixes: c14c6fd5c56a ("ext4: Add EXT4_IOC_MIGRATE ioctl")
Signed-off-by: Yichong Chen <chenyichong@uniontech.com>
---
 fs/ext4/migrate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/ext4/migrate.c b/fs/ext4/migrate.c
index 2ce587043945..e588b4157604 100644
--- a/fs/ext4/migrate.c
+++ b/fs/ext4/migrate.c
@@ -562,7 +562,7 @@ int ext4_ext_migrate(struct inode *inode)
 		free_ext_block(handle, tmp_inode);
 	else {
 		retval = ext4_ext_swap_inode_data(handle, inode, tmp_inode);
-		if (retval)
+		if (retval && !ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS))
 			/*
 			 * if we fail to swap inode data free the extent
 			 * details of the tmp inode
-- 
2.51.0

Re: [PATCH] ext4: don't free the tmp_inode tree after a partial migration swap

[sashiko-bot]

> ext4_ext_migrate() cleans up the temporary inode when
> ext4_ext_swap_inode_data() fails:
> 
> 	retval = ext4_ext_swap_inode_data(handle, inode, tmp_inode);
> 	if (retval)
> 		free_ext_block(handle, tmp_inode);
> 
> ext4_ext_swap_inode_data() switches the inode to extents and copies the
> tree root into it before it frees the old indirect block map, so it can
> return an error after the swap has already happened.  The copied root
> points at index blocks that tmp_inode allocated, and free_ext_block()
> walks that same tree and frees them, leaving the migrated inode with a
> tree node that the block bitmap considers free.
> 
> Only free the tmp_inode tree when the inode has not been switched to
> extents, which is exactly the case where the swap did not happen.
> 
> Fixes: c14c6fd5c56a ("ext4: Add EXT4_IOC_MIGRATE ioctl")
> Signed-off-by: Yichong Chen <chenyichong@uniontech.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260918101240.3721805-1-chenyichong@uniontech.com?part=1

Re: [PATCH] ext4: don't free the tmp_inode tree after a partial migration swap

[Jan Kara]

On Fri 18-09-26 18:12:40, Yichong Chen wrote:
> ext4_ext_migrate() cleans up the temporary inode when
> ext4_ext_swap_inode_data() fails:
> 
> 	retval = ext4_ext_swap_inode_data(handle, inode, tmp_inode);
> 	if (retval)
> 		free_ext_block(handle, tmp_inode);
> 
> ext4_ext_swap_inode_data() switches the inode to extents and copies the
> tree root into it before it frees the old indirect block map, so it can
> return an error after the swap has already happened.  The copied root
> points at index blocks that tmp_inode allocated, and free_ext_block()
> walks that same tree and frees them, leaving the migrated inode with a
> tree node that the block bitmap considers free.
> 
> Only free the tmp_inode tree when the inode has not been switched to
> extents, which is exactly the case where the swap did not happen.
> 
> Fixes: c14c6fd5c56a ("ext4: Add EXT4_IOC_MIGRATE ioctl")
> Signed-off-by: Yichong Chen <chenyichong@uniontech.com>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/ext4/migrate.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/ext4/migrate.c b/fs/ext4/migrate.c
> index 2ce587043945..e588b4157604 100644
> --- a/fs/ext4/migrate.c
> +++ b/fs/ext4/migrate.c
> @@ -562,7 +562,7 @@ int ext4_ext_migrate(struct inode *inode)
>  		free_ext_block(handle, tmp_inode);
>  	else {
>  		retval = ext4_ext_swap_inode_data(handle, inode, tmp_inode);
> -		if (retval)
> +		if (retval && !ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS))
>  			/*
>  			 * if we fail to swap inode data free the extent
>  			 * details of the tmp inode
> -- 
> 2.51.0
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR