linux-ext4 - Re: [PATCH 1/2] ext4: always allocate blocks only from groups inode can use

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]

Message-ID: <fmycqmyw25fpfkov5bvl77gqz7mc3kgsfi44al4rn4n5mhzara@ijqxuq6wpna7>
Date: Tue, 13 Jan 2026 16:28:07 +0000
From: Pedro Falcato <pfalcato@...e.de>
To: Jan Kara <jack@...e.cz>
Cc: Ted Tso <tytso@....edu>, linux-ext4@...r.kernel.org, 
	Baokun Li <libaokun1@...wei.com>
Subject: Re: [PATCH 1/2] ext4: always allocate blocks only from groups inode
 can use

On Fri, Jan 09, 2026 at 11:53:37AM +0100, Jan Kara wrote:
> For filesystems with more than 2^32 blocks inodes using indirect block
> based format cannot use blocks beyond the 32-bit limit.
> ext4_mb_scan_groups_linear() takes care to not select these unsupported
> groups for such inodes however other functions selecting groups for
> allocation don't. So far this is harmless because the other selection
> functions are used only with mb_optimize_scan and this is currently
> disabled for inodes with indirect blocks however in the following patch
> we want to enable mb_optimize_scan regardless of inode format.
> 
> Signed-off-by: Jan Kara <jack@...e.cz>
> ---
>  fs/ext4/mballoc.c | 26 +++++++++++++++++---------
>  1 file changed, 17 insertions(+), 9 deletions(-)
> 
> diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
> index 56d50fd3310b..f0e07bf11a93 100644
> --- a/fs/ext4/mballoc.c
> +++ b/fs/ext4/mballoc.c
> @@ -892,6 +892,18 @@ mb_update_avg_fragment_size(struct super_block *sb, struct ext4_group_info *grp)
>  	}
>  }
>  
> +static ext4_group_t ext4_get_allocation_groups_count(
> +				struct ext4_allocation_context *ac)
> +{
> +	ext4_group_t ngroups = ext4_get_groups_count(ac->ac_sb);
> +
> +	/* non-extent files are limited to low blocks/groups */
> +	if (!(ext4_test_inode_flag(ac->ac_inode, EXT4_INODE_EXTENTS)))
> +		ngroups = EXT4_SB(ac->ac_sb)->s_blockfile_groups;
> +
> +	return ngroups;
> +}

I know you're mostly only moving code around, but I think I see a problem here.
Namely, we (probably?) need an smp_rmb() right after the s_blockfile_groups
read to pair with the one in ext4_update_super(). The pre-existing smp_rmb()
in ext4_get_groups_acount() after the s_groups_count load perhaps *incidentally*
works here, but it seems to me like we need a new barrier. So fundamentally
something like:

static ext4_group_t ext4_get_allocation_groups_count(...)
{
	struct ext4_sb_info *sb = EXT4_SB(ac->ac_sb);
	ext4_group_t ngroups;

	ngroups = sb->s_groups_count;
	if (!ext4_test_inode_flag(ac->ac_inode, EXT4_INODE_EXTENTS))
		ngroups = sb->s_blockfile_groups;
	/* pairs with ext4_group_add() logic */
	smp_rmb();
	return ngroups;
}

and to be even more technically correct, we probably want READ_ONCE()
and WRITE_ONCE() here as well.

Does this make sense?

-- 
Pedro