CEPH filesystem development
 help / color / mirror / Atom feed
* [PATCH] ceph: acquire caps for read_folio requests without an rw context
@ 2026-09-02 12:40 Max Kellermann
  2026-09-04  4:54 ` Xiubo Li
  0 siblings, 1 reply; 2+ messages in thread
From: Max Kellermann @ 2026-09-02 12:40 UTC (permalink / raw)
  To: idryomov, amarkuze, xiubo.li, ceph-devel, linux-kernel
  Cc: Max Kellermann, stable

This fixes a data corruption problem that leaked permanently into
fscache.  File-backed erofs images are read using
read_mapping_folio(), and the Ceph implementation of this call forgets
to acquire Ceph caps.  Therefore, mounting an erofs image file from a
Ceph mount that was just written (but not yet committed to the OSD)
would fail because the erofs code saw only zero-filled pages.  These
zero-filled backes were then copied to the fscache, making this data
corruption permanent (on this host).

Usually, Ceph checks/acquires caps at its own entry points and not in
the `address_space_operations`: ceph_read_iter() and
ceph_filemap_fault() acquire Fr/Fc before calling filemap_read() or
filemap_fault(), and ceph_write_iter() holds Fw/Fb around write_begin.

Only readahead, which the VM can invoke without a Ceph entry point
above it, checks caps itself.  That check was added by commit
2b1ac852eb67 ("ceph: try getting buffer capability for readahead/fadvise")
in 2016 to the readpages path, converted to the rw
context list by commit 5d988308283e ("ceph: track read contexts in
ceph_file_info"), and moved into ceph_init_request() for
`origin==NETFS_READAHEAD` by commit a5c9dc445139 ("ceph: Make
ceph_init_request() check caps on readahead").

The single-folio read path never had such a check, neither in the old
ceph_readpage() nor in netfs_read_folio() via ceph_init_request().  It
assumes that the `read_folio` method is only ever reached from
filemap_read() or filemap_fault(), both of which Ceph wraps.

However, since Linux 6.12, erofs file-backed mounts
(commit ce63cb62d794 ("erofs: support unencoded inodes for fileio"))
read all metadata (including the superblock) by calling
read_mapping_folio() directly on the backing file's mapping.  On Ceph,
this issues an OSD read without holding any caps.

The result is silent data corruption.  Ceph clients do not write back
dirty pages on close(); a writer keeps Fb and its dirty data until the
MDS revokes the cap.  When another client opens the file, the MDS
initiates that revoke and replies to the open immediately.  A read()
would now block in ceph_get_caps() until the writer has flushed and
acked the cap-revoke, but the erofs superblock read goes to the OSD
without acquiring caps and thus races with the writeback.  If the
object does not exist yet, the OSD returns -ENOENT, which
finish_netfs_read() treats as "success, no data", and netfs zero-fills
the folio.  The folio is marked uptodate and, because it counts as
downloaded from the server, is also copied into fscache.  This never
recovers because Ceph invalidates the page cache and fscache only when
Fc is revoked, but gaining Fc later will not invalidate it.

My patch fixes this in ceph_init_request() by reusing the existing
`NETFS_READAHEAD` code for `NETFS_READPAGE`, but uses
__ceph_get_caps() instead of ceph_try_get_caps() to make it blocking
(something which would be undesirable for readahead).

Fixes: ce63cb62d794 ("erofs: support unencoded inodes for fileio")
Cc: stable@vger.kernel.org
Signed-off-by: Max Kellermann <max.kellermann@ionos.com>
---
 fs/ceph/addr.c | 69 ++++++++++++++++++++++++++++++++------------------
 1 file changed, 45 insertions(+), 24 deletions(-)

diff --git a/fs/ceph/addr.c b/fs/ceph/addr.c
index 657c2cb0f881..580b45a68a75 100644
--- a/fs/ceph/addr.c
+++ b/fs/ceph/addr.c
@@ -468,6 +468,7 @@ static int ceph_init_request(struct netfs_io_request *rreq, struct file *file)
 	struct inode *inode = rreq->inode;
 	struct ceph_fs_client *fsc = ceph_inode_to_fs_client(inode);
 	struct ceph_client *cl = ceph_inode_to_client(inode);
+	struct ceph_file_info *fi = file ? file->private_data : NULL;
 	int got = 0, want = CEPH_CAP_FILE_CACHE;
 	struct ceph_netfs_request_data *priv;
 	int ret = 0;
@@ -475,45 +476,65 @@ static int ceph_init_request(struct netfs_io_request *rreq, struct file *file)
 	/* [DEPRECATED] Use PG_private_2 to mark folio being written to the cache. */
 	__set_bit(NETFS_RREQ_USE_PGPRIV2, &rreq->flags);
 
-	if (rreq->origin != NETFS_READAHEAD)
+	if (rreq->origin != NETFS_READAHEAD && rreq->origin != NETFS_READPAGE)
 		return 0;
 
 	priv = kzalloc_obj(*priv, GFP_NOFS);
 	if (!priv)
 		return -ENOMEM;
 
-	if (file) {
-		struct ceph_rw_context *rw_ctx;
-		struct ceph_file_info *fi = file->private_data;
-
+	if (fi) {
 		priv->file_ra_pages = file->f_ra.ra_pages;
 		priv->file_ra_disabled = file->f_mode & FMODE_RANDOM;
 
-		rw_ctx = ceph_find_rw_context(fi);
-		if (rw_ctx) {
+		/*
+		 * ceph_read_iter() and ceph_filemap_fault() hold caps and
+		 * register an rw context before entering the page cache.
+		 */
+		if (ceph_find_rw_context(fi)) {
 			rreq->netfs_priv = priv;
 			return 0;
 		}
 	}
 
-	/*
-	 * readahead callers do not necessarily hold Fcb caps
-	 * (e.g. fadvise, madvise).
-	 */
-	ret = ceph_try_get_caps(inode, CEPH_CAP_FILE_RD, want, true, &got);
-	if (ret < 0) {
-		doutc(cl, "%llx.%llx, error getting cap\n", ceph_vinop(inode));
-		goto out;
-	}
+	if (rreq->origin == NETFS_READAHEAD) {
+		/*
+		 * readahead callers do not necessarily hold Fcb caps
+		 * (e.g. fadvise, madvise).  Readahead is optional, so do
+		 * not block; without caps the VM falls back to read_folio.
+		 */
+		ret = ceph_try_get_caps(inode, CEPH_CAP_FILE_RD, want, true,
+					&got);
+		if (ret < 0) {
+			doutc(cl, "%llx.%llx, error getting cap\n",
+			      ceph_vinop(inode));
+			goto out;
+		}
 
-	if (!(got & want)) {
-		doutc(cl, "%llx.%llx, no cache cap\n", ceph_vinop(inode));
-		ret = -EACCES;
-		goto out;
-	}
-	if (ret == 0) {
-		ret = -EACCES;
-		goto out;
+		if (!(got & want)) {
+			doutc(cl, "%llx.%llx, no cache cap\n", ceph_vinop(inode));
+			ret = -EACCES;
+			goto out;
+		}
+		if (ret == 0) {
+			ret = -EACCES;
+			goto out;
+		}
+	} else {
+		/*
+		 * Make sure we have caps, just in case read_folio was
+		 * called directly (e.g. by erofs) which may have
+		 * bypassed the usual Ceph cap checks.  This branch
+		 * uses __ceph_get_caps() which blocks until the
+		 * required cap has been granted.
+		 */
+		ret = __ceph_get_caps(inode, fi, CEPH_CAP_FILE_RD, want, -1,
+				      &got);
+		if (ret < 0) {
+			doutc(cl, "%llx.%llx, error getting cap\n",
+			      ceph_vinop(inode));
+			goto out;
+		}
 	}
 
 	priv->caps = got;
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] ceph: acquire caps for read_folio requests without an rw context
  2026-09-02 12:40 [PATCH] ceph: acquire caps for read_folio requests without an rw context Max Kellermann
@ 2026-09-04  4:54 ` Xiubo Li
  0 siblings, 0 replies; 2+ messages in thread
From: Xiubo Li @ 2026-09-04  4:54 UTC (permalink / raw)
  To: Max Kellermann; +Cc: idryomov, amarkuze, ceph-devel, linux-kernel, stable

Hi Max,

The fix mechanism looks right: a NETFS_READPAGE request issued via
read_mapping_folio() with no rw context should acquire caps, blocking,
before hitting the OSD, so it can't race a writer that hasn't flushed
yet. Two things though:

There is one existing Ceph-internal caller that now lands in the new
blocking branch: ceph_uninline_data() calls
read_mapping_folio(inode->i_mapping, 0, file) from ceph_init_file_info()
(fs/ceph/file.c:262) at open() time, with no rw context. On an
O_WRONLY open of an inline-data file with no recent readers, RD isn't
part of the file's wanted caps (__ceph_caps_file_wanted() derives them
from the used fmode), so __ceph_get_caps(RD) takes the -EUCLEAN path
into ceph_renew_caps(), which raises the wanted set via
__ceph_touch_fmode() and issues a synchronous MDS open request. Such an
open now pays an extra sync MDS round-trip, and open() can newly fail on
MDS errors. This read also doesn't need caps: the OSD object is not
created until the uninline CREATE, and this path intentionally relies
on the -ENOENT -> zero-fill behavior. Could you handle this internal
caller explicitly (e.g. have ceph_uninline_data()'s read bypass the caps
check), rather than making every cap-less READPAGE unconditionally
block?

The blocking acquisition now happens with the folio locked
(read_folio contract), inverting the usual caps-before-folio-lock order
of ceph_read_iter()/ceph_filemap_fault(). I checked the obvious cycles
and didn't find one: a folio that reaches read_folio is clean (dirty =>
uptodate => read_folio is never called, and netfs_read_folio() diverts
dirty folios to netfs_read_gaps() before the request is allocated), so
the Fb-revoke flush/writeback never needs to lock it, and the truncate
path uses folio_trylock. But please document that invariant in a
comment, since it's what makes waiting for caps under the folio lock
safe.

Minor: the new branch hardcodes want = CEPH_CAP_FILE_CACHE, while the
other read paths add CEPH_CAP_FILE_LAZYIO for lazyio files
(ceph_filemap_fault(), ceph_splice_read()); it may also be worth
aligning.

It might also help stable reviewers if the commit message notes that
current mainline erofs fileio reads via vfs_iocb_iter_read() ->
ceph_read_iter(), so the trigger is gone there and the patch mainly
matters for stable kernels and future direct read_mapping_folio()
callers.

Thanks!
- Xiubo

On Wed, 2 Sept 2026 at 05:40, Max Kellermann <max.kellermann@ionos.com> wrote:
>
> This fixes a data corruption problem that leaked permanently into
> fscache.  File-backed erofs images are read using
> read_mapping_folio(), and the Ceph implementation of this call forgets
> to acquire Ceph caps.  Therefore, mounting an erofs image file from a
> Ceph mount that was just written (but not yet committed to the OSD)
> would fail because the erofs code saw only zero-filled pages.  These
> zero-filled backes were then copied to the fscache, making this data
> corruption permanent (on this host).
>
> Usually, Ceph checks/acquires caps at its own entry points and not in
> the `address_space_operations`: ceph_read_iter() and
> ceph_filemap_fault() acquire Fr/Fc before calling filemap_read() or
> filemap_fault(), and ceph_write_iter() holds Fw/Fb around write_begin.
>
> Only readahead, which the VM can invoke without a Ceph entry point
> above it, checks caps itself.  That check was added by commit
> 2b1ac852eb67 ("ceph: try getting buffer capability for readahead/fadvise")
> in 2016 to the readpages path, converted to the rw
> context list by commit 5d988308283e ("ceph: track read contexts in
> ceph_file_info"), and moved into ceph_init_request() for
> `origin==NETFS_READAHEAD` by commit a5c9dc445139 ("ceph: Make
> ceph_init_request() check caps on readahead").
>
> The single-folio read path never had such a check, neither in the old
> ceph_readpage() nor in netfs_read_folio() via ceph_init_request().  It
> assumes that the `read_folio` method is only ever reached from
> filemap_read() or filemap_fault(), both of which Ceph wraps.
>
> However, since Linux 6.12, erofs file-backed mounts
> (commit ce63cb62d794 ("erofs: support unencoded inodes for fileio"))
> read all metadata (including the superblock) by calling
> read_mapping_folio() directly on the backing file's mapping.  On Ceph,
> this issues an OSD read without holding any caps.
>
> The result is silent data corruption.  Ceph clients do not write back
> dirty pages on close(); a writer keeps Fb and its dirty data until the
> MDS revokes the cap.  When another client opens the file, the MDS
> initiates that revoke and replies to the open immediately.  A read()
> would now block in ceph_get_caps() until the writer has flushed and
> acked the cap-revoke, but the erofs superblock read goes to the OSD
> without acquiring caps and thus races with the writeback.  If the
> object does not exist yet, the OSD returns -ENOENT, which
> finish_netfs_read() treats as "success, no data", and netfs zero-fills
> the folio.  The folio is marked uptodate and, because it counts as
> downloaded from the server, is also copied into fscache.  This never
> recovers because Ceph invalidates the page cache and fscache only when
> Fc is revoked, but gaining Fc later will not invalidate it.
>
> My patch fixes this in ceph_init_request() by reusing the existing
> `NETFS_READAHEAD` code for `NETFS_READPAGE`, but uses
> __ceph_get_caps() instead of ceph_try_get_caps() to make it blocking
> (something which would be undesirable for readahead).
>
> Fixes: ce63cb62d794 ("erofs: support unencoded inodes for fileio")
> Cc: stable@vger.kernel.org
> Signed-off-by: Max Kellermann <max.kellermann@ionos.com>
> ---
>  fs/ceph/addr.c | 69 ++++++++++++++++++++++++++++++++------------------
>  1 file changed, 45 insertions(+), 24 deletions(-)
>
> diff --git a/fs/ceph/addr.c b/fs/ceph/addr.c
> index 657c2cb0f881..580b45a68a75 100644
> --- a/fs/ceph/addr.c
> +++ b/fs/ceph/addr.c
> @@ -468,6 +468,7 @@ static int ceph_init_request(struct netfs_io_request *rreq, struct file *file)
>         struct inode *inode = rreq->inode;
>         struct ceph_fs_client *fsc = ceph_inode_to_fs_client(inode);
>         struct ceph_client *cl = ceph_inode_to_client(inode);
> +       struct ceph_file_info *fi = file ? file->private_data : NULL;
>         int got = 0, want = CEPH_CAP_FILE_CACHE;
>         struct ceph_netfs_request_data *priv;
>         int ret = 0;
> @@ -475,45 +476,65 @@ static int ceph_init_request(struct netfs_io_request *rreq, struct file *file)
>         /* [DEPRECATED] Use PG_private_2 to mark folio being written to the cache. */
>         __set_bit(NETFS_RREQ_USE_PGPRIV2, &rreq->flags);
>
> -       if (rreq->origin != NETFS_READAHEAD)
> +       if (rreq->origin != NETFS_READAHEAD && rreq->origin != NETFS_READPAGE)
>                 return 0;
>
>         priv = kzalloc_obj(*priv, GFP_NOFS);
>         if (!priv)
>                 return -ENOMEM;
>
> -       if (file) {
> -               struct ceph_rw_context *rw_ctx;
> -               struct ceph_file_info *fi = file->private_data;
> -
> +       if (fi) {
>                 priv->file_ra_pages = file->f_ra.ra_pages;
>                 priv->file_ra_disabled = file->f_mode & FMODE_RANDOM;
>
> -               rw_ctx = ceph_find_rw_context(fi);
> -               if (rw_ctx) {
> +               /*
> +                * ceph_read_iter() and ceph_filemap_fault() hold caps and
> +                * register an rw context before entering the page cache.
> +                */
> +               if (ceph_find_rw_context(fi)) {
>                         rreq->netfs_priv = priv;
>                         return 0;
>                 }
>         }
>
> -       /*
> -        * readahead callers do not necessarily hold Fcb caps
> -        * (e.g. fadvise, madvise).
> -        */
> -       ret = ceph_try_get_caps(inode, CEPH_CAP_FILE_RD, want, true, &got);
> -       if (ret < 0) {
> -               doutc(cl, "%llx.%llx, error getting cap\n", ceph_vinop(inode));
> -               goto out;
> -       }
> +       if (rreq->origin == NETFS_READAHEAD) {
> +               /*
> +                * readahead callers do not necessarily hold Fcb caps
> +                * (e.g. fadvise, madvise).  Readahead is optional, so do
> +                * not block; without caps the VM falls back to read_folio.
> +                */
> +               ret = ceph_try_get_caps(inode, CEPH_CAP_FILE_RD, want, true,
> +                                       &got);
> +               if (ret < 0) {
> +                       doutc(cl, "%llx.%llx, error getting cap\n",
> +                             ceph_vinop(inode));
> +                       goto out;
> +               }
>
> -       if (!(got & want)) {
> -               doutc(cl, "%llx.%llx, no cache cap\n", ceph_vinop(inode));
> -               ret = -EACCES;
> -               goto out;
> -       }
> -       if (ret == 0) {
> -               ret = -EACCES;
> -               goto out;
> +               if (!(got & want)) {
> +                       doutc(cl, "%llx.%llx, no cache cap\n", ceph_vinop(inode));
> +                       ret = -EACCES;
> +                       goto out;
> +               }
> +               if (ret == 0) {
> +                       ret = -EACCES;
> +                       goto out;
> +               }
> +       } else {
> +               /*
> +                * Make sure we have caps, just in case read_folio was
> +                * called directly (e.g. by erofs) which may have
> +                * bypassed the usual Ceph cap checks.  This branch
> +                * uses __ceph_get_caps() which blocks until the
> +                * required cap has been granted.
> +                */
> +               ret = __ceph_get_caps(inode, fi, CEPH_CAP_FILE_RD, want, -1,
> +                                     &got);
> +               if (ret < 0) {
> +                       doutc(cl, "%llx.%llx, error getting cap\n",
> +                             ceph_vinop(inode));
> +                       goto out;
> +               }
>         }
>
>         priv->caps = got;
> --
> 2.47.3
>

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-09-04  4:55 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-02 12:40 [PATCH] ceph: acquire caps for read_folio requests without an rw context Max Kellermann
2026-09-04  4:54 ` Xiubo Li

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox