mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v4 0/3] ceph: don't unregister an MDS session before removing its caps
@ 2026-09-04 15:03 Max Kellermann
  2026-09-04 15:03 ` [PATCH v4 1/3] ceph: fix use-after-free in check_new_map() after early session put Max Kellermann
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Max Kellermann @ 2026-09-04 15:03 UTC (permalink / raw)
  To: idryomov, amarkuze, xiubo.li, ceph-devel, linux-kernel; +Cc: Max Kellermann

handle_session() removed the session from mdsc->sessions[] at the very
top of `CEPH_SESSION_CLOSE` handling, before taking `s_mutex`.
Between session unregistration and remove_session_caps(), the MDS rank
has no registered session while the old session still owns all caps it
was granted.

Any concurrent filesystem operation may walk into that and the next
__do_request() call registers a new session for this rank.  Once it is
open and the MDS issues caps, ceph_fill_inode() calls
ceph_add_cap(), which looks caps up by rank, not
by session identity, finding old caps linked to the old session.

The list_move_tail() call then moves the cap object to the new
session, which is already a bad thing to do.  Since it doesn't
decrement `old_session->s_nr_caps`, this quickly triggers:

  kernel BUG at fs/ceph/mds_client.c:1959!
  Internal error: Oops - BUG: 00000000f2000800 [#1]  SMP
  [...]
  Workqueue: ceph-msgr ceph_con_workfn
  pstate: 20400009 (nzCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
  pc : remove_session_caps+0x2bc/0x2d8
  lr : remove_session_caps+0x74/0x2d8
  [...]
  Call trace:
   remove_session_caps+0x2bc/0x2d8 (P)
   mds_dispatch+0xf48/0x1b60
   ceph_con_process_message+0x74/0xa0
   ceph_con_v1_try_read+0x3a0/0x1510
   ceph_con_workfn+0x260/0x460
   process_one_work+0x168/0x3b8
   worker_thread+0x1bc/0x3a0
   kthread+0x118/0x1e0
   ret_from_fork+0x10/0x20

That's BUG_ON(session->s_nr_caps > 0).

I was able to reproduce this reliably by delaying the close and
starting I/O during the delay.

This patch keeps the session registered with
`CEPH_MDS_SESSION_CLOSED`.  New requests will be put on the
`s_waiting` list where they will be resumed on the new session.

Signed-off-by: Max Kellermann <max.kellermann@ionos.com>
---
v1->v2: skip CLOSED sessions in check_new_map()
v2->v3: split session pinning and stale-map guards into two preparatory
    patches; rework CLOSED-session and export-target teardown handling
v3->v4:
- replace "ceph/mds_client: pin sessions while checking a new
  MDS map" which was meanwhile superseded by commit ee611a750955
  ("ceph: fix UAF in check_new_map() on session freed during
  unlock") with one that fixes the remaining UAF bugs
- adjust ceph_mdsc_reset_workfn()
- serialize send_mds_reconnect() state transitions with session CLOSE and
  prevented reconnect failure rollback from overwriting CLOSED.

Max Kellermann (3):
  ceph: fix use-after-free in check_new_map() after early session put
  ceph: stop checking a stale MDS map after dropping mutex
  ceph: don't unregister an MDS session before removing its caps

 fs/ceph/caps.c       |   4 +
 fs/ceph/mds_client.c | 214 +++++++++++++++++++++++++++++++++++--------
 2 files changed, 179 insertions(+), 39 deletions(-)

-- 
2.47.3


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

* [PATCH v4 1/3] ceph: fix use-after-free in check_new_map() after early session put
  2026-09-04 15:03 [PATCH v4 0/3] ceph: don't unregister an MDS session before removing its caps Max Kellermann
@ 2026-09-04 15:03 ` Max Kellermann
  2026-09-05  1:52   ` Xiubo Li
  2026-09-04 15:03 ` [PATCH v4 2/3] ceph: stop checking a stale MDS map after dropping mutex Max Kellermann
  2026-09-04 15:03 ` [PATCH v4 3/3] ceph: don't unregister an MDS session before removing its caps Max Kellermann
  2 siblings, 1 reply; 7+ messages in thread
From: Max Kellermann @ 2026-09-04 15:03 UTC (permalink / raw)
  To: idryomov, amarkuze, xiubo.li, ceph-devel, linux-kernel
  Cc: Max Kellermann, stable

check_new_map() drops mdsc->mutex while it locks a session, prepares a
reconnect, or kicks flushing caps.  A concurrent teardown can call
__unregister_session() in that window and drop the sessions[]
reference. When check_new_map() reacquires mdsc->mutex, its temporary
reference may therefore be the only reference keeping the local
variable `s` alive.

Commit ee611a750955 ("ceph: fix UAF in check_new_map() on session freed
during unlock") addressed this by taking a temporary reference around
each unlock window, but it drops that reference as soon as mdsc->mutex
is reacquired, while `s` is still in use:

  ceph_get_mds_session(s);
  mutex_unlock(&mdsc->mutex);
  mutex_lock(&s->s_mutex);
  mutex_lock(&mdsc->mutex);
  ceph_put_mds_session(s);    /* may drop the last reference */
  ceph_con_close(&s->s_con);  /* use-after-free */
  mutex_unlock(&s->s_mutex);  /* use-after-free */
  s->s_state = CEPH_MDS_SESSION_RESTARTING;

If the session was unregistered during the window, the sessions[]
reference is already gone, so this ceph_put_mds_session() frees it.

Additionally, the session could be freed while its s_mutex is locked,
which trips the WARN_ON(mutex_is_locked(&s->s_mutex)) in
ceph_put_mds_session() and then unlocks freed memory.

Fix this by holding a single reference for the whole loop iteration:
look the session up with __ceph_lookup_mds_session(), which returns it
with a reference held, and release that reference on every exit from
the loop body.  This subsumes the per-window get/put pairs, so remove
them.

Fixes: ee611a750955 ("ceph: fix UAF in check_new_map() on session freed during unlock")
Cc: stable@vger.kernel.org
Signed-off-by: Max Kellermann <max.kellermann@ionos.com>
---
Note the stable maintainers: this is a fixup for ee611a750955, but the
bug has existed before; see
https://lore.kernel.org/ceph-devel/20260828174504.1247038-2-max.kellermann@ionos.com/
for a patch that applies to pre-7.2 kernel versions.
---
 fs/ceph/mds_client.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
index a091f77cedaf..d36a114747ae 100644
--- a/fs/ceph/mds_client.c
+++ b/fs/ceph/mds_client.c
@@ -5860,9 +5860,9 @@ static void check_new_map(struct ceph_mds_client *mdsc,
 	}
 
 	for (i = 0; i < oldmap->possible_max_rank && i < mdsc->max_sessions; i++) {
-		if (!mdsc->sessions[i])
+		s = __ceph_lookup_mds_session(mdsc, i);
+		if (!s)
 			continue;
-		s = mdsc->sessions[i];
 		oldstate = ceph_mdsmap_get_state(oldmap, i);
 		newstate = ceph_mdsmap_get_state(newmap, i);
 
@@ -5875,7 +5875,6 @@ static void check_new_map(struct ceph_mds_client *mdsc,
 
 		if (i >= newmap->possible_max_rank) {
 			/* force close session for stopped mds */
-			ceph_get_mds_session(s);
 			__unregister_session(mdsc, s);
 			__wake_requests(mdsc, &s->s_waiting);
 			mutex_unlock(&mdsc->mutex);
@@ -5896,15 +5895,14 @@ static void check_new_map(struct ceph_mds_client *mdsc,
 			   ceph_mdsmap_get_addr(newmap, i),
 			   sizeof(struct ceph_entity_addr))) {
 			/* just close it */
-			ceph_get_mds_session(s);
 			mutex_unlock(&mdsc->mutex);
 			mutex_lock(&s->s_mutex);
 			mutex_lock(&mdsc->mutex);
-			ceph_put_mds_session(s);
 			ceph_con_close(&s->s_con);
 			mutex_unlock(&s->s_mutex);
 			s->s_state = CEPH_MDS_SESSION_RESTARTING;
 		} else if (oldstate == newstate) {
+			ceph_put_mds_session(s);
 			continue;  /* nothing new with this mds */
 		}
 
@@ -5915,7 +5913,6 @@ static void check_new_map(struct ceph_mds_client *mdsc,
 		    newstate >= CEPH_MDS_STATE_RECONNECT) {
 			int rc;
 
-			ceph_get_mds_session(s);
 			mutex_unlock(&mdsc->mutex);
 			clear_bit(i, targets);
 			rc = send_mds_reconnect(mdsc, s);
@@ -5924,7 +5921,6 @@ static void check_new_map(struct ceph_mds_client *mdsc,
 					       "mds%d reconnect failed: %d\n",
 					       i, rc);
 			mutex_lock(&mdsc->mutex);
-			ceph_put_mds_session(s);
 		}
 
 		/*
@@ -5937,15 +5933,14 @@ static void check_new_map(struct ceph_mds_client *mdsc,
 				pr_info_client(cl, "mds%d recovery completed\n",
 					       s->s_mds);
 			kick_requests(mdsc, i);
-			ceph_get_mds_session(s);
 			mutex_unlock(&mdsc->mutex);
 			mutex_lock(&s->s_mutex);
 			mutex_lock(&mdsc->mutex);
-			ceph_put_mds_session(s);
 			ceph_kick_flushing_caps(mdsc, s);
 			mutex_unlock(&s->s_mutex);
 			wake_up_session_caps(s, RECONNECT);
 		}
+		ceph_put_mds_session(s);
 	}
 
 	/*
-- 
2.47.3


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

* [PATCH v4 2/3] ceph: stop checking a stale MDS map after dropping mutex
  2026-09-04 15:03 [PATCH v4 0/3] ceph: don't unregister an MDS session before removing its caps Max Kellermann
  2026-09-04 15:03 ` [PATCH v4 1/3] ceph: fix use-after-free in check_new_map() after early session put Max Kellermann
@ 2026-09-04 15:03 ` Max Kellermann
  2026-09-05  1:52   ` Xiubo Li
  2026-09-04 15:03 ` [PATCH v4 3/3] ceph: don't unregister an MDS session before removing its caps Max Kellermann
  2 siblings, 1 reply; 7+ messages in thread
From: Max Kellermann @ 2026-09-04 15:03 UTC (permalink / raw)
  To: idryomov, amarkuze, xiubo.li, ceph-devel, linux-kernel
  Cc: Max Kellermann, stable

check_new_map() drops `mdsc->mutex` in several slow paths.  Another
map handler can install a newer map and destroy the map being checked
before the original invocation re-locks the mutex.  Continuing to
dereference newmap after that results in a use-after-free.

Use the `mdsmap->m_epoch` field to identify stale maps.

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

diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
index d36a114747ae..86d592f06196 100644
--- a/fs/ceph/mds_client.c
+++ b/fs/ceph/mds_client.c
@@ -5844,6 +5844,7 @@ static void check_new_map(struct ceph_mds_client *mdsc,
 			  struct ceph_mdsmap *newmap,
 			  struct ceph_mdsmap *oldmap)
 {
+	u32 map_epoch = newmap->m_epoch;
 	int i, j, err;
 	int oldstate, newstate;
 	struct ceph_mds_session *s;
@@ -5887,6 +5888,8 @@ static void check_new_map(struct ceph_mds_client *mdsc,
 			ceph_put_mds_session(s);
 
 			mutex_lock(&mdsc->mutex);
+			if (mdsc->mdsmap->m_epoch != map_epoch)
+				return;
 			kick_requests(mdsc, i);
 			continue;
 		}
@@ -5898,6 +5901,11 @@ static void check_new_map(struct ceph_mds_client *mdsc,
 			mutex_unlock(&mdsc->mutex);
 			mutex_lock(&s->s_mutex);
 			mutex_lock(&mdsc->mutex);
+			if (mdsc->mdsmap->m_epoch != map_epoch) {
+				mutex_unlock(&s->s_mutex);
+				ceph_put_mds_session(s);
+				return;
+			}
 			ceph_con_close(&s->s_con);
 			mutex_unlock(&s->s_mutex);
 			s->s_state = CEPH_MDS_SESSION_RESTARTING;
@@ -5921,6 +5929,10 @@ static void check_new_map(struct ceph_mds_client *mdsc,
 					       "mds%d reconnect failed: %d\n",
 					       i, rc);
 			mutex_lock(&mdsc->mutex);
+			if (mdsc->mdsmap->m_epoch != map_epoch) {
+				ceph_put_mds_session(s);
+				return;
+			}
 		}
 
 		/*
@@ -5936,6 +5948,11 @@ static void check_new_map(struct ceph_mds_client *mdsc,
 			mutex_unlock(&mdsc->mutex);
 			mutex_lock(&s->s_mutex);
 			mutex_lock(&mdsc->mutex);
+			if (mdsc->mdsmap->m_epoch != map_epoch) {
+				mutex_unlock(&s->s_mutex);
+				ceph_put_mds_session(s);
+				return;
+			}
 			ceph_kick_flushing_caps(mdsc, s);
 			mutex_unlock(&s->s_mutex);
 			wake_up_session_caps(s, RECONNECT);
@@ -5991,6 +6008,8 @@ static void check_new_map(struct ceph_mds_client *mdsc,
 				       i, err);
 		ceph_put_mds_session(s);
 		mutex_lock(&mdsc->mutex);
+		if (mdsc->mdsmap->m_epoch != map_epoch)
+			return;
 	}
 
 	for (i = 0; i < newmap->possible_max_rank && i < mdsc->max_sessions; i++) {
-- 
2.47.3


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

* [PATCH v4 3/3] ceph: don't unregister an MDS session before removing its caps
  2026-09-04 15:03 [PATCH v4 0/3] ceph: don't unregister an MDS session before removing its caps Max Kellermann
  2026-09-04 15:03 ` [PATCH v4 1/3] ceph: fix use-after-free in check_new_map() after early session put Max Kellermann
  2026-09-04 15:03 ` [PATCH v4 2/3] ceph: stop checking a stale MDS map after dropping mutex Max Kellermann
@ 2026-09-04 15:03 ` Max Kellermann
  2026-09-05  1:56   ` Xiubo Li
  2 siblings, 1 reply; 7+ messages in thread
From: Max Kellermann @ 2026-09-04 15:03 UTC (permalink / raw)
  To: idryomov, amarkuze, xiubo.li, ceph-devel, linux-kernel
  Cc: Max Kellermann, stable

handle_session() removed the session from mdsc->sessions[] at the very
top of `CEPH_SESSION_CLOSE` handling, before taking `s_mutex`.
Between session unregistration and remove_session_caps(), the MDS rank
has no registered session while the old session still owns all caps it
was granted.

Any concurrent filesystem operation may walk into that and the next
__do_request() call registers a new session for this rank.  Once it is
open and the MDS issues caps, ceph_fill_inode() calls
ceph_add_cap(), which looks caps up by rank, not
by session identity, finding old caps linked to the old session.

The list_move_tail() call then moves the cap object to the new
session, which is already a bad thing to do.  Since it doesn't
decrement `old_session->s_nr_caps`, this quickly triggers:

  kernel BUG at fs/ceph/mds_client.c:1959!
  Internal error: Oops - BUG: 00000000f2000800 [#1]  SMP
  [...]
  Workqueue: ceph-msgr ceph_con_workfn
  pstate: 20400009 (nzCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
  pc : remove_session_caps+0x2bc/0x2d8
  lr : remove_session_caps+0x74/0x2d8
  [...]
  Call trace:
   remove_session_caps+0x2bc/0x2d8 (P)
   mds_dispatch+0xf48/0x1b60
   ceph_con_process_message+0x74/0xa0
   ceph_con_v1_try_read+0x3a0/0x1510
   ceph_con_workfn+0x260/0x460
   process_one_work+0x168/0x3b8
   worker_thread+0x1bc/0x3a0
   kthread+0x118/0x1e0
   ret_from_fork+0x10/0x20

That's BUG_ON(session->s_nr_caps > 0).

I was able to reproduce this reliably by delaying the close and
starting I/O during the delay.

This patch keeps the session registered with
`CEPH_MDS_SESSION_CLOSED`.  New requests will be put on the
`s_waiting` list where they will be resumed on the new session.

Since `CLOSE` now sets `CLOSED` before taking `s_mutex`, make
send_mds_reconnect() validate and update the session state under
`mdsc->mutex`. Preserve `CLOSED` if reconnect preparation later fails.

Fixes: 2600d2dd5085 ("ceph: drop messages on unregistered mds sessions; cleanup")
Cc: stable@vger.kernel.org
Signed-off-by: Max Kellermann <max.kellermann@ionos.com>
---
 fs/ceph/caps.c       |   4 +
 fs/ceph/mds_client.c | 182 ++++++++++++++++++++++++++++++++++++-------
 2 files changed, 156 insertions(+), 30 deletions(-)

diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c
index bcb04c6cb92c..024714d7f34e 100644
--- a/fs/ceph/caps.c
+++ b/fs/ceph/caps.c
@@ -4261,6 +4261,10 @@ static void handle_cap_export(struct inode *inode, struct ceph_mds_caps *ex,
 		}
 		new_cap = ceph_get_cap(mdsc, NULL);
 	} else {
+		if (tsession == ERR_PTR(-EAGAIN))
+			/* locks already dropped */
+			return;
+
 		WARN_ON(1);
 		tsession = NULL;
 		target = -1;
diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
index 86d592f06196..f091f0eaafc2 100644
--- a/fs/ceph/mds_client.c
+++ b/fs/ceph/mds_client.c
@@ -1783,6 +1783,30 @@ static int __open_session(struct ceph_mds_client *mdsc,
 	return 0;
 }
 
+/* Is this rank still occupied by a session being torn down? */
+static bool __mds_rank_closing(struct ceph_mds_client *mdsc, int mds)
+{
+	return mds < mdsc->max_sessions && mdsc->sessions[mds] &&
+		mdsc->sessions[mds]->s_state == CEPH_MDS_SESSION_CLOSED;
+}
+
+/*
+ * Wait until the rank is no longer occupied by a CLOSED session.
+ *
+ * The caller must hold mdsc->mutex.  The mutex is dropped while sleeping and
+ * held again on return.  Another session may occupy the rank by then.  The
+ * wait also ends when shutdown starts, so the caller must check
+ * mdsc->stopping before proceeding.
+ */
+static void wait_for_mds_rank_not_closing(struct ceph_mds_client *mdsc,
+					  int mds)
+{
+	wait_event_cmd(mdsc->session_close_wq,
+		       !__mds_rank_closing(mdsc, mds) || mdsc->stopping,
+		       mutex_unlock(&mdsc->mutex),
+		       mutex_lock(&mdsc->mutex));
+}
+
 /*
  * open sessions for any export targets for the given mds
  *
@@ -1800,6 +1824,16 @@ __open_export_target_session(struct ceph_mds_client *mdsc, int target)
 		if (IS_ERR(session))
 			return session;
 	}
+	if (session->s_state == CEPH_MDS_SESSION_CLOSED) {
+		/*
+		 * handle_session() is currently closing this session;
+		 * it stays registered until its caps are gone.  Do
+		 * not return it to our caller because we don't want
+		 * it to attach new caps to it.
+		 */
+		ceph_put_mds_session(session);
+		return ERR_PTR(-EAGAIN);
+	}
 	if (session->s_state == CEPH_MDS_SESSION_NEW ||
 	    session->s_state == CEPH_MDS_SESSION_CLOSING) {
 		ret = __open_session(mdsc, session);
@@ -1819,7 +1853,19 @@ ceph_mdsc_open_export_target_session(struct ceph_mds_client *mdsc, int target)
 	doutc(cl, "to mds%d\n", target);
 
 	mutex_lock(&mdsc->mutex);
-	session = __open_export_target_session(mdsc, target);
+	for (;;) {
+		session = __open_export_target_session(mdsc, target);
+		if (session != ERR_PTR(-EAGAIN) || mdsc->stopping)
+			break;
+
+		/*
+		 * Keep the exported cap on its old session until the
+		 * target rank is vacant.  Dropping it here can discard a
+		 * dirty auth cap; handle_session() wakes us after removing
+		 * the old target session's caps and unregistering it.
+		 */
+		wait_for_mds_rank_not_closing(mdsc, target);
+	}
 	mutex_unlock(&mdsc->mutex);
 
 	return session;
@@ -4553,8 +4599,20 @@ static void handle_session(struct ceph_mds_session *session,
 			ceph_metric_bind_session(mdsc, session);
 	}
 	if (op == CEPH_SESSION_CLOSE) {
+		/*
+		 * Pin the session for the rest of this function.  The
+		 * __unregister_session() call is deferred until after
+		 * remove_session_caps() below, or else other
+		 * processes may find caps still assigned to this
+		 * session while working with a new session object.
+		 */
 		ceph_get_mds_session(session);
-		__unregister_session(mdsc, session);
+
+		if (session->s_state == CEPH_MDS_SESSION_RECONNECTING)
+			pr_info_client(cl, "mds%d reconnect denied\n",
+				       session->s_mds);
+
+		session->s_state = CEPH_MDS_SESSION_CLOSED;
 	}
 	/* FIXME: this ttl calculation is generous */
 	session->s_ttl = jiffies + HZ*mdsc->mdsmap->m_session_autoclose;
@@ -4612,12 +4670,24 @@ static void handle_session(struct ceph_mds_session *session,
 		break;
 
 	case CEPH_SESSION_CLOSE:
-		if (session->s_state == CEPH_MDS_SESSION_RECONNECTING)
-			pr_info_client(cl, "mds%d reconnect denied\n",
-				       session->s_mds);
-		session->s_state = CEPH_MDS_SESSION_CLOSED;
 		cleanup_session_requests(mdsc, session);
 		remove_session_caps(session);
+
+		/*
+		 * Now that all caps are removed, it is safe to release
+		 * the MDS rank and allow other processes to create a
+		 * new session object.
+		 *
+		 * A concurrent ceph_mdsc_close_sessions() or
+		 * check_new_map() may have unregistered the session
+		 * already, so check __verify_registered_session()
+		 * first.
+		 */
+		mutex_lock(&mdsc->mutex);
+		if (!__verify_registered_session(mdsc, session))
+			__unregister_session(mdsc, session);
+		mutex_unlock(&mdsc->mutex);
+
 		wake = 2; /* for good measure */
 		wake_up_all(&mdsc->session_close_wq);
 		break;
@@ -5168,16 +5238,6 @@ static int send_mds_reconnect(struct ceph_mds_client *mdsc,
 	/* Serialized by s_mutex against concurrent ceph_get_deleg_ino(). */
 	xa_destroy(&session->s_delegated_inos);
 	atomic_set(&session->s_num_deleg_inos, 0);
-	if (session->s_state == CEPH_MDS_SESSION_CLOSED ||
-	    session->s_state == CEPH_MDS_SESSION_REJECTED) {
-		pr_info_client(cl, "mds%d skipping reconnect, session %s\n",
-			       mds,
-			       ceph_session_state_name(session->s_state));
-		mutex_unlock(&session->s_mutex);
-		ceph_msg_put(reply);
-		err = -ESTALE;
-		goto fail_return;
-	}
 
 	/* s_mutex -> mdsc->mutex matches cleanup_session_requests() order. */
 	mutex_lock(&mdsc->mutex);
@@ -5191,12 +5251,23 @@ static int send_mds_reconnect(struct ceph_mds_client *mdsc,
 		err = -ENOENT;
 		goto fail_return;
 	}
-	mutex_unlock(&mdsc->mutex);
+	if (session->s_state == CEPH_MDS_SESSION_CLOSED ||
+	    session->s_state == CEPH_MDS_SESSION_REJECTED) {
+		mutex_unlock(&mdsc->mutex);
+		pr_info_client(cl, "mds%d skipping reconnect, session %s\n",
+			       mds,
+			       ceph_session_state_name(session->s_state));
+		mutex_unlock(&session->s_mutex);
+		ceph_msg_put(reply);
+		err = -ESTALE;
+		goto fail_return;
+	}
 
 	pr_info_client(cl, "mds%d reconnect start\n", mds);
 	old_state = session->s_state;
 	session->s_state = CEPH_MDS_SESSION_RECONNECTING;
 	session->s_seq = 0;
+	mutex_unlock(&mdsc->mutex);
 
 	doutc(cl, "session %p state %s\n", session,
 	      ceph_session_state_name(session->s_state));
@@ -5338,7 +5409,10 @@ static int send_mds_reconnect(struct ceph_mds_client *mdsc,
 	 * (check_new_map) can retry.  Without this, a transient build
 	 * failure strands the session in RECONNECTING indefinitely.
 	 */
-	session->s_state = old_state;
+	mutex_lock(&mdsc->mutex);
+	if (session->s_state == CEPH_MDS_SESSION_RECONNECTING)
+		session->s_state = old_state;
+	mutex_unlock(&mdsc->mutex);
 	mutex_unlock(&session->s_mutex);
 fail_nomsg:
 	ceph_pagelist_release(recon_state.pagelist);
@@ -5746,23 +5820,29 @@ static void ceph_mdsc_reset_workfn(struct work_struct *work)
 			continue;
 		}
 		sessions[i]->s_state = CEPH_MDS_SESSION_CLOSED;
-		__unregister_session(mdsc, sessions[i]);
-		__wake_requests(mdsc, &sessions[i]->s_waiting);
 		mutex_unlock(&mdsc->mutex);
 
 		mutex_lock(&sessions[i]->s_mutex);
 		cleanup_session_requests(mdsc, sessions[i]);
 		remove_session_caps(sessions[i]);
+
+		/* Keep the rank occupied until all old-session caps are gone. */
+		mutex_lock(&mdsc->mutex);
+		if (!__verify_registered_session(mdsc, sessions[i]))
+			__unregister_session(mdsc, sessions[i]);
+		mutex_unlock(&mdsc->mutex);
+
 		mutex_unlock(&sessions[i]->s_mutex);
 
 		wake_up_all(&mdsc->session_close_wq);
 
-		ceph_put_mds_session(sessions[i]);
-
 		mutex_lock(&mdsc->mutex);
+		__wake_requests(mdsc, &sessions[i]->s_waiting);
 		kick_requests(mdsc, mds);
 		mutex_unlock(&mdsc->mutex);
 
+		ceph_put_mds_session(sessions[i]);
+
 		torn_down++;
 		pr_info_client(cl, "mds%d session reset complete\n", mds);
 	}
@@ -5864,6 +5944,12 @@ static void check_new_map(struct ceph_mds_client *mdsc,
 		s = __ceph_lookup_mds_session(mdsc, i);
 		if (!s)
 			continue;
+
+		if (s->s_state == CEPH_MDS_SESSION_CLOSED) {
+			ceph_put_mds_session(s);
+			continue;
+		}
+
 		oldstate = ceph_mdsmap_get_state(oldmap, i);
 		newstate = ceph_mdsmap_get_state(newmap, i);
 
@@ -5876,18 +5962,24 @@ static void check_new_map(struct ceph_mds_client *mdsc,
 
 		if (i >= newmap->possible_max_rank) {
 			/* force close session for stopped mds */
-			__unregister_session(mdsc, s);
-			__wake_requests(mdsc, &s->s_waiting);
+			s->s_state = CEPH_MDS_SESSION_CLOSED;
 			mutex_unlock(&mdsc->mutex);
 
 			mutex_lock(&s->s_mutex);
 			cleanup_session_requests(mdsc, s);
 			remove_session_caps(s);
+
+			mutex_lock(&mdsc->mutex);
+			if (!__verify_registered_session(mdsc, s))
+				__unregister_session(mdsc, s);
+			mutex_unlock(&mdsc->mutex);
 			mutex_unlock(&s->s_mutex);
 
-			ceph_put_mds_session(s);
+			wake_up_all(&mdsc->session_close_wq);
 
 			mutex_lock(&mdsc->mutex);
+			__wake_requests(mdsc, &s->s_waiting);
+			ceph_put_mds_session(s);
 			if (mdsc->mdsmap->m_epoch != map_epoch)
 				return;
 			kick_requests(mdsc, i);
@@ -5906,6 +5998,16 @@ static void check_new_map(struct ceph_mds_client *mdsc,
 				ceph_put_mds_session(s);
 				return;
 			}
+			if (s->s_state == CEPH_MDS_SESSION_CLOSED) {
+				/*
+				 * handle_session() set state=CLOSED
+				 * in the mutex gap above and is tearing it
+				 * down
+				 */
+				mutex_unlock(&s->s_mutex);
+				ceph_put_mds_session(s);
+				continue;
+			}
 			ceph_con_close(&s->s_con);
 			mutex_unlock(&s->s_mutex);
 			s->s_state = CEPH_MDS_SESSION_RESTARTING;
@@ -5964,6 +6066,9 @@ static void check_new_map(struct ceph_mds_client *mdsc,
 	 * Only open and reconnect sessions that don't exist yet.
 	 */
 	for (i = 0; i < newmap->possible_max_rank; i++) {
+		if (mdsc->stopping)
+			return;
+
 		/*
 		 * In case the import MDS is crashed just after
 		 * the EImportStart journal is flushed, so when
@@ -5989,6 +6094,19 @@ static void check_new_map(struct ceph_mds_client *mdsc,
 		 * reconnection request in up:reconnect state.
 		 */
 		s = __ceph_lookup_mds_session(mdsc, i);
+		if (s && s->s_state == CEPH_MDS_SESSION_CLOSED) {
+			/*
+			 * Wait for handle_session() to remove the caps and
+			 * unregister this session, so the reconnect below
+			 * uses a fresh session on the now vacant rank
+			 */
+			ceph_put_mds_session(s);
+			wait_for_mds_rank_not_closing(mdsc, i);
+			if (mdsc->stopping ||
+			    mdsc->mdsmap->m_epoch != map_epoch)
+				return;
+			s = NULL;
+		}
 		if (likely(!s)) {
 			s = __open_export_target_session(mdsc, i);
 			if (IS_ERR(s)) {
@@ -7149,9 +7267,7 @@ static void mds_peer_reset(struct ceph_connection *con)
 	 * Snapshot session state with READ_ONCE, then revalidate under
 	 * mdsc->mutex before acting.  The subsequent mdsc->mutex
 	 * section rechecks s_state to catch concurrent transitions, so
-	 * the lockless snapshot here is safe.  s->s_mutex is taken
-	 * separately for cleanup after unregistration, which avoids
-	 * introducing a new s->s_mutex + mdsc->mutex nesting.
+	 * the lockless snapshot here is safe.
 	 */
 	session_state = READ_ONCE(s->s_state);
 
@@ -7176,18 +7292,24 @@ static void mds_peer_reset(struct ceph_connection *con)
 
 		ceph_get_mds_session(s);
 		s->s_state = CEPH_MDS_SESSION_CLOSED;
-		__unregister_session(mdsc, s);
-		__wake_requests(mdsc, &s->s_waiting);
 		mutex_unlock(&mdsc->mutex);
 
 		mutex_lock(&s->s_mutex);
 		cleanup_session_requests(mdsc, s);
 		remove_session_caps(s);
+
+		/* Keep the rank occupied until all old-session caps are gone. */
+		mutex_lock(&mdsc->mutex);
+		if (!__verify_registered_session(mdsc, s))
+			__unregister_session(mdsc, s);
+		mutex_unlock(&mdsc->mutex);
+
 		mutex_unlock(&s->s_mutex);
 
 		wake_up_all(&mdsc->session_close_wq);
 
 		mutex_lock(&mdsc->mutex);
+		__wake_requests(mdsc, &s->s_waiting);
 		kick_requests(mdsc, s->s_mds);
 		mutex_unlock(&mdsc->mutex);
 
-- 
2.47.3


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

* Re: [PATCH v4 1/3] ceph: fix use-after-free in check_new_map() after early session put
  2026-09-04 15:03 ` [PATCH v4 1/3] ceph: fix use-after-free in check_new_map() after early session put Max Kellermann
@ 2026-09-05  1:52   ` Xiubo Li
  0 siblings, 0 replies; 7+ messages in thread
From: Xiubo Li @ 2026-09-05  1:52 UTC (permalink / raw)
  To: Max Kellermann; +Cc: idryomov, amarkuze, ceph-devel, linux-kernel, stable

Good catch Max.

LGTM.

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

On Fri, 4 Sept 2026 at 08:03, Max Kellermann <max.kellermann@ionos.com> wrote:
>
> check_new_map() drops mdsc->mutex while it locks a session, prepares a
> reconnect, or kicks flushing caps.  A concurrent teardown can call
> __unregister_session() in that window and drop the sessions[]
> reference. When check_new_map() reacquires mdsc->mutex, its temporary
> reference may therefore be the only reference keeping the local
> variable `s` alive.
>
> Commit ee611a750955 ("ceph: fix UAF in check_new_map() on session freed
> during unlock") addressed this by taking a temporary reference around
> each unlock window, but it drops that reference as soon as mdsc->mutex
> is reacquired, while `s` is still in use:
>
>   ceph_get_mds_session(s);
>   mutex_unlock(&mdsc->mutex);
>   mutex_lock(&s->s_mutex);
>   mutex_lock(&mdsc->mutex);
>   ceph_put_mds_session(s);    /* may drop the last reference */
>   ceph_con_close(&s->s_con);  /* use-after-free */
>   mutex_unlock(&s->s_mutex);  /* use-after-free */
>   s->s_state = CEPH_MDS_SESSION_RESTARTING;
>
> If the session was unregistered during the window, the sessions[]
> reference is already gone, so this ceph_put_mds_session() frees it.
>
> Additionally, the session could be freed while its s_mutex is locked,
> which trips the WARN_ON(mutex_is_locked(&s->s_mutex)) in
> ceph_put_mds_session() and then unlocks freed memory.
>
> Fix this by holding a single reference for the whole loop iteration:
> look the session up with __ceph_lookup_mds_session(), which returns it
> with a reference held, and release that reference on every exit from
> the loop body.  This subsumes the per-window get/put pairs, so remove
> them.
>
> Fixes: ee611a750955 ("ceph: fix UAF in check_new_map() on session freed during unlock")
> Cc: stable@vger.kernel.org
> Signed-off-by: Max Kellermann <max.kellermann@ionos.com>
> ---
> Note the stable maintainers: this is a fixup for ee611a750955, but the
> bug has existed before; see
> https://lore.kernel.org/ceph-devel/20260828174504.1247038-2-max.kellermann@ionos.com/
> for a patch that applies to pre-7.2 kernel versions.
> ---
>  fs/ceph/mds_client.c | 13 ++++---------
>  1 file changed, 4 insertions(+), 9 deletions(-)
>
> diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
> index a091f77cedaf..d36a114747ae 100644
> --- a/fs/ceph/mds_client.c
> +++ b/fs/ceph/mds_client.c
> @@ -5860,9 +5860,9 @@ static void check_new_map(struct ceph_mds_client *mdsc,
>         }
>
>         for (i = 0; i < oldmap->possible_max_rank && i < mdsc->max_sessions; i++) {
> -               if (!mdsc->sessions[i])
> +               s = __ceph_lookup_mds_session(mdsc, i);
> +               if (!s)
>                         continue;
> -               s = mdsc->sessions[i];
>                 oldstate = ceph_mdsmap_get_state(oldmap, i);
>                 newstate = ceph_mdsmap_get_state(newmap, i);
>
> @@ -5875,7 +5875,6 @@ static void check_new_map(struct ceph_mds_client *mdsc,
>
>                 if (i >= newmap->possible_max_rank) {
>                         /* force close session for stopped mds */
> -                       ceph_get_mds_session(s);
>                         __unregister_session(mdsc, s);
>                         __wake_requests(mdsc, &s->s_waiting);
>                         mutex_unlock(&mdsc->mutex);
> @@ -5896,15 +5895,14 @@ static void check_new_map(struct ceph_mds_client *mdsc,
>                            ceph_mdsmap_get_addr(newmap, i),
>                            sizeof(struct ceph_entity_addr))) {
>                         /* just close it */
> -                       ceph_get_mds_session(s);
>                         mutex_unlock(&mdsc->mutex);
>                         mutex_lock(&s->s_mutex);
>                         mutex_lock(&mdsc->mutex);
> -                       ceph_put_mds_session(s);
>                         ceph_con_close(&s->s_con);
>                         mutex_unlock(&s->s_mutex);
>                         s->s_state = CEPH_MDS_SESSION_RESTARTING;
>                 } else if (oldstate == newstate) {
> +                       ceph_put_mds_session(s);
>                         continue;  /* nothing new with this mds */
>                 }
>
> @@ -5915,7 +5913,6 @@ static void check_new_map(struct ceph_mds_client *mdsc,
>                     newstate >= CEPH_MDS_STATE_RECONNECT) {
>                         int rc;
>
> -                       ceph_get_mds_session(s);
>                         mutex_unlock(&mdsc->mutex);
>                         clear_bit(i, targets);
>                         rc = send_mds_reconnect(mdsc, s);
> @@ -5924,7 +5921,6 @@ static void check_new_map(struct ceph_mds_client *mdsc,
>                                                "mds%d reconnect failed: %d\n",
>                                                i, rc);
>                         mutex_lock(&mdsc->mutex);
> -                       ceph_put_mds_session(s);
>                 }
>
>                 /*
> @@ -5937,15 +5933,14 @@ static void check_new_map(struct ceph_mds_client *mdsc,
>                                 pr_info_client(cl, "mds%d recovery completed\n",
>                                                s->s_mds);
>                         kick_requests(mdsc, i);
> -                       ceph_get_mds_session(s);
>                         mutex_unlock(&mdsc->mutex);
>                         mutex_lock(&s->s_mutex);
>                         mutex_lock(&mdsc->mutex);
> -                       ceph_put_mds_session(s);
>                         ceph_kick_flushing_caps(mdsc, s);
>                         mutex_unlock(&s->s_mutex);
>                         wake_up_session_caps(s, RECONNECT);
>                 }
> +               ceph_put_mds_session(s);
>         }
>
>         /*
> --
> 2.47.3
>

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

* Re: [PATCH v4 2/3] ceph: stop checking a stale MDS map after dropping mutex
  2026-09-04 15:03 ` [PATCH v4 2/3] ceph: stop checking a stale MDS map after dropping mutex Max Kellermann
@ 2026-09-05  1:52   ` Xiubo Li
  0 siblings, 0 replies; 7+ messages in thread
From: Xiubo Li @ 2026-09-05  1:52 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 08:03, Max Kellermann <max.kellermann@ionos.com> wrote:
>
> check_new_map() drops `mdsc->mutex` in several slow paths.  Another
> map handler can install a newer map and destroy the map being checked
> before the original invocation re-locks the mutex.  Continuing to
> dereference newmap after that results in a use-after-free.
>
> Use the `mdsmap->m_epoch` field to identify stale maps.
>
> Fixes: 2f2dc053404f ("ceph: MDS client")
> Cc: stable@vger.kernel.org
> Signed-off-by: Max Kellermann <max.kellermann@ionos.com>
> ---
>  fs/ceph/mds_client.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
>
> diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
> index d36a114747ae..86d592f06196 100644
> --- a/fs/ceph/mds_client.c
> +++ b/fs/ceph/mds_client.c
> @@ -5844,6 +5844,7 @@ static void check_new_map(struct ceph_mds_client *mdsc,
>                           struct ceph_mdsmap *newmap,
>                           struct ceph_mdsmap *oldmap)
>  {
> +       u32 map_epoch = newmap->m_epoch;
>         int i, j, err;
>         int oldstate, newstate;
>         struct ceph_mds_session *s;
> @@ -5887,6 +5888,8 @@ static void check_new_map(struct ceph_mds_client *mdsc,
>                         ceph_put_mds_session(s);
>
>                         mutex_lock(&mdsc->mutex);
> +                       if (mdsc->mdsmap->m_epoch != map_epoch)
> +                               return;
>                         kick_requests(mdsc, i);
>                         continue;
>                 }
> @@ -5898,6 +5901,11 @@ static void check_new_map(struct ceph_mds_client *mdsc,
>                         mutex_unlock(&mdsc->mutex);
>                         mutex_lock(&s->s_mutex);
>                         mutex_lock(&mdsc->mutex);
> +                       if (mdsc->mdsmap->m_epoch != map_epoch) {
> +                               mutex_unlock(&s->s_mutex);
> +                               ceph_put_mds_session(s);
> +                               return;
> +                       }
>                         ceph_con_close(&s->s_con);
>                         mutex_unlock(&s->s_mutex);
>                         s->s_state = CEPH_MDS_SESSION_RESTARTING;
> @@ -5921,6 +5929,10 @@ static void check_new_map(struct ceph_mds_client *mdsc,
>                                                "mds%d reconnect failed: %d\n",
>                                                i, rc);
>                         mutex_lock(&mdsc->mutex);
> +                       if (mdsc->mdsmap->m_epoch != map_epoch) {
> +                               ceph_put_mds_session(s);
> +                               return;
> +                       }
>                 }
>
>                 /*
> @@ -5936,6 +5948,11 @@ static void check_new_map(struct ceph_mds_client *mdsc,
>                         mutex_unlock(&mdsc->mutex);
>                         mutex_lock(&s->s_mutex);
>                         mutex_lock(&mdsc->mutex);
> +                       if (mdsc->mdsmap->m_epoch != map_epoch) {
> +                               mutex_unlock(&s->s_mutex);
> +                               ceph_put_mds_session(s);
> +                               return;
> +                       }
>                         ceph_kick_flushing_caps(mdsc, s);
>                         mutex_unlock(&s->s_mutex);
>                         wake_up_session_caps(s, RECONNECT);
> @@ -5991,6 +6008,8 @@ static void check_new_map(struct ceph_mds_client *mdsc,
>                                        i, err);
>                 ceph_put_mds_session(s);
>                 mutex_lock(&mdsc->mutex);
> +               if (mdsc->mdsmap->m_epoch != map_epoch)
> +                       return;
>         }
>
>         for (i = 0; i < newmap->possible_max_rank && i < mdsc->max_sessions; i++) {
> --
> 2.47.3
>

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

* Re: [PATCH v4 3/3] ceph: don't unregister an MDS session before removing its caps
  2026-09-04 15:03 ` [PATCH v4 3/3] ceph: don't unregister an MDS session before removing its caps Max Kellermann
@ 2026-09-05  1:56   ` Xiubo Li
  0 siblings, 0 replies; 7+ messages in thread
From: Xiubo Li @ 2026-09-05  1:56 UTC (permalink / raw)
  To: Max Kellermann; +Cc: idryomov, amarkuze, ceph-devel, linux-kernel, stable

Hi Max,

The fix is correct.

Just one remaining teardown path still unregisters the session before
removing its caps: the fallback loop in ceph_mdsc_close_sessions():

mutex_lock(&mdsc->mutex);
for (i = 0; i < mdsc->max_sessions; i++) {
        if (mdsc->sessions[i]) {
                session = ceph_get_mds_session(mdsc->sessions[i]);
                __unregister_session(mdsc, session);
                mutex_unlock(&mdsc->mutex);
                mutex_lock(&session->s_mutex);
                remove_session_caps(session);
                mutex_unlock(&session->s_mutex);
                ...

The rest of this series consistently changes the ordering to keep the
rank occupied until remove_session_caps() has completed. Is the
ordering in ceph_mdsc_close_sessions() intentionally excluded?

This path is not limited to shutdown: it is also reached from the
corrupt snap-trace handling in ceph_handle_caps(), ceph_handle_reply()
and handle_snap(). In those cases mdsc->stopping may be false and the
mount need not be in CEPH_MOUNT_FENCE_IO, so a concurrent
__do_request() can register a new session for the rank in the window
between __unregister_session() and remove_session_caps().

That appears to be the same race this series is intended to eliminate.

Unless there is an invariant preventing a new session from being
created in this path, I think this teardown should use the same CLOSED
-> remove caps -> verify/unregister ordering, and wake
mdsc->session_close_wq after unregistering so that
wait_for_mds_rank_not_closing() waiters can proceed.

Thanks
- Xiubo

On Fri, 4 Sept 2026 at 08:03, Max Kellermann <max.kellermann@ionos.com> wrote:
>
> handle_session() removed the session from mdsc->sessions[] at the very
> top of `CEPH_SESSION_CLOSE` handling, before taking `s_mutex`.
> Between session unregistration and remove_session_caps(), the MDS rank
> has no registered session while the old session still owns all caps it
> was granted.
>
> Any concurrent filesystem operation may walk into that and the next
> __do_request() call registers a new session for this rank.  Once it is
> open and the MDS issues caps, ceph_fill_inode() calls
> ceph_add_cap(), which looks caps up by rank, not
> by session identity, finding old caps linked to the old session.
>
> The list_move_tail() call then moves the cap object to the new
> session, which is already a bad thing to do.  Since it doesn't
> decrement `old_session->s_nr_caps`, this quickly triggers:
>
>   kernel BUG at fs/ceph/mds_client.c:1959!
>   Internal error: Oops - BUG: 00000000f2000800 [#1]  SMP
>   [...]
>   Workqueue: ceph-msgr ceph_con_workfn
>   pstate: 20400009 (nzCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
>   pc : remove_session_caps+0x2bc/0x2d8
>   lr : remove_session_caps+0x74/0x2d8
>   [...]
>   Call trace:
>    remove_session_caps+0x2bc/0x2d8 (P)
>    mds_dispatch+0xf48/0x1b60
>    ceph_con_process_message+0x74/0xa0
>    ceph_con_v1_try_read+0x3a0/0x1510
>    ceph_con_workfn+0x260/0x460
>    process_one_work+0x168/0x3b8
>    worker_thread+0x1bc/0x3a0
>    kthread+0x118/0x1e0
>    ret_from_fork+0x10/0x20
>
> That's BUG_ON(session->s_nr_caps > 0).
>
> I was able to reproduce this reliably by delaying the close and
> starting I/O during the delay.
>
> This patch keeps the session registered with
> `CEPH_MDS_SESSION_CLOSED`.  New requests will be put on the
> `s_waiting` list where they will be resumed on the new session.
>
> Since `CLOSE` now sets `CLOSED` before taking `s_mutex`, make
> send_mds_reconnect() validate and update the session state under
> `mdsc->mutex`. Preserve `CLOSED` if reconnect preparation later fails.
>
> Fixes: 2600d2dd5085 ("ceph: drop messages on unregistered mds sessions; cleanup")
> Cc: stable@vger.kernel.org
> Signed-off-by: Max Kellermann <max.kellermann@ionos.com>
> ---
>  fs/ceph/caps.c       |   4 +
>  fs/ceph/mds_client.c | 182 ++++++++++++++++++++++++++++++++++++-------
>  2 files changed, 156 insertions(+), 30 deletions(-)
>
> diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c
> index bcb04c6cb92c..024714d7f34e 100644
> --- a/fs/ceph/caps.c
> +++ b/fs/ceph/caps.c
> @@ -4261,6 +4261,10 @@ static void handle_cap_export(struct inode *inode, struct ceph_mds_caps *ex,
>                 }
>                 new_cap = ceph_get_cap(mdsc, NULL);
>         } else {
> +               if (tsession == ERR_PTR(-EAGAIN))
> +                       /* locks already dropped */
> +                       return;
> +
>                 WARN_ON(1);
>                 tsession = NULL;
>                 target = -1;
> diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
> index 86d592f06196..f091f0eaafc2 100644
> --- a/fs/ceph/mds_client.c
> +++ b/fs/ceph/mds_client.c
> @@ -1783,6 +1783,30 @@ static int __open_session(struct ceph_mds_client *mdsc,
>         return 0;
>  }
>
> +/* Is this rank still occupied by a session being torn down? */
> +static bool __mds_rank_closing(struct ceph_mds_client *mdsc, int mds)
> +{
> +       return mds < mdsc->max_sessions && mdsc->sessions[mds] &&
> +               mdsc->sessions[mds]->s_state == CEPH_MDS_SESSION_CLOSED;
> +}
> +
> +/*
> + * Wait until the rank is no longer occupied by a CLOSED session.
> + *
> + * The caller must hold mdsc->mutex.  The mutex is dropped while sleeping and
> + * held again on return.  Another session may occupy the rank by then.  The
> + * wait also ends when shutdown starts, so the caller must check
> + * mdsc->stopping before proceeding.
> + */
> +static void wait_for_mds_rank_not_closing(struct ceph_mds_client *mdsc,
> +                                         int mds)
> +{
> +       wait_event_cmd(mdsc->session_close_wq,
> +                      !__mds_rank_closing(mdsc, mds) || mdsc->stopping,
> +                      mutex_unlock(&mdsc->mutex),
> +                      mutex_lock(&mdsc->mutex));
> +}
> +
>  /*
>   * open sessions for any export targets for the given mds
>   *
> @@ -1800,6 +1824,16 @@ __open_export_target_session(struct ceph_mds_client *mdsc, int target)
>                 if (IS_ERR(session))
>                         return session;
>         }
> +       if (session->s_state == CEPH_MDS_SESSION_CLOSED) {
> +               /*
> +                * handle_session() is currently closing this session;
> +                * it stays registered until its caps are gone.  Do
> +                * not return it to our caller because we don't want
> +                * it to attach new caps to it.
> +                */
> +               ceph_put_mds_session(session);
> +               return ERR_PTR(-EAGAIN);
> +       }
>         if (session->s_state == CEPH_MDS_SESSION_NEW ||
>             session->s_state == CEPH_MDS_SESSION_CLOSING) {
>                 ret = __open_session(mdsc, session);
> @@ -1819,7 +1853,19 @@ ceph_mdsc_open_export_target_session(struct ceph_mds_client *mdsc, int target)
>         doutc(cl, "to mds%d\n", target);
>
>         mutex_lock(&mdsc->mutex);
> -       session = __open_export_target_session(mdsc, target);
> +       for (;;) {
> +               session = __open_export_target_session(mdsc, target);
> +               if (session != ERR_PTR(-EAGAIN) || mdsc->stopping)
> +                       break;
> +
> +               /*
> +                * Keep the exported cap on its old session until the
> +                * target rank is vacant.  Dropping it here can discard a
> +                * dirty auth cap; handle_session() wakes us after removing
> +                * the old target session's caps and unregistering it.
> +                */
> +               wait_for_mds_rank_not_closing(mdsc, target);
> +       }
>         mutex_unlock(&mdsc->mutex);
>
>         return session;
> @@ -4553,8 +4599,20 @@ static void handle_session(struct ceph_mds_session *session,
>                         ceph_metric_bind_session(mdsc, session);
>         }
>         if (op == CEPH_SESSION_CLOSE) {
> +               /*
> +                * Pin the session for the rest of this function.  The
> +                * __unregister_session() call is deferred until after
> +                * remove_session_caps() below, or else other
> +                * processes may find caps still assigned to this
> +                * session while working with a new session object.
> +                */
>                 ceph_get_mds_session(session);
> -               __unregister_session(mdsc, session);
> +
> +               if (session->s_state == CEPH_MDS_SESSION_RECONNECTING)
> +                       pr_info_client(cl, "mds%d reconnect denied\n",
> +                                      session->s_mds);
> +
> +               session->s_state = CEPH_MDS_SESSION_CLOSED;
>         }
>         /* FIXME: this ttl calculation is generous */
>         session->s_ttl = jiffies + HZ*mdsc->mdsmap->m_session_autoclose;
> @@ -4612,12 +4670,24 @@ static void handle_session(struct ceph_mds_session *session,
>                 break;
>
>         case CEPH_SESSION_CLOSE:
> -               if (session->s_state == CEPH_MDS_SESSION_RECONNECTING)
> -                       pr_info_client(cl, "mds%d reconnect denied\n",
> -                                      session->s_mds);
> -               session->s_state = CEPH_MDS_SESSION_CLOSED;
>                 cleanup_session_requests(mdsc, session);
>                 remove_session_caps(session);
> +
> +               /*
> +                * Now that all caps are removed, it is safe to release
> +                * the MDS rank and allow other processes to create a
> +                * new session object.
> +                *
> +                * A concurrent ceph_mdsc_close_sessions() or
> +                * check_new_map() may have unregistered the session
> +                * already, so check __verify_registered_session()
> +                * first.
> +                */
> +               mutex_lock(&mdsc->mutex);
> +               if (!__verify_registered_session(mdsc, session))
> +                       __unregister_session(mdsc, session);
> +               mutex_unlock(&mdsc->mutex);
> +
>                 wake = 2; /* for good measure */
>                 wake_up_all(&mdsc->session_close_wq);
>                 break;
> @@ -5168,16 +5238,6 @@ static int send_mds_reconnect(struct ceph_mds_client *mdsc,
>         /* Serialized by s_mutex against concurrent ceph_get_deleg_ino(). */
>         xa_destroy(&session->s_delegated_inos);
>         atomic_set(&session->s_num_deleg_inos, 0);
> -       if (session->s_state == CEPH_MDS_SESSION_CLOSED ||
> -           session->s_state == CEPH_MDS_SESSION_REJECTED) {
> -               pr_info_client(cl, "mds%d skipping reconnect, session %s\n",
> -                              mds,
> -                              ceph_session_state_name(session->s_state));
> -               mutex_unlock(&session->s_mutex);
> -               ceph_msg_put(reply);
> -               err = -ESTALE;
> -               goto fail_return;
> -       }
>
>         /* s_mutex -> mdsc->mutex matches cleanup_session_requests() order. */
>         mutex_lock(&mdsc->mutex);
> @@ -5191,12 +5251,23 @@ static int send_mds_reconnect(struct ceph_mds_client *mdsc,
>                 err = -ENOENT;
>                 goto fail_return;
>         }
> -       mutex_unlock(&mdsc->mutex);
> +       if (session->s_state == CEPH_MDS_SESSION_CLOSED ||
> +           session->s_state == CEPH_MDS_SESSION_REJECTED) {
> +               mutex_unlock(&mdsc->mutex);
> +               pr_info_client(cl, "mds%d skipping reconnect, session %s\n",
> +                              mds,
> +                              ceph_session_state_name(session->s_state));
> +               mutex_unlock(&session->s_mutex);
> +               ceph_msg_put(reply);
> +               err = -ESTALE;
> +               goto fail_return;
> +       }
>
>         pr_info_client(cl, "mds%d reconnect start\n", mds);
>         old_state = session->s_state;
>         session->s_state = CEPH_MDS_SESSION_RECONNECTING;
>         session->s_seq = 0;
> +       mutex_unlock(&mdsc->mutex);
>
>         doutc(cl, "session %p state %s\n", session,
>               ceph_session_state_name(session->s_state));
> @@ -5338,7 +5409,10 @@ static int send_mds_reconnect(struct ceph_mds_client *mdsc,
>          * (check_new_map) can retry.  Without this, a transient build
>          * failure strands the session in RECONNECTING indefinitely.
>          */
> -       session->s_state = old_state;
> +       mutex_lock(&mdsc->mutex);
> +       if (session->s_state == CEPH_MDS_SESSION_RECONNECTING)
> +               session->s_state = old_state;
> +       mutex_unlock(&mdsc->mutex);
>         mutex_unlock(&session->s_mutex);
>  fail_nomsg:
>         ceph_pagelist_release(recon_state.pagelist);
> @@ -5746,23 +5820,29 @@ static void ceph_mdsc_reset_workfn(struct work_struct *work)
>                         continue;
>                 }
>                 sessions[i]->s_state = CEPH_MDS_SESSION_CLOSED;
> -               __unregister_session(mdsc, sessions[i]);
> -               __wake_requests(mdsc, &sessions[i]->s_waiting);
>                 mutex_unlock(&mdsc->mutex);
>
>                 mutex_lock(&sessions[i]->s_mutex);
>                 cleanup_session_requests(mdsc, sessions[i]);
>                 remove_session_caps(sessions[i]);
> +
> +               /* Keep the rank occupied until all old-session caps are gone. */
> +               mutex_lock(&mdsc->mutex);
> +               if (!__verify_registered_session(mdsc, sessions[i]))
> +                       __unregister_session(mdsc, sessions[i]);
> +               mutex_unlock(&mdsc->mutex);
> +
>                 mutex_unlock(&sessions[i]->s_mutex);
>
>                 wake_up_all(&mdsc->session_close_wq);
>
> -               ceph_put_mds_session(sessions[i]);
> -
>                 mutex_lock(&mdsc->mutex);
> +               __wake_requests(mdsc, &sessions[i]->s_waiting);
>                 kick_requests(mdsc, mds);
>                 mutex_unlock(&mdsc->mutex);
>
> +               ceph_put_mds_session(sessions[i]);
> +
>                 torn_down++;
>                 pr_info_client(cl, "mds%d session reset complete\n", mds);
>         }
> @@ -5864,6 +5944,12 @@ static void check_new_map(struct ceph_mds_client *mdsc,
>                 s = __ceph_lookup_mds_session(mdsc, i);
>                 if (!s)
>                         continue;
> +
> +               if (s->s_state == CEPH_MDS_SESSION_CLOSED) {
> +                       ceph_put_mds_session(s);
> +                       continue;
> +               }
> +
>                 oldstate = ceph_mdsmap_get_state(oldmap, i);
>                 newstate = ceph_mdsmap_get_state(newmap, i);
>
> @@ -5876,18 +5962,24 @@ static void check_new_map(struct ceph_mds_client *mdsc,
>
>                 if (i >= newmap->possible_max_rank) {
>                         /* force close session for stopped mds */
> -                       __unregister_session(mdsc, s);
> -                       __wake_requests(mdsc, &s->s_waiting);
> +                       s->s_state = CEPH_MDS_SESSION_CLOSED;
>                         mutex_unlock(&mdsc->mutex);
>
>                         mutex_lock(&s->s_mutex);
>                         cleanup_session_requests(mdsc, s);
>                         remove_session_caps(s);
> +
> +                       mutex_lock(&mdsc->mutex);
> +                       if (!__verify_registered_session(mdsc, s))
> +                               __unregister_session(mdsc, s);
> +                       mutex_unlock(&mdsc->mutex);
>                         mutex_unlock(&s->s_mutex);
>
> -                       ceph_put_mds_session(s);
> +                       wake_up_all(&mdsc->session_close_wq);
>
>                         mutex_lock(&mdsc->mutex);
> +                       __wake_requests(mdsc, &s->s_waiting);
> +                       ceph_put_mds_session(s);
>                         if (mdsc->mdsmap->m_epoch != map_epoch)
>                                 return;
>                         kick_requests(mdsc, i);
> @@ -5906,6 +5998,16 @@ static void check_new_map(struct ceph_mds_client *mdsc,
>                                 ceph_put_mds_session(s);
>                                 return;
>                         }
> +                       if (s->s_state == CEPH_MDS_SESSION_CLOSED) {
> +                               /*
> +                                * handle_session() set state=CLOSED
> +                                * in the mutex gap above and is tearing it
> +                                * down
> +                                */
> +                               mutex_unlock(&s->s_mutex);
> +                               ceph_put_mds_session(s);
> +                               continue;
> +                       }
>                         ceph_con_close(&s->s_con);
>                         mutex_unlock(&s->s_mutex);
>                         s->s_state = CEPH_MDS_SESSION_RESTARTING;
> @@ -5964,6 +6066,9 @@ static void check_new_map(struct ceph_mds_client *mdsc,
>          * Only open and reconnect sessions that don't exist yet.
>          */
>         for (i = 0; i < newmap->possible_max_rank; i++) {
> +               if (mdsc->stopping)
> +                       return;
> +
>                 /*
>                  * In case the import MDS is crashed just after
>                  * the EImportStart journal is flushed, so when
> @@ -5989,6 +6094,19 @@ static void check_new_map(struct ceph_mds_client *mdsc,
>                  * reconnection request in up:reconnect state.
>                  */
>                 s = __ceph_lookup_mds_session(mdsc, i);
> +               if (s && s->s_state == CEPH_MDS_SESSION_CLOSED) {
> +                       /*
> +                        * Wait for handle_session() to remove the caps and
> +                        * unregister this session, so the reconnect below
> +                        * uses a fresh session on the now vacant rank
> +                        */
> +                       ceph_put_mds_session(s);
> +                       wait_for_mds_rank_not_closing(mdsc, i);
> +                       if (mdsc->stopping ||
> +                           mdsc->mdsmap->m_epoch != map_epoch)
> +                               return;
> +                       s = NULL;
> +               }
>                 if (likely(!s)) {
>                         s = __open_export_target_session(mdsc, i);
>                         if (IS_ERR(s)) {
> @@ -7149,9 +7267,7 @@ static void mds_peer_reset(struct ceph_connection *con)
>          * Snapshot session state with READ_ONCE, then revalidate under
>          * mdsc->mutex before acting.  The subsequent mdsc->mutex
>          * section rechecks s_state to catch concurrent transitions, so
> -        * the lockless snapshot here is safe.  s->s_mutex is taken
> -        * separately for cleanup after unregistration, which avoids
> -        * introducing a new s->s_mutex + mdsc->mutex nesting.
> +        * the lockless snapshot here is safe.
>          */
>         session_state = READ_ONCE(s->s_state);
>
> @@ -7176,18 +7292,24 @@ static void mds_peer_reset(struct ceph_connection *con)
>
>                 ceph_get_mds_session(s);
>                 s->s_state = CEPH_MDS_SESSION_CLOSED;
> -               __unregister_session(mdsc, s);
> -               __wake_requests(mdsc, &s->s_waiting);
>                 mutex_unlock(&mdsc->mutex);
>
>                 mutex_lock(&s->s_mutex);
>                 cleanup_session_requests(mdsc, s);
>                 remove_session_caps(s);
> +
> +               /* Keep the rank occupied until all old-session caps are gone. */
> +               mutex_lock(&mdsc->mutex);
> +               if (!__verify_registered_session(mdsc, s))
> +                       __unregister_session(mdsc, s);
> +               mutex_unlock(&mdsc->mutex);
> +
>                 mutex_unlock(&s->s_mutex);
>
>                 wake_up_all(&mdsc->session_close_wq);
>
>                 mutex_lock(&mdsc->mutex);
> +               __wake_requests(mdsc, &s->s_waiting);
>                 kick_requests(mdsc, s->s_mds);
>                 mutex_unlock(&mdsc->mutex);
>
> --
> 2.47.3
>

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

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

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-04 15:03 [PATCH v4 0/3] ceph: don't unregister an MDS session before removing its caps Max Kellermann
2026-09-04 15:03 ` [PATCH v4 1/3] ceph: fix use-after-free in check_new_map() after early session put Max Kellermann
2026-09-05  1:52   ` Xiubo Li
2026-09-04 15:03 ` [PATCH v4 2/3] ceph: stop checking a stale MDS map after dropping mutex Max Kellermann
2026-09-05  1:52   ` Xiubo Li
2026-09-04 15:03 ` [PATCH v4 3/3] ceph: don't unregister an MDS session before removing its caps Max Kellermann
2026-09-05  1:56   ` 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®