[PATCH] erofs: fix managed cache race for unaligned extents

Gao Xiang posted 1 patch 1 month, 2 weeks ago
fs/erofs/zdata.c | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
[PATCH] erofs: fix managed cache race for unaligned extents
Posted by Gao Xiang 1 month, 2 weeks ago
After unaligned compressed extents were introduced, the following race
could occur:

[Thread 1]                                   [Thread 2]
(z_erofs_fill_bio_vec)
<handle a Z_EROFS_PREALLOCATED_FOLIO folio>
...
filemap_add_folio (1)
                                             (z_erofs_bind_cache)
                                             <the same folio is found..>
                                             ..
                                             ..
folio_attach_private (2)
                                             filemap_add_folio (3) again

Since (1) is executed but (2) hasn't been executed yet, it's possible
that another thread finds the same managed folio in z_erofs_bind_cache()
for a different pcluster and calls filemap_add_folio() again since
folio->private is still Z_EROFS_PREALLOCATED_FOLIO.

Fix this by explicitly clearing folio->private before making the folio
visible in the managed cache so that another pcluster can simply wait
on the locked managed folio as what we did for other shared cases [1].

This only impacts unaligned data compression (`-E48bit` with zstd,
for example).

[1] Commit 9e2f9d34dd12 ("erofs: handle overlapped pclusters out of
 crafted images properly") was originally introduced to handle crafted
 overlapped extents, but it addresses unaligned extents as well.

Fixes: 7361d1e3763b ("erofs: support unaligned encoded data")
Reported-by: Arseniy Krasnov <avkrasnov@salutedevices.com>
Closes: https://lore.kernel.org/r/4a2f3801-fac1-42fe-ae75-da315822e088@salutedevices.com
Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com>
---
 fs/erofs/zdata.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/fs/erofs/zdata.c b/fs/erofs/zdata.c
index 8a0b15511931..6b647e75ec04 100644
--- a/fs/erofs/zdata.c
+++ b/fs/erofs/zdata.c
@@ -1509,8 +1509,15 @@ static void z_erofs_fill_bio_vec(struct bio_vec *bvec,
 	DBG_BUGON(z_erofs_is_shortlived_page(bvec->bv_page));
 
 	folio = page_folio(zbv.page);
-	/* For preallocated managed folios, add them to page cache here */
+	/*
+	 * Preallocated folios are added to the managed cache here rather than
+	 * in z_erofs_bind_cache() in order to keep these folios locked in
+	 * increasing (physical) address order.
+	 * Clear folio->private before these folios become visible to others in
+	 * the managed cache to avoid duplicate additions for unaligned extents.
+	 */
 	if (folio->private == Z_EROFS_PREALLOCATED_FOLIO) {
+		folio->private = NULL;
 		tocache = true;
 		goto out_tocache;
 	}
@@ -1546,14 +1553,8 @@ static void z_erofs_fill_bio_vec(struct bio_vec *bvec,
 			}
 			return;
 		}
-		/*
-		 * Already linked with another pcluster, which only appears in
-		 * crafted images by fuzzers for now.  But handle this anyway.
-		 */
-		tocache = false;	/* use temporary short-lived pages */
 	} else {
 		DBG_BUGON(1); /* referenced managed folios can't be truncated */
-		tocache = true;
 	}
 	folio_unlock(folio);
 	folio_put(folio);
-- 
2.43.5
Re: [PATCH] erofs: fix managed cache race for unaligned extents
Posted by Arseniy Krasnov 3 weeks, 6 days ago

On 28/04/2026 07:34, Gao Xiang wrote:
> After unaligned compressed extents were introduced, the following race
> could occur:
> 
> [Thread 1]                                   [Thread 2]
> (z_erofs_fill_bio_vec)
> <handle a Z_EROFS_PREALLOCATED_FOLIO folio>
> ...
> filemap_add_folio (1)
>                                              (z_erofs_bind_cache)
>                                              <the same folio is found..>
>                                              ..
>                                              ..
> folio_attach_private (2)
>                                              filemap_add_folio (3) again
> 
> Since (1) is executed but (2) hasn't been executed yet, it's possible
> that another thread finds the same managed folio in z_erofs_bind_cache()
> for a different pcluster and calls filemap_add_folio() again since
> folio->private is still Z_EROFS_PREALLOCATED_FOLIO.
> 
> Fix this by explicitly clearing folio->private before making the folio
> visible in the managed cache so that another pcluster can simply wait
> on the locked managed folio as what we did for other shared cases [1].
> 
> This only impacts unaligned data compression (`-E48bit` with zstd,
> for example).
> 
> [1] Commit 9e2f9d34dd12 ("erofs: handle overlapped pclusters out of
>  crafted images properly") was originally introduced to handle crafted
>  overlapped extents, but it addresses unaligned extents as well.
> 
> Fixes: 7361d1e3763b ("erofs: support unaligned encoded data")
> Reported-by: Arseniy Krasnov <avkrasnov@salutedevices.com>
> Closes: https://lore.kernel.org/r/4a2f3801-fac1-42fe-ae75-da315822e088@salutedevices.com
> Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com>


Tested-by: Arseniy Krasnov <avkrasnov@salutedevices.com>


> ---
>  fs/erofs/zdata.c | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/erofs/zdata.c b/fs/erofs/zdata.c
> index 8a0b15511931..6b647e75ec04 100644
> --- a/fs/erofs/zdata.c
> +++ b/fs/erofs/zdata.c
> @@ -1509,8 +1509,15 @@ static void z_erofs_fill_bio_vec(struct bio_vec *bvec,
>  	DBG_BUGON(z_erofs_is_shortlived_page(bvec->bv_page));
>  
>  	folio = page_folio(zbv.page);
> -	/* For preallocated managed folios, add them to page cache here */
> +	/*
> +	 * Preallocated folios are added to the managed cache here rather than
> +	 * in z_erofs_bind_cache() in order to keep these folios locked in
> +	 * increasing (physical) address order.
> +	 * Clear folio->private before these folios become visible to others in
> +	 * the managed cache to avoid duplicate additions for unaligned extents.
> +	 */
>  	if (folio->private == Z_EROFS_PREALLOCATED_FOLIO) {
> +		folio->private = NULL;
>  		tocache = true;
>  		goto out_tocache;
>  	}
> @@ -1546,14 +1553,8 @@ static void z_erofs_fill_bio_vec(struct bio_vec *bvec,
>  			}
>  			return;
>  		}
> -		/*
> -		 * Already linked with another pcluster, which only appears in
> -		 * crafted images by fuzzers for now.  But handle this anyway.
> -		 */
> -		tocache = false;	/* use temporary short-lived pages */
>  	} else {
>  		DBG_BUGON(1); /* referenced managed folios can't be truncated */
> -		tocache = true;
>  	}
>  	folio_unlock(folio);
>  	folio_put(folio);