ext4: cap migration extent length at EXT_INIT_MAX_LEN

[PATCH] ext4: cap migration extent length at EXT_INIT_MAX_LEN

Posted by Yichong Chen 2 weeks, 1 day ago

update_extent_range() keeps merging physically and logically contiguous
blocks without limiting the accumulated range.  The maximum length of an
initialized extent is EXT_INIT_MAX_LEN, because the MSB of the 16-bit
ee_len field is used to mark unwritten extents.

finish_range() can therefore store a range longer than EXT_INIT_MAX_LEN
in ee_len.  A range of 32769 blocks is stored as 0x8001, which
ext4_ext_is_unwritten() treats as an unwritten extent and
ext4_ext_get_actual_len() reports as 1 block, so the migrated file
silently loses its data.

Stop merging once the accumulated range reaches EXT_INIT_MAX_LEN, so
migration creates several valid extents instead.

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

diff --git a/fs/ext4/migrate.c b/fs/ext4/migrate.c
index d467b5a13c62..2aa6572088cf 100644
--- a/fs/ext4/migrate.c
+++ b/fs/ext4/migrate.c
@@ -70,7 +70,8 @@ static int update_extent_range(handle_t *handle, struct inode *inode,
 	 */
 	if (lb->first_pblock &&
 		(lb->last_pblock+1 == pblock) &&
-		(lb->last_block+1 == lb->curr_block)) {
+		(lb->last_block+1 == lb->curr_block) &&
+		(lb->last_block - lb->first_block + 1 < EXT_INIT_MAX_LEN)) {
 		lb->last_pblock = pblock;
 		lb->last_block = lb->curr_block;
 		lb->curr_block++;
-- 
2.51.0

Re: [PATCH] ext4: cap migration extent length at EXT_INIT_MAX_LEN

Posted by Jan Kara 2 weeks, 1 day ago

On Thu 10-09-26 11:40:00, Yichong Chen wrote:
> update_extent_range() keeps merging physically and logically contiguous
> blocks without limiting the accumulated range.  The maximum length of an
> initialized extent is EXT_INIT_MAX_LEN, because the MSB of the 16-bit
> ee_len field is used to mark unwritten extents.
> 
> finish_range() can therefore store a range longer than EXT_INIT_MAX_LEN
> in ee_len.  A range of 32769 blocks is stored as 0x8001, which
> ext4_ext_is_unwritten() treats as an unwritten extent and
> ext4_ext_get_actual_len() reports as 1 block, so the migrated file
> silently loses its data.
> 
> Stop merging once the accumulated range reaches EXT_INIT_MAX_LEN, so
> migration creates several valid extents instead.

Yeah, or the extent length could overflow the u16 type. The fix looks good.
Feel free to add:

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

								Honza


> 
> Fixes: c14c6fd5c56a ("ext4: Add EXT4_IOC_MIGRATE ioctl")
> Signed-off-by: Yichong Chen <chenyichong@uniontech.com>
> ---
>  fs/ext4/migrate.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/ext4/migrate.c b/fs/ext4/migrate.c
> index d467b5a13c62..2aa6572088cf 100644
> --- a/fs/ext4/migrate.c
> +++ b/fs/ext4/migrate.c
> @@ -70,7 +70,8 @@ static int update_extent_range(handle_t *handle, struct inode *inode,
>  	 */
>  	if (lb->first_pblock &&
>  		(lb->last_pblock+1 == pblock) &&
> -		(lb->last_block+1 == lb->curr_block)) {
> +		(lb->last_block+1 == lb->curr_block) &&
> +		(lb->last_block - lb->first_block + 1 < EXT_INIT_MAX_LEN)) {
>  		lb->last_pblock = pblock;
>  		lb->last_block = lb->curr_block;
>  		lb->curr_block++;
> -- 
> 2.51.0
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR