[PATCH] erofs: fix folio reuse from a different address_space in erofs_bread()

[PATCH] erofs: fix folio reuse from a different address_space in erofs_bread()

[Binglei Wang]

From: Binglei Wang <l3b2w1@gmail.com>

erofs_bread() reuses a cached folio on page index match without checking folio->mapping.

xattr.c calls erofs_init_metabuf() twice on the same buffer without erofs_put_metabuf();
the two in_metabox sources differ, so buf->mapping can switch
while buf->page still belongs to the old address_space.
The next erofs_bread() then reads the wrong mapping, dropping shared xattrs with METABOX.

We require folio->mapping == buf->mapping in the reuse check;
otherwise drop the cached folio and re-read via the slow path.

Fixes: 414091322c63 ("erofs: implement metadata compression")
Cc: Bo Liu (OpenAnolis) <liubo03@inspur.com>
Signed-off-by: Binglei Wang <l3b2w1@gmail.com>
---
 fs/erofs/data.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/fs/erofs/data.c b/fs/erofs/data.c
index be63b89f0862..d8b6523bc218 100644
--- a/fs/erofs/data.c
+++ b/fs/erofs/data.c
@@ -33,8 +33,13 @@ void *erofs_bread(struct erofs_buf *buf, erofs_off_t offset, bool need_kmap)
 	if (buf->page) {
 		folio = page_folio(buf->page);
-		if (folio_file_page(folio, index) != buf->page)
+		if (folio->mapping != buf->mapping) {
+			/* the cached folio belongs to another address_space */
+			erofs_put_metabuf(buf);
+			folio = NULL;
+		} else if (folio_file_page(folio, index) != buf->page) {
 			erofs_unmap_metabuf(buf);
+		}
 	}
 	if (!folio || !folio_contains(folio, index)) {
 		erofs_put_metabuf(buf);
--
2.43.0

Re: [PATCH] erofs: fix folio reuse from a different address_space in erofs_bread()

[Gao Xiang]

On Wed, Sep 30, 2026 at 07:21:27PM +0800, Binglei Wang wrote:
> From: Binglei Wang <l3b2w1@gmail.com>
> 
> erofs_bread() reuses a cached folio on page index match without checking folio->mapping.

Each line in the commit message should not exceed 72 chars.

> 
> xattr.c calls erofs_init_metabuf() twice on the same buffer without erofs_put_metabuf();
> the two in_metabox sources differ, so buf->mapping can switch
> while buf->page still belongs to the old address_space.
> The next erofs_bread() then reads the wrong mapping, dropping shared xattrs with METABOX.
> 
> We require folio->mapping == buf->mapping in the reuse check;
> otherwise drop the cached folio and re-read via the slow path.
> 
> Fixes: 414091322c63 ("erofs: implement metadata compression")
> Cc: Bo Liu (OpenAnolis) <liubo03@inspur.com>
> Signed-off-by: Binglei Wang <l3b2w1@gmail.com>
> ---
>  fs/erofs/data.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/erofs/data.c b/fs/erofs/data.c
> index be63b89f0862..d8b6523bc218 100644
> --- a/fs/erofs/data.c
> +++ b/fs/erofs/data.c
> @@ -33,8 +33,13 @@ void *erofs_bread(struct erofs_buf *buf, erofs_off_t offset, bool need_kmap)
>  	if (buf->page) {
>  		folio = page_folio(buf->page);
> -		if (folio_file_page(folio, index) != buf->page)
> +		if (folio->mapping != buf->mapping) {
> +			/* the cached folio belongs to another address_space */
> +			erofs_put_metabuf(buf);
> +			folio = NULL;

As I said, you could just drop `erofs_put_metabuf(buf);` here,
and folio = NULL will do the correct thing instead.

Thanks,
Gao Xiang

> +		} else if (folio_file_page(folio, index) != buf->page) {
>  			erofs_unmap_metabuf(buf);
> +		}
>  	}
>  	if (!folio || !folio_contains(folio, index)) {
>  		erofs_put_metabuf(buf);
> --
> 2.43.0
> 
> 

[PATCH v2] erofs: fix folio reuse from a different address_space in erofs_bread()

[Binglei Wang]

erofs_bread() reuses a cached folio on page index match without
checking folio->mapping.

xattr.c calls erofs_init_metabuf() twice on the same buffer without
erofs_put_metabuf(); the two in_metabox sources differ, so buf->mapping
can switch while buf->page still belongs to the old address_space.
The next erofs_bread() then reads the wrong mapping, dropping shared
xattrs with METABOX.

We require folio->mapping == buf->mapping in the reuse check; otherwise
drop the cached folio and re-read via the slow path.

Fixes: 414091322c63 ("erofs: implement metadata compression")
Cc: Bo Liu (OpenAnolis) <liubo03@inspur.com>
Signed-off-by: Binglei Wang <l3b2w1@gmail.com>
---
Changes since v1:
 - Drop the redundant erofs_put_metabuf() call; setting folio to NULL is
   enough since the slow path already releases the cached folio (Gao Xiang).
 - Rewrap the commit message to 72 columns.

 fs/erofs/data.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/fs/erofs/data.c b/fs/erofs/data.c
index be63b89f0862..4b9fe670fdda 100644
--- a/fs/erofs/data.c
+++ b/fs/erofs/data.c
@@ -33,8 +33,12 @@ void *erofs_bread(struct erofs_buf *buf, erofs_off_t offset, bool need_kmap)
 	if (buf->page) {
 		folio = page_folio(buf->page);
-		if (folio_file_page(folio, index) != buf->page)
+		if (folio->mapping != buf->mapping) {
+			/* the cached folio belongs to another address_space */
+			folio = NULL;
+		} else if (folio_file_page(folio, index) != buf->page) {
 			erofs_unmap_metabuf(buf);
+		}
 	}
 	if (!folio || !folio_contains(folio, index)) {
 		erofs_put_metabuf(buf);

Re: [PATCH v2] erofs: fix folio reuse from a different address_space in erofs_bread()

[Gao Xiang]

On Thu, Oct 01, 2026 at 03:19:01AM +0800, Binglei Wang wrote:
> erofs_bread() reuses a cached folio on page index match without
> checking folio->mapping.
> 
> xattr.c calls erofs_init_metabuf() twice on the same buffer without
> erofs_put_metabuf(); the two in_metabox sources differ, so buf->mapping
> can switch while buf->page still belongs to the old address_space.
> The next erofs_bread() then reads the wrong mapping, dropping shared
> xattrs with METABOX.
> 
> We require folio->mapping == buf->mapping in the reuse check; otherwise
> drop the cached folio and re-read via the slow path.
> 
> Fixes: 414091322c63 ("erofs: implement metadata compression")
> Cc: Bo Liu (OpenAnolis) <liubo03@inspur.com>
> Signed-off-by: Binglei Wang <l3b2w1@gmail.com>

I have to tell you that the patch is still broken, and I failed to
apply like this:


Applying: erofs: fix folio reuse from a different address_space in erofs_bread()
Using index info to reconstruct a base tree...
error: patch failed: fs/erofs/data.c:33
error: fs/erofs/data.c: patch does not apply
error: Did you hand edit your patch?
It does not apply to blobs recorded in its index.
Patch failed at 0001 erofs: fix folio reuse from a different address_space in erofs_bread()


Could you just apply locally before sending out a version?
I could make a patch for you manually, but if you apply more patches
later, the broken email client should be fixed.

Thanks,
Gao Xiang

Re: [PATCH] erofs: fix folio reuse from a different address_space in erofs_bread()

[binglei wang]

Sorry, my webbrowser email client configs with something wrong. Thanks
for your reply.

I re-sent the patch from my Linux shell. I believe it's OK this time.


Gao Xiang <xiang@kernel.org> 于2026年9月30日周三 15:52写道：
>
> On Wed, Sep 30, 2026 at 11:37:53AM +0800, binglei wang wrote:
> > erofs_bread() caches the last folio in struct erofs_buf and reuses it when
> > the next request lands on the same folio, but the reuse predicate only
> > compares the page index; it never checks that the cached folio still
> > belongs to buf->mapping.  Correctness therefore relies on an invariant
> > that is nowhere enforced.
> >
> > fs/erofs/xattr.c already breaks it: erofs_xattr_iter_inline() and
> > erofs_xattr_iter_shared() call erofs_init_metabuf() on the same buffer
> > with no intervening erofs_put_metabuf(), and their in_metabox arguments
> > come from different sources (per-inode vs per-fs).  When the two differ,
> > buf->mapping is switched while buf->page still holds a folio of the
> > previous address_space, so the next erofs_bread() can return data from
> > the wrong one.
> >
> > This is observable with METABOX enabled, where shared xattrs silently
> > disappear on the mounted fs, while the same tree built without METABOX
> > reports them fine.
> >
> > Fix it by validating the address_space in the reuse predicate too: if the
> > cached folio belongs to another mapping, drop it so that the existing
> > slow path re-reads from buf->mapping.  Reading folio->mapping is safe
> > here because a reference on the cached folio is still held; if the folio
> > was already truncated, folio->mapping is NULL and the slow path is taken,
> > which is the safe direction.
> >
> > Fixes: 414091322c63 ("erofs: implement metadata compression")
> > Cc: Bo Liu (OpenAnolis) <liubo03@inspur.com>
> > Signed-off-by: Binglei Wang <l3b2w1@gmail.com>
>
> Thanks for the patch.
>
>  1) The patch format is still broken as your previous patch, please
>     check your email client again before sending out a new patch
>     (as I said, you could send a patch to yourself and try to apply
>      the patch and see if it works);
>
>  2) the commit message of this patch is too over long (mostly
> in the LLM-generated style), I think you could simplify a bit since
> it's more friendly to human developers.
>
> > ---
> >  fs/erofs/data.c | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/fs/erofs/data.c b/fs/erofs/data.c
> > index be63b89f0862..d8b6523bc218 100644
> > --- a/fs/erofs/data.c
> > +++ b/fs/erofs/data.c
> > @@ -33,8 +33,13 @@ void *erofs_bread(struct erofs_buf *buf,
> > erofs_off_t offset, bool need_kmap)
> >
> >   if (buf->page) {
> >   folio = page_folio(buf->page);
> > - if (folio_file_page(folio, index) != buf->page)
> > + if (folio->mapping != buf->mapping) {
> > + /* the cached folio belongs to another address_space */
> > + erofs_put_metabuf(buf);
> > + folio = NULL;
>
>      `folio = NULL` is enough?
>
> Thanks,
> Gao Xiang
>
> > + } else if (folio_file_page(folio, index) != buf->page) {
> >   erofs_unmap_metabuf(buf);
> > + }
> >   }
> >   if (!folio || !folio_contains(folio, index)) {
> >   erofs_put_metabuf(buf);

Re: [PATCH] erofs: fix folio reuse from a different address_space in erofs_bread()

[Gao Xiang]

On Wed, Sep 30, 2026 at 11:37:53AM +0800, binglei wang wrote:
> erofs_bread() caches the last folio in struct erofs_buf and reuses it when
> the next request lands on the same folio, but the reuse predicate only
> compares the page index; it never checks that the cached folio still
> belongs to buf->mapping.  Correctness therefore relies on an invariant
> that is nowhere enforced.
> 
> fs/erofs/xattr.c already breaks it: erofs_xattr_iter_inline() and
> erofs_xattr_iter_shared() call erofs_init_metabuf() on the same buffer
> with no intervening erofs_put_metabuf(), and their in_metabox arguments
> come from different sources (per-inode vs per-fs).  When the two differ,
> buf->mapping is switched while buf->page still holds a folio of the
> previous address_space, so the next erofs_bread() can return data from
> the wrong one.
> 
> This is observable with METABOX enabled, where shared xattrs silently
> disappear on the mounted fs, while the same tree built without METABOX
> reports them fine.
> 
> Fix it by validating the address_space in the reuse predicate too: if the
> cached folio belongs to another mapping, drop it so that the existing
> slow path re-reads from buf->mapping.  Reading folio->mapping is safe
> here because a reference on the cached folio is still held; if the folio
> was already truncated, folio->mapping is NULL and the slow path is taken,
> which is the safe direction.
> 
> Fixes: 414091322c63 ("erofs: implement metadata compression")
> Cc: Bo Liu (OpenAnolis) <liubo03@inspur.com>
> Signed-off-by: Binglei Wang <l3b2w1@gmail.com>

Thanks for the patch.

 1) The patch format is still broken as your previous patch, please
    check your email client again before sending out a new patch
    (as I said, you could send a patch to yourself and try to apply
     the patch and see if it works);

 2) the commit message of this patch is too over long (mostly
in the LLM-generated style), I think you could simplify a bit since
it's more friendly to human developers.

> ---
>  fs/erofs/data.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/erofs/data.c b/fs/erofs/data.c
> index be63b89f0862..d8b6523bc218 100644
> --- a/fs/erofs/data.c
> +++ b/fs/erofs/data.c
> @@ -33,8 +33,13 @@ void *erofs_bread(struct erofs_buf *buf,
> erofs_off_t offset, bool need_kmap)
> 
>   if (buf->page) {
>   folio = page_folio(buf->page);
> - if (folio_file_page(folio, index) != buf->page)
> + if (folio->mapping != buf->mapping) {
> + /* the cached folio belongs to another address_space */
> + erofs_put_metabuf(buf);
> + folio = NULL;

     `folio = NULL` is enough?

Thanks,
Gao Xiang

> + } else if (folio_file_page(folio, index) != buf->page) {
>   erofs_unmap_metabuf(buf);
> + }
>   }
>   if (!folio || !folio_contains(folio, index)) {
>   erofs_put_metabuf(buf);

[PATCH] erofs: fix folio reuse from a different address_space in erofs_bread()

[binglei wang]

[-- Attachment #1: Type: text/plain, Size: 2189 bytes --]

erofs_bread() caches the last folio in struct erofs_buf and reuses it when
the next request lands on the same folio, but the reuse predicate only
compares the page index; it never checks that the cached folio still
belongs to buf->mapping.  Correctness therefore relies on an invariant
that is nowhere enforced.

fs/erofs/xattr.c already breaks it: erofs_xattr_iter_inline() and
erofs_xattr_iter_shared() call erofs_init_metabuf() on the same buffer
with no intervening erofs_put_metabuf(), and their in_metabox arguments
come from different sources (per-inode vs per-fs).  When the two differ,
buf->mapping is switched while buf->page still holds a folio of the
previous address_space, so the next erofs_bread() can return data from
the wrong one.

This is observable with METABOX enabled, where shared xattrs silently
disappear on the mounted fs, while the same tree built without METABOX
reports them fine.

Fix it by validating the address_space in the reuse predicate too: if the
cached folio belongs to another mapping, drop it so that the existing
slow path re-reads from buf->mapping.  Reading folio->mapping is safe
here because a reference on the cached folio is still held; if the folio
was already truncated, folio->mapping is NULL and the slow path is taken,
which is the safe direction.

Fixes: 414091322c63 ("erofs: implement metadata compression")
Cc: Bo Liu (OpenAnolis) <liubo03@inspur.com>
Signed-off-by: Binglei Wang <l3b2w1@gmail.com>
---
 fs/erofs/data.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/fs/erofs/data.c b/fs/erofs/data.c
index be63b89f0862..d8b6523bc218 100644
--- a/fs/erofs/data.c
+++ b/fs/erofs/data.c
@@ -33,8 +33,13 @@ void *erofs_bread(struct erofs_buf *buf,
erofs_off_t offset, bool need_kmap)

  if (buf->page) {
  folio = page_folio(buf->page);
- if (folio_file_page(folio, index) != buf->page)
+ if (folio->mapping != buf->mapping) {
+ /* the cached folio belongs to another address_space */
+ erofs_put_metabuf(buf);
+ folio = NULL;
+ } else if (folio_file_page(folio, index) != buf->page) {
  erofs_unmap_metabuf(buf);
+ }
  }
  if (!folio || !folio_contains(folio, index)) {
  erofs_put_metabuf(buf);
-- 
2.33.0

[-- Attachment #2: 0001-erofs-fix-folio-reuse-from-a-different-address_space.patch --]
[-- Type: text/x-diff, Size: 2442 bytes --]

From a17eaf6b476a66e6bc6d85c569584911accb05bf Mon Sep 17 00:00:00 2001
From: Binglei Wang <l3b2w1@gmail.com>
Date: Wed, 30 Sep 2026 10:34:48 +0800
Subject: [PATCH] erofs: fix folio reuse from a different address_space in
 erofs_bread()

erofs_bread() caches the last folio in struct erofs_buf and reuses it when
the next request lands on the same folio, but the reuse predicate only
compares the page index; it never checks that the cached folio still
belongs to buf->mapping.  Correctness therefore relies on an invariant
that is nowhere enforced.

fs/erofs/xattr.c already breaks it: erofs_xattr_iter_inline() and
erofs_xattr_iter_shared() call erofs_init_metabuf() on the same buffer
with no intervening erofs_put_metabuf(), and their in_metabox arguments
come from different sources (per-inode vs per-fs).  When the two differ,
buf->mapping is switched while buf->page still holds a folio of the
previous address_space, so the next erofs_bread() can return data from
the wrong one.

This is observable with METABOX enabled, where shared xattrs silently
disappear on the mounted fs, while the same tree built without METABOX
reports them fine.

Fix it by validating the address_space in the reuse predicate too: if the
cached folio belongs to another mapping, drop it so that the existing
slow path re-reads from buf->mapping.  Reading folio->mapping is safe
here because a reference on the cached folio is still held; if the folio
was already truncated, folio->mapping is NULL and the slow path is taken,
which is the safe direction.

Fixes: 414091322c63 ("erofs: implement metadata compression")
Cc: Bo Liu (OpenAnolis) <liubo03@inspur.com>
Signed-off-by: Binglei Wang <l3b2w1@gmail.com>
---
 fs/erofs/data.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/fs/erofs/data.c b/fs/erofs/data.c
index be63b89f0862..d8b6523bc218 100644
--- a/fs/erofs/data.c
+++ b/fs/erofs/data.c
@@ -33,8 +33,13 @@ void *erofs_bread(struct erofs_buf *buf, erofs_off_t offset, bool need_kmap)
 
 	if (buf->page) {
 		folio = page_folio(buf->page);
-		if (folio_file_page(folio, index) != buf->page)
+		if (folio->mapping != buf->mapping) {
+			/* the cached folio belongs to another address_space */
+			erofs_put_metabuf(buf);
+			folio = NULL;
+		} else if (folio_file_page(folio, index) != buf->page) {
 			erofs_unmap_metabuf(buf);
+		}
 	}
 	if (!folio || !folio_contains(folio, index)) {
 		erofs_put_metabuf(buf);
-- 
2.33.0