Re: [PATCH 07/11] ext4: Convert group-count barrier protocol to acquire/release

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] ext4_get_group_info() reads s_groups_count without an acquire barrier, leading to potential out-of-bounds array access during online resize.
--

--- Patch [7]: [PATCH 07/11] ext4: Convert group-count barrier protocol to acquire/release ---
commit f34a19e39cbb154b52efeb9c4d1fc4d48a993810
Author: Jinjie Ruan <ruanjinjie@huawei.com>

ext4: Convert group-count barrier protocol to acquire/release

During an online resize, reading s_groups_count and s_blockfile_groups
requires strict ordering against initialized block group metadata. This
commit replaces the smp_wmb()/smp_rmb() pairs with smp_store_release()
and smp_load_acquire().

> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index 2fdff50526e93..e7f15c9e9187a 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -3527,16 +3527,12 @@ static inline void ext4_isize_set(struct ext4_inode *raw_inode, loff_t i_size)
>  }
>  
>  /*
> - * Reading s_groups_count requires using smp_rmb() afterwards.  See
> - * the locking protocol documented in the comments of ext4_group_add()
> - * in resize.c
> + * Reading s_groups_count uses acquire semantics.
>   */
>  static inline ext4_group_t ext4_get_groups_count(struct super_block *sb)
>  {
> -	ext4_group_t	ngroups = EXT4_SB(sb)->s_groups_count;
> -
> -	smp_rmb();
> -	return ngroups;
> +	/* Pairs with smp_store_release() in ext4_update_super() */
> +	return smp_load_acquire(&EXT4_SB(sb)->s_groups_count);
>  }

[Severity: High]
This is a pre-existing issue, but since we are formalizing the barrier
protocol around s_groups_count, should we also fix the plain read of
s_groups_count in ext4_get_group_info()?

If a concurrent reader calls ext4_get_group_info() during an online resize:

fs/ext4/balloc.c:ext4_get_group_info() {
	...
	if (unlikely(group >= EXT4_SB(sb)->s_groups_count))
		return NULL;
	indexv = group >> (EXT4_DESC_PER_BLOCK_BITS(sb));
	indexh = group & ((EXT4_DESC_PER_BLOCK(sb)) - 1);
	grp_info = sbi_array_rcu_deref(EXT4_SB(sb), s_group_info, indexv);
	...
}

Because there is no acquire barrier (it does not use ext4_get_groups_count()),
can the CPU speculatively execute the rcu_dereference of s_group_info before
the load of s_groups_count?

If the resize writer just published a newly allocated s_group_info array
and incremented s_groups_count via smp_store_release() in ext4_update_super(),
could this reader see the new s_groups_count but load the old s_group_info
array pointer?

If so, would the boundary check pass, causing an out-of-bounds read into the
old, smaller array using the new group index?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260825095422.3166067-1-ruanjinjie@huawei.com?part=7