erofs: use the opener's credential when verifing metadata accesses

[PATCH] erofs: use the opener's credential when verifing metadata accesses

Posted by Gao Xiang 1 month, 1 week ago

Similar to commit 905eeb2b7c33 ("erofs: impersonate the opener's
credentials when accessing backing file"), rw_verify_area() needs
the same too.

Fixes: 307210c262a2 ("erofs: verify metadata accesses for file-backed mounts")
Cc: Carlos Llamas <cmllamas@google.com>
Cc: Sandeep Dhavale <dhavale@google.com>
Cc: Tatsuyuki Ishi <ishitatsuyuki@google.com>
Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com>
---
Can we verify this patch resolve the android-mainline issue?

 fs/erofs/data.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/fs/erofs/data.c b/fs/erofs/data.c
index b2c12c5856ac..51b8e860b6b2 100644
--- a/fs/erofs/data.c
+++ b/fs/erofs/data.c
@@ -40,9 +40,11 @@ void *erofs_bread(struct erofs_buf *buf, erofs_off_t offset, bool need_kmap)
 	 */
 	if (buf->file) {
 		fpos = (loff_t)index << PAGE_SHIFT;
-		err = rw_verify_area(READ, buf->file, &fpos, PAGE_SIZE);
-		if (err < 0)
-			return ERR_PTR(err);
+		scoped_with_creds(buf->file->f_cred) {
+			err = rw_verify_area(READ, buf->file, &fpos, PAGE_SIZE);
+			if (err < 0)
+				return ERR_PTR(err);
+		}
 	}
 
 	if (buf->page) {
-- 
2.43.5

Re: [PATCH] erofs: use the opener's credential when verifing metadata accesses

Posted by Christoph Hellwig 1 month ago

On Tue, May 05, 2026 at 11:56:15PM +0800, Gao Xiang wrote:
> Similar to commit 905eeb2b7c33 ("erofs: impersonate the opener's
> credentials when accessing backing file"), rw_verify_area() needs
> the same too.

Two things here:

 - rw_verify_area is a helper for use inside the VFS and file system
   read/write method implementation.  Erofs as a user of the VFS should
   not use it at all.
 - using the opener credentials when accessing the backing file seems
   wrong.  The entity accessing it is the file system, so it should
   have system or mounter credentials, not that of someone causing
   metadata / fs data access.  And this applies to all access by
   a file system backed by a backing file.

Re: [PATCH] erofs: use the opener's credential when verifing metadata accesses

Posted by Tatsuyuki Ishi 1 month ago

>  - using the opener credentials when accessing the backing file seems
>    wrong.  The entity accessing it is the file system, so it should
>    have system or mounter credentials, not that of someone causing
>    metadata / fs data access.  And this applies to all access by
>    a file system backed by a backing file.

I think there's probably some confusion of terminology here. buf->file
is opened with the mounter's credentials, so we are impersonating the
mounter here. Perhaps the commit message could describe that more
clearly. Same for the previous patches mentioned.

[resend: previous mail was rejected due to HTML]

Re: [PATCH] erofs: use the opener's credential when verifing metadata accesses

Posted by Carlos Llamas 1 month, 1 week ago

On Tue, May 05, 2026 at 11:56:15PM +0800, Gao Xiang wrote:
> Similar to commit 905eeb2b7c33 ("erofs: impersonate the opener's
> credentials when accessing backing file"), rw_verify_area() needs
> the same too.
> 
> Fixes: 307210c262a2 ("erofs: verify metadata accesses for file-backed mounts")
> Cc: Carlos Llamas <cmllamas@google.com>
> Cc: Sandeep Dhavale <dhavale@google.com>
> Cc: Tatsuyuki Ishi <ishitatsuyuki@google.com>
> Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com>
> ---
> Can we verify this patch resolve the android-mainline issue?

Yes, it does. Thanks!

Tested-by: Carlos Llamas <cmllamas@google.com>

[PATCH v2] erofs: use the opener's credential when verifying metadata accesses

Posted by Gao Xiang 1 month, 1 week ago

Similar to commit 905eeb2b7c33 ("erofs: impersonate the opener's
credentials when accessing backing file"), rw_verify_area() needs
the same too.

Fixes: 307210c262a2 ("erofs: verify metadata accesses for file-backed mounts")
Cc: Carlos Llamas <cmllamas@google.com>
Cc: Sandeep Dhavale <dhavale@google.com>
Cc: Tatsuyuki Ishi <ishitatsuyuki@google.com>
Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com>
---
v2:
 - apply sashiko's suggestion
   https://sashiko.dev/#/patchset/20260505155615.2719500-1-hsiangkao%40linux.alibaba.com

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

diff --git a/fs/erofs/data.c b/fs/erofs/data.c
index 44da21c9d777..d6f6035fd714 100644
--- a/fs/erofs/data.c
+++ b/fs/erofs/data.c
@@ -29,6 +29,7 @@ void erofs_put_metabuf(struct erofs_buf *buf)
 void *erofs_bread(struct erofs_buf *buf, erofs_off_t offset, bool need_kmap)
 {
 	pgoff_t index = (buf->off + offset) >> PAGE_SHIFT;
+	const struct cred *old_cred = NULL;
 	struct folio *folio = NULL;
 	loff_t fpos;
 	int err;
@@ -40,9 +41,12 @@ void *erofs_bread(struct erofs_buf *buf, erofs_off_t offset, bool need_kmap)
 	 */
 	if (buf->file) {
 		fpos = (loff_t)index << PAGE_SHIFT;
+		old_cred = override_creds(buf->file->f_cred);
 		err = rw_verify_area(READ, buf->file, &fpos, PAGE_SIZE);
-		if (err < 0)
+		if (err < 0) {
+			revert_creds(old_cred);
 			return ERR_PTR(err);
+		}
 	}
 
 	if (buf->page) {
@@ -53,6 +57,8 @@ void *erofs_bread(struct erofs_buf *buf, erofs_off_t offset, bool need_kmap)
 	if (!folio || !folio_contains(folio, index)) {
 		erofs_put_metabuf(buf);
 		folio = read_mapping_folio(buf->mapping, index, buf->file);
+		if (old_cred)
+			revert_creds(old_cred);
 		if (IS_ERR(folio))
 			return folio;
 	}
-- 
2.43.5