linux-ext4 - Re: [PATCH 4/4] ext4: avoid prealloc space being skipped due to overflow

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 21 Jul 2023 00:52:50 +0530
From:   Ritesh Harjani (IBM) <ritesh.list@...il.com>
To:     Baokun Li <libaokun1@...wei.com>, linux-ext4@...r.kernel.org
Cc:     tytso@....edu, adilger.kernel@...ger.ca, jack@...e.cz,
        ojaswin@...ux.ibm.com, linux-kernel@...r.kernel.org,
        yi.zhang@...wei.com, yangerkun@...wei.com, yukuai3@...wei.com,
        libaokun1@...wei.com
Subject: Re: [PATCH 4/4] ext4: avoid prealloc space being skipped due to overflow

Baokun Li <libaokun1@...wei.com> writes:

> If there is a pa in the i_prealloc_list of an inode with a tmp_pa_end
> of 4294967296(0x100000000), since tmp_pa_end is of type ext4_lblk_t,
> tmp_pa_end will be recognized as 0 due to overflow, which causes
> (ac->ac_o_ ex.fe_logical >= tmp_pa_end) always holds, so that pa will
> always be skipped. This then triggers the regular allocation process,
> and if the excess tail of the free extent from that allocation is put
> into the i_prealloc_list again, it will again not be used. This ends up
> leaving us with a lot of free physical blocks in the i_prealloc_list.
>
> We avoid this problem by using pa_end() to compute tmp_pa_end and
> declaring tmp_pa_end to be of type loff_t.
>
> Signed-off-by: Baokun Li <libaokun1@...wei.com>

Looks good to me with fex_end() and pa_end() dropped.

-ritesh


> ---
>  fs/ext4/mballoc.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
> index 77d47af525d9..06db40fb29d6 100644
> --- a/fs/ext4/mballoc.c
> +++ b/fs/ext4/mballoc.c
> @@ -4765,7 +4765,8 @@ ext4_mb_use_preallocated(struct ext4_allocation_context *ac)
>  	struct ext4_inode_info *ei = EXT4_I(ac->ac_inode);
>  	struct ext4_locality_group *lg;
>  	struct ext4_prealloc_space *tmp_pa, *cpa = NULL;
> -	ext4_lblk_t tmp_pa_start, tmp_pa_end;
> +	ext4_lblk_t tmp_pa_start;
> +	loff_t tmp_pa_end;
>  	struct rb_node *iter;
>  	ext4_fsblk_t goal_block;
>  
> @@ -4784,7 +4785,7 @@ ext4_mb_use_preallocated(struct ext4_allocation_context *ac)
>  		/* all fields in this condition don't change,
>  		 * so we can skip locking for them */
>  		tmp_pa_start = tmp_pa->pa_lstart;
> -		tmp_pa_end = tmp_pa->pa_lstart + EXT4_C2B(sbi, tmp_pa->pa_len);
> +		tmp_pa_end = pa_end(sbi, tmp_pa);
>  
>  		/* original request start doesn't lie in this PA */
>  		if (ac->ac_o_ex.fe_logical < tmp_pa_start ||
> -- 
> 2.31.1