mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v2] ceph: acquire caps for read_folio without an rw context
@ 2026-09-04 10:51 Max Kellermann
  2026-09-05  1:06 ` Xiubo Li
  0 siblings, 1 reply; 2+ messages in thread
From: Max Kellermann @ 2026-09-04 10:51 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 reads metadata 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 pages 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 passing through a Ceph
entry point, checks caps itself.  That check was added by
commit 2b1ac852eb67 ("ceph: try getting buffer capability for
readahead/fadvise") to the readpages path, converted to use the rw
context list by commit 5d988308283e ("ceph: track read contexts in
ceph_file_info"), and moved into ceph_init_request() for
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.

Add a Ceph read_folio wrapper which acquires caps synchronously when no
rw context is present.  Cap acquisition can process a pending truncate,
so drop the folio lock before acquiring caps.  After relocking, verify
that the folio still belongs to the mapping and has not already become
uptodate before passing it to netfs_read_folio().  Leave reads with an
existing rw context and the legacy inline-data path unchanged.

Fixes: ce63cb62d794 ("erofs: support unencoded inodes for fileio")
Cc: stable@vger.kernel.org
Signed-off-by: Max Kellermann <max.kellermann@ionos.com>
---
Note:
- commit e587a984d332 ("erofs: use dedicated meta inodes for
  file-backed mounts") eliminates this trigger, so Linux 7.3-rc1 and
  later are not affected.  The fix remains relevant to older stable
  kernels and to future direct read_mapping_folio() callers.
v1 -> v2:
- Move cap acquisition from ceph_init_request() into a read_folio
  wrapper, leaving the readahead path unchanged.
- Drop the folio lock around blocking cap acquisition, then relock and
  revalidate the folio to avoid a truncate deadlock.
- Preserve the legacy inline-data path.
- Handle a NULL file argument without acquiring caps a second time.
- Require Fc before populating the page cache; CEPH_CAP_FILE_LAZYIO
  cannot be added because the want mask is conjunctive

Signed-off-by: Max Kellermann <max.kellermann@ionos.com>
---
 fs/ceph/addr.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 57 insertions(+), 1 deletion(-)

diff --git a/fs/ceph/addr.c b/fs/ceph/addr.c
index 657c2cb0f881..78b805ccc5d4 100644
--- a/fs/ceph/addr.c
+++ b/fs/ceph/addr.c
@@ -1959,8 +1959,64 @@ static int ceph_write_end(const struct kiocb *iocb,
 	return copied;
 }
 
+static int ceph_read_folio(struct file *file, struct folio *folio)
+{
+	struct inode *inode = folio_inode(folio);
+	struct ceph_inode_info *ci = ceph_inode(inode);
+	struct ceph_file_info *fi = file ? file->private_data : NULL;
+	int got = 0;
+	int ret;
+
+	/*
+	 * Existing Ceph read paths acquire caps before entering the page
+	 * cache.  Keep the legacy inline-data path unchanged.
+	 */
+	if (ceph_has_inline_data(ci) ||
+	    (fi && ceph_find_rw_context(fi)))
+		return netfs_read_folio(file, folio);
+
+	/*
+	 * Cap acquisition can process a pending truncate, which may lock and
+	 * remove this folio.  It must therefore happen without the folio lock.
+	 */
+	folio_unlock(folio);
+	ret = __ceph_get_caps(inode, fi, CEPH_CAP_FILE_RD,
+			      CEPH_CAP_FILE_CACHE, -1, &got);
+	if (ret < 0)
+		return ret;
+
+	if (!(got & CEPH_CAP_FILE_CACHE)) {
+		ret = -EACCES;
+		goto out;
+	}
+
+	folio_lock(folio);
+
+	/*
+	 * After re-locking, check if the folio still belongs to the
+	 * mapping...
+	 */
+	if (folio->mapping != inode->i_mapping) {
+		folio_unlock(folio);
+		ret = AOP_TRUNCATED_PAGE;
+		goto out;
+	}
+
+	/* .. or has been filled already meanwhile */
+	if (folio_test_uptodate(folio)) {
+		folio_unlock(folio);
+		ret = 0;
+		goto out;
+	}
+
+	ret = netfs_read_folio(file, folio);
+out:
+	ceph_put_cap_refs(ci, got);
+	return ret;
+}
+
 const struct address_space_operations ceph_aops = {
-	.read_folio = netfs_read_folio,
+	.read_folio = ceph_read_folio,
 	.readahead = netfs_readahead,
 	.writepages = ceph_writepages_start,
 	.write_begin = ceph_write_begin,
-- 
2.47.3


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

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

LGTM.

Reviewed-by: Xiubo Li <xiubo.li@clyso.com>

On Fri, 4 Sept 2026 at 03:51, Max Kellermann <max.kellermann@ionos.com> wrote:
>
> This fixes a data corruption problem that leaked permanently into
> fscache.  File-backed erofs reads metadata 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 pages 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 passing through a Ceph
> entry point, checks caps itself.  That check was added by
> commit 2b1ac852eb67 ("ceph: try getting buffer capability for
> readahead/fadvise") to the readpages path, converted to use the rw
> context list by commit 5d988308283e ("ceph: track read contexts in
> ceph_file_info"), and moved into ceph_init_request() for
> 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.
>
> Add a Ceph read_folio wrapper which acquires caps synchronously when no
> rw context is present.  Cap acquisition can process a pending truncate,
> so drop the folio lock before acquiring caps.  After relocking, verify
> that the folio still belongs to the mapping and has not already become
> uptodate before passing it to netfs_read_folio().  Leave reads with an
> existing rw context and the legacy inline-data path unchanged.
>
> Fixes: ce63cb62d794 ("erofs: support unencoded inodes for fileio")
> Cc: stable@vger.kernel.org
> Signed-off-by: Max Kellermann <max.kellermann@ionos.com>
> ---
> Note:
> - commit e587a984d332 ("erofs: use dedicated meta inodes for
>   file-backed mounts") eliminates this trigger, so Linux 7.3-rc1 and
>   later are not affected.  The fix remains relevant to older stable
>   kernels and to future direct read_mapping_folio() callers.
> v1 -> v2:
> - Move cap acquisition from ceph_init_request() into a read_folio
>   wrapper, leaving the readahead path unchanged.
> - Drop the folio lock around blocking cap acquisition, then relock and
>   revalidate the folio to avoid a truncate deadlock.
> - Preserve the legacy inline-data path.
> - Handle a NULL file argument without acquiring caps a second time.
> - Require Fc before populating the page cache; CEPH_CAP_FILE_LAZYIO
>   cannot be added because the want mask is conjunctive
>
> Signed-off-by: Max Kellermann <max.kellermann@ionos.com>
> ---
>  fs/ceph/addr.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 57 insertions(+), 1 deletion(-)
>
> diff --git a/fs/ceph/addr.c b/fs/ceph/addr.c
> index 657c2cb0f881..78b805ccc5d4 100644
> --- a/fs/ceph/addr.c
> +++ b/fs/ceph/addr.c
> @@ -1959,8 +1959,64 @@ static int ceph_write_end(const struct kiocb *iocb,
>         return copied;
>  }
>
> +static int ceph_read_folio(struct file *file, struct folio *folio)
> +{
> +       struct inode *inode = folio_inode(folio);
> +       struct ceph_inode_info *ci = ceph_inode(inode);
> +       struct ceph_file_info *fi = file ? file->private_data : NULL;
> +       int got = 0;
> +       int ret;
> +
> +       /*
> +        * Existing Ceph read paths acquire caps before entering the page
> +        * cache.  Keep the legacy inline-data path unchanged.
> +        */
> +       if (ceph_has_inline_data(ci) ||
> +           (fi && ceph_find_rw_context(fi)))
> +               return netfs_read_folio(file, folio);
> +
> +       /*
> +        * Cap acquisition can process a pending truncate, which may lock and
> +        * remove this folio.  It must therefore happen without the folio lock.
> +        */
> +       folio_unlock(folio);
> +       ret = __ceph_get_caps(inode, fi, CEPH_CAP_FILE_RD,
> +                             CEPH_CAP_FILE_CACHE, -1, &got);
> +       if (ret < 0)
> +               return ret;
> +
> +       if (!(got & CEPH_CAP_FILE_CACHE)) {
> +               ret = -EACCES;
> +               goto out;
> +       }
> +
> +       folio_lock(folio);
> +
> +       /*
> +        * After re-locking, check if the folio still belongs to the
> +        * mapping...
> +        */
> +       if (folio->mapping != inode->i_mapping) {
> +               folio_unlock(folio);
> +               ret = AOP_TRUNCATED_PAGE;
> +               goto out;
> +       }
> +
> +       /* .. or has been filled already meanwhile */
> +       if (folio_test_uptodate(folio)) {
> +               folio_unlock(folio);
> +               ret = 0;
> +               goto out;
> +       }
> +
> +       ret = netfs_read_folio(file, folio);
> +out:
> +       ceph_put_cap_refs(ci, got);
> +       return ret;
> +}
> +
>  const struct address_space_operations ceph_aops = {
> -       .read_folio = netfs_read_folio,
> +       .read_folio = ceph_read_folio,
>         .readahead = netfs_readahead,
>         .writepages = ceph_writepages_start,
>         .write_begin = ceph_write_begin,
> --
> 2.47.3
>

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

end of thread, other threads:[~2026-09-05  1:07 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-04 10:51 [PATCH v2] ceph: acquire caps for read_folio without an rw context Max Kellermann
2026-09-05  1:06 ` Xiubo Li

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

all inboxes | Powered by JetHome®