linux-ext4 - Re: [RFC PATCH] ext4: fix e4b bitmap inconsistency reports

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: <e9a0aa3e-c4a6-40b3-a35a-aa017f94126e@huawei.com>
Date: Fri, 9 Jan 2026 10:38:55 +0800
From: Baokun Li <libaokun1@...wei.com>
To: <sunyongjian1@...wei.com>
CC: <linux-ext4@...r.kernel.org>, <linux-fsdevel@...r.kernel.org>,
	<tytso@....edu>, <jack@...e.cz>, <yangerkun@...wei.com>,
	<yi.zhang@...wei.com>, <chengzhihao1@...wei.com>
Subject: Re: [RFC PATCH] ext4: fix e4b bitmap inconsistency reports

On 2026-01-06 17:08, Yongjian Sun wrote:
> From: Yongjian Sun <sunyongjian1@...wei.com>
>
> A bitmap inconsistency issue was observed during stress tests under
> mixed huge-page workloads. Ext4 reported multiple e4b bitmap check
> failures like:
>
> ext4_mb_complex_scan_group:2508: group 350, 8179 free clusters as
> per group info. But got 8192 blocks
>
> Analysis and experimentation confirmed that the issue is caused by a
> race condition between page migration and bitmap modification. Although
> this timing window is extremely narrow, it is still hit in practice:
>
> folio_lock                        ext4_mb_load_buddy
> __migrate_folio
>   check ref count
>   folio_mc_copy                     __filemap_get_folio
>                                       folio_try_get(folio)
>                                   ......
>                                   mb_mark_used
>                                   ext4_mb_unload_buddy
>   __folio_migrate_mapping
>     folio_ref_freeze
> folio_unlock
>
> The root cause of this issue is that the fast path of load_buddy only
> increments the folio's reference count, which is insufficient to prevent
> concurrent folio migration. We observed that the folio migration process
> acquires the folio lock. Therefore, we can determine whether to take the
> fast path in load_buddy by checking the lock status. If the folio is
> locked, we opt for the slow path (which acquires the lock) to close this
> concurrency window.
>
> Additionally, this change addresses the following issues:
>
> When the DOUBLE_CHECK macro is enabled to inspect bitmap-related
> issues, the following error may be triggered:
>
> corruption in group 324 at byte 784(6272): f in copy != ff on
> disk/prealloc
>
> Analysis reveals that this is a false positive. There is a specific race
> window where the bitmap and the group descriptor become momentarily
> inconsistent, leading to this error report:
>
> ext4_mb_load_buddy                   ext4_mb_load_buddy
>   __filemap_get_folio(create|lock)
>     folio_lock
>   ext4_mb_init_cache
>     folio_mark_uptodate
>                                      __filemap_get_folio(no lock)
>                                      ......
>                                      mb_mark_used
>                                        mb_mark_used_double
>   mb_cmp_bitmaps
>                                        mb_set_bits(e4b->bd_bitmap)
>   folio_unlock
>
> The original logic assumed that since mb_cmp_bitmaps is called when the
> bitmap is newly loaded from disk, the folio lock would be sufficient to
> prevent concurrent access. However, this overlooks a specific race
> condition: if another process attempts to load buddy and finds the folio
> is already in an uptodate state, it will immediately begin using it without
> holding folio lock.
>
> Signed-off-by: Yongjian Sun <sunyongjian1@...wei.com>

Looks good. Feel free to add:

Reviewed-by: Baokun Li <libaokun1@...wei.com>

> ---
>  fs/ext4/mballoc.c | 21 +++++++++++----------
>  1 file changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
> index 56d50fd3310b..de4cacb740b3 100644
> --- a/fs/ext4/mballoc.c
> +++ b/fs/ext4/mballoc.c
> @@ -1706,16 +1706,17 @@ ext4_mb_load_buddy_gfp(struct super_block *sb, ext4_group_t group,
>  
>  	/* Avoid locking the folio in the fast path ... */
>  	folio = __filemap_get_folio(inode->i_mapping, pnum, FGP_ACCESSED, 0);
> -	if (IS_ERR(folio) || !folio_test_uptodate(folio)) {
> +	if (IS_ERR(folio) || !folio_test_uptodate(folio) || folio_test_locked(folio)) {
> +		/*
> +		 * folio_test_locked is employed to detect ongoing folio
> +		 * migrations, since concurrent migrations can lead to
> +		 * bitmap inconsistency. And if we are not uptodate that
> +		 * implies somebody just created the folio but is yet to
> +		 * initialize it. We can drop the folio reference and
> +		 * try to get the folio with lock in both cases to avoid
> +		 * concurrency.
> +		 */
>  		if (!IS_ERR(folio))
> -			/*
> -			 * drop the folio reference and try
> -			 * to get the folio with lock. If we
> -			 * are not uptodate that implies
> -			 * somebody just created the folio but
> -			 * is yet to initialize it. So
> -			 * wait for it to initialize.
> -			 */
>  			folio_put(folio);
>  		folio = __filemap_get_folio(inode->i_mapping, pnum,
>  				FGP_LOCK | FGP_ACCESSED | FGP_CREAT, gfp);
> @@ -1764,7 +1765,7 @@ ext4_mb_load_buddy_gfp(struct super_block *sb, ext4_group_t group,
>  
>  	/* we need another folio for the buddy */
>  	folio = __filemap_get_folio(inode->i_mapping, pnum, FGP_ACCESSED, 0);
> -	if (IS_ERR(folio) || !folio_test_uptodate(folio)) {
> +	if (IS_ERR(folio) || !folio_test_uptodate(folio) || folio_test_locked(folio)) {
>  		if (!IS_ERR(folio))
>  			folio_put(folio);
>  		folio = __filemap_get_folio(inode->i_mapping, pnum,