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

Yichong Chen posted 1 patch 6 days, 13 hours ago
fs/ext4/migrate.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] ext4: don't free the tmp_inode tree after a partial migration swap
Posted by Yichong Chen 6 days, 13 hours ago
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
Posted by Jan Kara 7 hours ago
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