[PATCH 0/9] ceph: fix mdsmap and osdmap use-after-free in unlocked readers

[PATCH 0/9] ceph: fix mdsmap and osdmap use-after-free in unlocked readers

[Max Kellermann]

The MDS and OSD clients replace their maps and free the old ones
under a lock: mdsc->mutex for the mdsmap and osdc->lock for the
osdmap.  Several readers dereference these maps without that lock and
can therefore access freed memory if a new map arrives concurrently.
The worst is ceph_statfs(), which any user can reach and which loads
and dereferences a pointer from the old mdsmap.

Each patch takes the lock that protects the replacement around the
reads.  __ceph_setxattr() cannot take mdsc->mutex under i_ceph_lock,
so it uses a copy of m_max_xattr_size kept in the mds client.

The same problem in ceph_compare_options() was already fixed by
commit 19db525816db ("libceph: use RCU to protect monmap in
ceph_compare_options()").

These bugs were found by code review and have not been reproduced.

Max Kellermann (9):
  ceph: fix mdsmap use-after-free in ceph_statfs()
  ceph: fix mdsmap use-after-free in debugfs mdsmap_show()
  ceph: fix mdsmap use-after-free in __ceph_setxattr()
  ceph: fix mdsmap use-after-free in send_renew_caps() and
    renewed_caps()
  ceph: fix mdsmap use-after-free in mds_peer_reset()
  ceph: fix mdsmap use-after-free in ceph_get_tree() error path
  ceph: fix osdmap use-after-free in ceph_sync_setxattr()
  libceph: fix osdmap use-after-free in ceph_wait_for_latest_osdmap()
  rbd: fix osdmap use-after-free in pool lookups

 drivers/block/rbd.c    | 13 +++++++++++--
 fs/ceph/debugfs.c      | 12 ++++++++++--
 fs/ceph/mds_client.c   | 18 ++++++++++++++++--
 fs/ceph/mds_client.h   |  7 +++++++
 fs/ceph/super.c        | 19 ++++++++++++++-----
 fs/ceph/xattr.c        | 13 +++++++++----
 net/ceph/ceph_common.c |  8 +++++++-
 7 files changed, 74 insertions(+), 16 deletions(-)

-- 
2.53.0

[PATCH 1/9] ceph: fix mdsmap use-after-free in ceph_statfs()

[Max Kellermann]

ceph_statfs() reads mdsc->mdsmap without holding mdsc->mutex, but
ceph_mdsc_handle_mdsmap() replaces the mdsmap and frees the old one
under that mutex.  A statfs() racing with an mdsmap update can
therefore read m_num_data_pg_pools from freed memory, and then load
the m_data_pg_pools pointer from it and dereference it.

Any user can call statfs(), and mdsmap updates happen whenever an
MDS changes state.

Read the data pool while holding mdsc->mutex.

Fixes: 06d74376c8af ("ceph: more accurate statfs")
Cc: stable@vger.kernel.org
Signed-off-by: Max Kellermann <max.kellermann@ionos.com>
---
 fs/ceph/super.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/fs/ceph/super.c b/fs/ceph/super.c
index bb5591c1e154..01f92d77ef45 100644
--- a/fs/ceph/super.c
+++ b/fs/ceph/super.c
@@ -64,6 +64,7 @@ static int ceph_statfs(struct dentry *dentry, struct kstatfs *buf)
 {
 	struct ceph_fs_client *fsc = ceph_inode_to_fs_client(d_inode(dentry));
 	struct ceph_mon_client *monc = &fsc->client->monc;
+	struct ceph_mds_client *mdsc = fsc->mdsc;
 	struct ceph_monmap *monmap;
 	struct ceph_statfs st;
 	int i, err;
@@ -73,11 +74,14 @@ static int ceph_statfs(struct dentry *dentry, struct kstatfs *buf)
 	ceph_blog_enter(fsc, &__ji);
 
 	boutc(fsc->client, "begin\n");
-	if (fsc->mdsc->mdsmap->m_num_data_pg_pools == 1) {
-		data_pool = fsc->mdsc->mdsmap->m_data_pg_pools[0];
-	} else {
+
+	/* ceph_mdsc_handle_mdsmap() replaces and frees the mdsmap */
+	mutex_lock(&mdsc->mutex);
+	if (mdsc->mdsmap->m_num_data_pg_pools == 1)
+		data_pool = mdsc->mdsmap->m_data_pg_pools[0];
+	else
 		data_pool = CEPH_NOPOOL;
-	}
+	mutex_unlock(&mdsc->mutex);
 
 	err = ceph_monc_do_statfs(monc, data_pool, &st);
 	if (err < 0) {
-- 
2.53.0

[PATCH 2/9] ceph: fix mdsmap use-after-free in debugfs mdsmap_show()

[Max Kellermann]

mdsmap_show() walks mdsc->mdsmap without holding mdsc->mutex, but
ceph_mdsc_handle_mdsmap() replaces the mdsmap and frees the old one
under that mutex.  Reading the debugfs "mdsmap" file while an MDS
changes state can therefore access freed memory, including the m_info
array.

Hold mdsc->mutex while printing the map.

Fixes: 76aa844d5b2f ("ceph: debugfs")
Cc: stable@vger.kernel.org
Signed-off-by: Max Kellermann <max.kellermann@ionos.com>
---
 fs/ceph/debugfs.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/fs/ceph/debugfs.c b/fs/ceph/debugfs.c
index 6a9a5551d9d7..01c9f1896aa1 100644
--- a/fs/ceph/debugfs.c
+++ b/fs/ceph/debugfs.c
@@ -59,11 +59,17 @@ static int mdsmap_show(struct seq_file *s, void *p)
 {
 	int i;
 	struct ceph_fs_client *fsc = s->private;
+	struct ceph_mds_client *mdsc = fsc->mdsc;
 	struct ceph_mdsmap *mdsmap;
 
-	if (!fsc->mdsc || !fsc->mdsc->mdsmap)
+	if (!mdsc)
 		return 0;
-	mdsmap = fsc->mdsc->mdsmap;
+
+	/* ceph_mdsc_handle_mdsmap() replaces and frees the mdsmap */
+	mutex_lock(&mdsc->mutex);
+	mdsmap = mdsc->mdsmap;
+	if (!mdsmap)
+		goto out;
 	seq_printf(s, "epoch %d\n", mdsmap->m_epoch);
 	seq_printf(s, "root %d\n", mdsmap->m_root);
 	seq_printf(s, "max_mds %d\n", mdsmap->m_max_mds);
@@ -76,6 +82,8 @@ static int mdsmap_show(struct seq_file *s, void *p)
 			       ceph_pr_addr(addr),
 			       ceph_mds_state_name(state));
 	}
+out:
+	mutex_unlock(&mdsc->mutex);
 	return 0;
 }
 
-- 
2.53.0

[PATCH 3/9] ceph: fix mdsmap use-after-free in __ceph_setxattr()

[Max Kellermann]

__ceph_setxattr() reads mdsc->mdsmap->m_max_xattr_size while holding
only ci->i_ceph_lock, but ceph_mdsc_handle_mdsmap() replaces the
mdsmap and frees the old one under mdsc->mutex.  A setxattr() racing
with an mdsmap update can therefore read freed memory.

mdsc->mutex cannot be taken under i_ceph_lock, so keep a copy of the
limit in struct ceph_mds_client, updated by ceph_mdsc_handle_mdsmap()
under the mutex, and read that instead.

Fixes: d93231a6bc8a ("ceph: prevent a client from exceeding the MDS maximum xattr size")
Cc: stable@vger.kernel.org
Signed-off-by: Max Kellermann <max.kellermann@ionos.com>
---
 fs/ceph/mds_client.c |  1 +
 fs/ceph/mds_client.h |  7 +++++++
 fs/ceph/xattr.c      | 10 ++++++----
 3 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
index 15df9d22008d..da40f2cb80f2 100644
--- a/fs/ceph/mds_client.c
+++ b/fs/ceph/mds_client.c
@@ -7247,6 +7247,7 @@ void ceph_mdsc_handle_mdsmap(struct ceph_mds_client *mdsc, struct ceph_msg *msg)
 	} else {
 		mdsc->mdsmap = newmap;  /* first mds map */
 	}
+	WRITE_ONCE(mdsc->max_xattr_size, mdsc->mdsmap->m_max_xattr_size);
 	mdsc->fsc->max_file_size = min((loff_t)mdsc->mdsmap->m_max_file_size,
 					MAX_LFS_FILESIZE);
 
diff --git a/fs/ceph/mds_client.h b/fs/ceph/mds_client.h
index ff9ba09f3b59..bd97f73ede20 100644
--- a/fs/ceph/mds_client.h
+++ b/fs/ceph/mds_client.h
@@ -495,6 +495,13 @@ struct ceph_mds_client {
 	struct mutex            mutex;         /* all nested structures */
 
 	struct ceph_mdsmap      *mdsmap;
+
+	/*
+	 * Copy of mdsmap->m_max_xattr_size for readers that cannot take
+	 * the mutex; updated by ceph_mdsc_handle_mdsmap().
+	 */
+	u64			max_xattr_size;
+
 	struct completion       safe_umount_waiters;
 	wait_queue_head_t       session_close_wq;
 	struct list_head        waiting_for_map;
diff --git a/fs/ceph/xattr.c b/fs/ceph/xattr.c
index c6085191c29a..8f97b6448d7a 100644
--- a/fs/ceph/xattr.c
+++ b/fs/ceph/xattr.c
@@ -1211,6 +1211,7 @@ int __ceph_setxattr(struct inode *inode, const char *name,
 	struct ceph_mds_client *mdsc = fsc->mdsc;
 	struct ceph_cap_flush *prealloc_cf = NULL;
 	struct ceph_buffer *old_blob = NULL;
+	u64 max_xattr_size;
 	int issued;
 	int err;
 	int dirty = 0;
@@ -1260,13 +1261,14 @@ int __ceph_setxattr(struct inode *inode, const char *name,
 
 	spin_lock(&ci->i_ceph_lock);
 retry:
+	max_xattr_size = READ_ONCE(mdsc->max_xattr_size);
 	issued = __ceph_caps_issued(ci, NULL);
 	required_blob_size = __get_required_blob_size(ci, name_len, val_len);
 	if ((ci->i_xattrs.version == 0) || !(issued & CEPH_CAP_XATTR_EXCL) ||
-	    (required_blob_size > mdsc->mdsmap->m_max_xattr_size)) {
+	    (required_blob_size > max_xattr_size)) {
 		boutc(cl, "sync version: %llu size: %d max: %llu\n",
 		      ci->i_xattrs.version, required_blob_size,
-		      mdsc->mdsmap->m_max_xattr_size);
+		      max_xattr_size);
 		goto do_sync;
 	}
 
@@ -1294,9 +1296,9 @@ int __ceph_setxattr(struct inode *inode, const char *name,
 	 * prealloc_blob size check below reflects the current tree state.
 	 */
 	required_blob_size = __get_required_blob_size(ci, name_len, val_len);
-	if (required_blob_size > mdsc->mdsmap->m_max_xattr_size) {
+	if (required_blob_size > max_xattr_size) {
 		boutc(cl, "sync (size too large): %d > %llu\n",
-		      required_blob_size, mdsc->mdsmap->m_max_xattr_size);
+		      required_blob_size, max_xattr_size);
 		goto do_sync;
 	}
 
-- 
2.53.0

[PATCH 4/9] ceph: fix mdsmap use-after-free in send_renew_caps() and renewed_caps()

[Max Kellermann]

send_renew_caps() and renewed_caps() read mdsc->mdsmap while holding
only session->s_mutex, but ceph_mdsc_handle_mdsmap() replaces the
mdsmap and frees the old one under mdsc->mutex.  They are called from
delayed_work() and handle_session(), both of which can run while a
new mdsmap arrives, so they can read the MDS state and the session
timeout from freed memory.

Read those values while holding mdsc->mutex.  Callers hold s_mutex
but not mdsc->mutex, and taking mdsc->mutex under s_mutex is the
existing lock order (e.g. check_new_map(), handle_session()).

Fixes: 2f2dc053404f ("ceph: MDS client")
Cc: stable@vger.kernel.org
Signed-off-by: Max Kellermann <max.kellermann@ionos.com>
---
 fs/ceph/mds_client.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
index da40f2cb80f2..a43ce23d48f4 100644
--- a/fs/ceph/mds_client.c
+++ b/fs/ceph/mds_client.c
@@ -2190,7 +2190,9 @@ static int send_renew_caps(struct ceph_mds_client *mdsc,
 
 	/* do not try to renew caps until a recovering mds has reconnected
 	 * with its clients. */
+	mutex_lock(&mdsc->mutex);
 	state = ceph_mdsmap_get_state(mdsc->mdsmap, session->s_mds);
+	mutex_unlock(&mdsc->mutex);
 	if (state < CEPH_MDS_STATE_RECONNECT) {
 		boutc(cl, "ignoring mds%d (%s)\n", session->s_mds,
 		      ceph_mds_state_name(state));
@@ -2232,14 +2234,19 @@ static void renewed_caps(struct ceph_mds_client *mdsc,
 			 struct ceph_mds_session *session, int is_renew)
 {
 	struct ceph_client *cl = mdsc->fsc->client;
+	u32 session_timeout;
 	int was_stale;
 	int wake = 0;
 
+	mutex_lock(&mdsc->mutex);
+	session_timeout = mdsc->mdsmap->m_session_timeout;
+	mutex_unlock(&mdsc->mutex);
+
 	spin_lock(&session->s_cap_lock);
 	was_stale = is_renew && time_after_eq(jiffies, session->s_cap_ttl);
 
 	session->s_cap_ttl = session->s_renew_requested +
-		mdsc->mdsmap->m_session_timeout*HZ;
+		session_timeout * HZ;
 
 	if (was_stale) {
 		if (time_before(jiffies, session->s_cap_ttl)) {
-- 
2.53.0

[PATCH 5/9] ceph: fix mdsmap use-after-free in mds_peer_reset()

[Max Kellermann]

mds_peer_reset() reads the MDS state from mdsc->mdsmap without holding
mdsc->mutex, but ceph_mdsc_handle_mdsmap() replaces the mdsmap and
frees the old one under that mutex.  A peer reset often coincides with
an MDS failover, which is exactly when a new mdsmap arrives, so this
can read freed memory.

Read the state while holding mdsc->mutex.  The messenger drops
con->mutex before calling ->peer_reset(), so no lock is held here.

Fixes: cbcb358b744b ("ceph: skip reconnecting if MDS is not ready")
Cc: stable@vger.kernel.org
Signed-off-by: Max Kellermann <max.kellermann@ionos.com>
---
 fs/ceph/mds_client.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
index a43ce23d48f4..aa4406fd65b9 100644
--- a/fs/ceph/mds_client.c
+++ b/fs/ceph/mds_client.c
@@ -7300,13 +7300,19 @@ static void mds_peer_reset(struct ceph_connection *con)
 {
 	struct ceph_mds_session *s = con->private;
 	struct ceph_mds_client *mdsc = s->s_mdsc;
+	int mds_state;
 	int rc;
 
 	pr_warn_client(mdsc->fsc->client, "mds%d closed our session\n",
 		       s->s_mds);
 
+	/* ceph_mdsc_handle_mdsmap() replaces and frees the mdsmap */
+	mutex_lock(&mdsc->mutex);
+	mds_state = ceph_mdsmap_get_state(mdsc->mdsmap, s->s_mds);
+	mutex_unlock(&mdsc->mutex);
+
 	if (READ_ONCE(mdsc->fsc->mount_state) == CEPH_MOUNT_FENCE_IO ||
-	    ceph_mdsmap_get_state(mdsc->mdsmap, s->s_mds) < CEPH_MDS_STATE_RECONNECT)
+	    mds_state < CEPH_MDS_STATE_RECONNECT)
 		return;
 
 	/*
-- 
2.53.0

[PATCH 6/9] ceph: fix mdsmap use-after-free in ceph_get_tree() error path

[Max Kellermann]

When mounting fails, ceph_get_tree() calls
ceph_mdsmap_is_cluster_available() on mdsc->mdsmap without holding
mdsc->mutex.  The MDS client is still running at that point, and
ceph_mdsc_handle_mdsmap() can replace the mdsmap and free the old one
concurrently, so this can walk freed memory.

Check the map while holding mdsc->mutex.

Fixes: 97820058fb28 ("ceph: check availability of mds cluster on mount after wait timeout")
Cc: stable@vger.kernel.org
Signed-off-by: Max Kellermann <max.kellermann@ionos.com>
---
 fs/ceph/super.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/fs/ceph/super.c b/fs/ceph/super.c
index 01f92d77ef45..94ff935492e5 100644
--- a/fs/ceph/super.c
+++ b/fs/ceph/super.c
@@ -1364,6 +1364,7 @@ static int ceph_get_tree(struct fs_context *fc)
 	struct dentry *res;
 	int (*compare_super)(struct super_block *, struct fs_context *) =
 		ceph_compare_super;
+	bool available;
 	int err;
 
 	dout("ceph_get_tree\n");
@@ -1429,7 +1430,11 @@ static int ceph_get_tree(struct fs_context *fc)
 	return 0;
 
 out_splat:
-	if (!ceph_mdsmap_is_cluster_available(fsc->mdsc->mdsmap)) {
+	/* ceph_mdsc_handle_mdsmap() replaces and frees the mdsmap */
+	mutex_lock(&fsc->mdsc->mutex);
+	available = ceph_mdsmap_is_cluster_available(fsc->mdsc->mdsmap);
+	mutex_unlock(&fsc->mdsc->mutex);
+	if (!available) {
 		pr_info("No mds server is up or the cluster is laggy\n");
 		err = -EHOSTUNREACH;
 	}
-- 
2.53.0

[PATCH 7/9] ceph: fix osdmap use-after-free in ceph_sync_setxattr()

[Max Kellermann]

ceph_sync_setxattr() reads osdc->osdmap->epoch without holding
osdc->lock, but ceph_osdc_handle_map() replaces the osdmap and frees
the old one under that lock (held for writing).  A setxattr() racing
with an osdmap update can therefore read the epoch from freed memory
and send a garbage epoch to the MDS, which uses it to decide whether
it needs a newer osdmap to validate the layout.

Read the epoch while holding osdc->lock for reading.

Fixes: 968cd14edc3a ("ceph: set osdmap epoch for setxattr")
Cc: stable@vger.kernel.org
Signed-off-by: Max Kellermann <max.kellermann@ionos.com>
---
 fs/ceph/xattr.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/ceph/xattr.c b/fs/ceph/xattr.c
index 8f97b6448d7a..64ccf70aa3dd 100644
--- a/fs/ceph/xattr.c
+++ b/fs/ceph/xattr.c
@@ -1179,8 +1179,11 @@ static int ceph_sync_setxattr(struct inode *inode, const char *name,
 
 	if (op == CEPH_MDS_OP_SETXATTR) {
 		req->r_args.setxattr.flags = cpu_to_le32(flags);
+		/* ceph_osdc_handle_map() replaces and frees the osdmap */
+		down_read(&osdc->lock);
 		req->r_args.setxattr.osdmap_epoch =
 			cpu_to_le32(osdc->osdmap->epoch);
+		up_read(&osdc->lock);
 		req->r_pagelist = pagelist;
 		pagelist = NULL;
 	}
-- 
2.53.0

[PATCH 8/9] libceph: fix osdmap use-after-free in ceph_wait_for_latest_osdmap()

[Max Kellermann]

ceph_wait_for_latest_osdmap() reads osdc->osdmap->epoch without
holding osdc->lock, but ceph_osdc_handle_map() replaces the osdmap and
frees the old one under that lock (held for writing).  The function
waits for the monitor's reply first, so an osdmap update arriving in
the meantime is likely, and the epoch can then be read from freed
memory.

Read the epoch while holding osdc->lock for reading.

Fixes: bb229bbb3bf6 ("libceph: wait for latest osdmap in ceph_monc_blacklist_add()")
Cc: stable@vger.kernel.org
Signed-off-by: Max Kellermann <max.kellermann@ionos.com>
---
 net/ceph/ceph_common.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/net/ceph/ceph_common.c b/net/ceph/ceph_common.c
index 67e88f30409d..39a0ddde3554 100644
--- a/net/ceph/ceph_common.c
+++ b/net/ceph/ceph_common.c
@@ -863,13 +863,19 @@ int ceph_wait_for_latest_osdmap(struct ceph_client *client,
 				unsigned long timeout)
 {
 	u64 newest_epoch;
+	u32 epoch;
 	int ret;
 
 	ret = ceph_monc_get_version(&client->monc, "osdmap", &newest_epoch);
 	if (ret)
 		return ret;
 
-	if (client->osdc.osdmap->epoch >= newest_epoch)
+	/* ceph_osdc_handle_map() replaces and frees the osdmap */
+	down_read(&client->osdc.lock);
+	epoch = client->osdc.osdmap->epoch;
+	up_read(&client->osdc.lock);
+
+	if (epoch >= newest_epoch)
 		return 0;
 
 	ceph_osdc_maybe_request_map(&client->osdc);
-- 
2.53.0

[PATCH 9/9] rbd: fix osdmap use-after-free in pool lookups

[Max Kellermann]

rbd_spec_fill_names() and do_rbd_add() look up pools in osdc->osdmap
without holding osdc->lock, but ceph_osdc_handle_map() replaces the
osdmap and frees the old one under that lock (held for writing).  An
"rbd map" racing with an osdmap update can therefore walk the freed
pool tree, and rbd_spec_fill_names() can kstrdup() a pool name that
has been freed already.

Do the lookups while holding osdc->lock for reading, and copy the pool
name before releasing it.  Use GFP_NOIO for the copy, because reclaim
could issue I/O to an rbd device, which needs osdc->lock while a writer
may be waiting for it.

Fixes: 9e15b77d9af3 ("rbd: get additional info in parent spec")
Fixes: 30ba1f020221 ("rbd: make sure we have latest osdmap on 'rbd map'")
Cc: stable@vger.kernel.org
Signed-off-by: Max Kellermann <max.kellermann@ionos.com>
---
 drivers/block/rbd.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index 96743c059d98..d287c4882ca0 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -6001,14 +6001,21 @@ static int rbd_spec_fill_names(struct rbd_device *rbd_dev)
 	rbd_assert(spec->image_id);
 	rbd_assert(spec->snap_id != CEPH_NOSNAP);
 
-	/* Get the pool name; we have to make our own copy of this */
+	/*
+	 * Get the pool name; we have to make our own copy of this
+	 * because ceph_osdc_handle_map() replaces and frees the
+	 * osdmap
+	 */
 
+	down_read(&osdc->lock);
 	pool_name = ceph_pg_pool_name_by_id(osdc->osdmap, spec->pool_id);
 	if (!pool_name) {
+		up_read(&osdc->lock);
 		rbd_warn(rbd_dev, "no pool with id %llu", spec->pool_id);
 		return -EIO;
 	}
-	pool_name = kstrdup(pool_name, GFP_KERNEL);
+	pool_name = kstrdup(pool_name, GFP_NOIO);
+	up_read(&osdc->lock);
 	if (!pool_name)
 		return -ENOMEM;
 
@@ -7100,7 +7107,9 @@ static ssize_t do_rbd_add(const char *buf, size_t count)
 	}
 
 	/* pick the pool */
+	down_read(&rbdc->client->osdc.lock);
 	rc = ceph_pg_poolid_by_name(rbdc->client->osdc.osdmap, spec->pool_name);
+	up_read(&rbdc->client->osdc.lock);
 	if (rc < 0) {
 		if (rc == -ENOENT)
 			pr_info("pool %s does not exist\n", spec->pool_name);
-- 
2.53.0

Re: [PATCH 0/9] ceph: fix mdsmap and osdmap use-after-free in unlocked readers

[Xiubo Li]

Hi Max,

Tested on a KASAN+lockdep kernel (on top of the ceph testing branch)
with sleep injection widening each window: all the UAFs fixed by
patches 1-6 and 9 reproduce on the base and are gone with the series;
patches 7-8 were exercised under osdmap churn; lockdep stays quiet,
also across fsstress with MDS failovers.

One reader is missed: send_mds_reconnect() still does

        ceph_con_open(&session->s_con, CEPH_ENTITY_TYPE_MDS, mds,
                      ceph_mdsmap_get_addr(mdsc->mdsmap, mds));

with only s_mutex held, and it is reached from mds_peer_reset() (the
path patch 5 fixes).  With the series applied:

  BUG: KASAN: slab-use-after-free in ceph_con_open+0x14f/0x230 [libceph]
  Read of size 136 ...
   send_mds_reconnect+0xa91/0x1710 [ceph]
   mds_peer_reset+0x26c/0x36b [ceph]
   process_session_reset+0x349/0x540 [libceph]
  Freed by: ceph_mdsmap_destroy <- ceph_mdsc_handle_mdsmap

Copying the address into a local while mdsc->mutex is held for the
sessions[mds] check a few lines earlier would fix it; could you add
that as a follow-up patch?

Nit for 7-9: incremental maps are applied in place, so the osdmap is
only replaced on a full map; the rbd case is really hit through
incrementals (a pool rename kfree()s pi->name).

Thanks
- Xiubo

Max Kellermann <max.kellermann@ionos.com> 于2026年11月11日周三 17:25写道：
>
> The MDS and OSD clients replace their maps and free the old ones
> under a lock: mdsc->mutex for the mdsmap and osdc->lock for the
> osdmap.  Several readers dereference these maps without that lock and
> can therefore access freed memory if a new map arrives concurrently.
> The worst is ceph_statfs(), which any user can reach and which loads
> and dereferences a pointer from the old mdsmap.
>
> Each patch takes the lock that protects the replacement around the
> reads.  __ceph_setxattr() cannot take mdsc->mutex under i_ceph_lock,
> so it uses a copy of m_max_xattr_size kept in the mds client.
>
> The same problem in ceph_compare_options() was already fixed by
> commit 19db525816db ("libceph: use RCU to protect monmap in
> ceph_compare_options()").
>
> These bugs were found by code review and have not been reproduced.
>
> Max Kellermann (9):
>   ceph: fix mdsmap use-after-free in ceph_statfs()
>   ceph: fix mdsmap use-after-free in debugfs mdsmap_show()
>   ceph: fix mdsmap use-after-free in __ceph_setxattr()
>   ceph: fix mdsmap use-after-free in send_renew_caps() and
>     renewed_caps()
>   ceph: fix mdsmap use-after-free in mds_peer_reset()
>   ceph: fix mdsmap use-after-free in ceph_get_tree() error path
>   ceph: fix osdmap use-after-free in ceph_sync_setxattr()
>   libceph: fix osdmap use-after-free in ceph_wait_for_latest_osdmap()
>   rbd: fix osdmap use-after-free in pool lookups
>
>  drivers/block/rbd.c    | 13 +++++++++++--
>  fs/ceph/debugfs.c      | 12 ++++++++++--
>  fs/ceph/mds_client.c   | 18 ++++++++++++++++--
>  fs/ceph/mds_client.h   |  7 +++++++
>  fs/ceph/super.c        | 19 ++++++++++++++-----
>  fs/ceph/xattr.c        | 13 +++++++++----
>  net/ceph/ceph_common.c |  8 +++++++-
>  7 files changed, 74 insertions(+), 16 deletions(-)
>
> --
> 2.53.0
>
>