[PATCH v4 0/3] ceph: don't unregister an MDS session before removing its caps

[PATCH v4 0/3] ceph: don't unregister an MDS session before removing its caps

[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

[PATCH v4 1/3] ceph: fix use-after-free in check_new_map() after early session put

[Max Kellermann]

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

Re: [PATCH v4 1/3] ceph: fix use-after-free in check_new_map() after early session put

[Xiubo Li]

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
>

[PATCH v4 2/3] ceph: stop checking a stale MDS map after dropping mutex

[Max Kellermann]

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

Re: [PATCH v4 2/3] ceph: stop checking a stale MDS map after dropping mutex

[Xiubo Li]

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
>

[PATCH v4 3/3] ceph: don't unregister an MDS session before removing its caps

[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.

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

Re: [PATCH v4 3/3] ceph: don't unregister an MDS session before removing its caps

[Xiubo Li]

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
>

Re: [PATCH v4 3/3] ceph: don't unregister an MDS session before removing its caps

[Max Kellermann]

On Sat, Sep 5, 2026 at 3:56 AM Xiubo Li <xiubo.li@clyso.com> wrote:
> 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().

I don't think that's true for the call paths you mentioned. They call
ceph_mdsc_close_sessions() only after ceph_update_snap_trace() has
failed, and on failure, that function always sets CEPH_MOUNT_FENCE_IO,
which prevents creating a new session.

The only other callers are ceph_put_super() and the mount-failure path
in ceph_get_tree(), where this is not relevant (no requests exist).

Am I missing something?

-- 
Max Kellermann
Principal Architect
Hosting Technology

cm4all | Im Mediapark 6a | 50670 Köln | Germany
General information about the company can be found here:
https://www.cm4all.com/impressum
A member of the IONOS Group

Re: [PATCH v4 3/3] ceph: don't unregister an MDS session before removing its caps

[Xiubo Li]

Hi Max

I think you are right. I just checked the code again, I might have
messed up the code with my local temp patches.

Then this LGTM.

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

Thanks

On Mon, 7 Sept 2026 at 12:15, Max Kellermann <max.kellermann@ionos.com> wrote:
>
> On Sat, Sep 5, 2026 at 3:56 AM Xiubo Li <xiubo.li@clyso.com> wrote:
> > 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().
>
> I don't think that's true for the call paths you mentioned. They call
> ceph_mdsc_close_sessions() only after ceph_update_snap_trace() has
> failed, and on failure, that function always sets CEPH_MOUNT_FENCE_IO,
> which prevents creating a new session.
>
> The only other callers are ceph_put_super() and the mount-failure path
> in ceph_get_tree(), where this is not relevant (no requests exist).
>
> Am I missing something?
>
> --
> Max Kellermann
> Principal Architect
> Hosting Technology
>
> cm4all | Im Mediapark 6a | 50670 Köln | Germany
> General information about the company can be found here:
> https://www.cm4all.com/impressum
> A member of the IONOS Group

Re: [v4,3/3] ceph: don't unregister an MDS session before removing its caps

[Alex Markuze]

Hi Max,

NACK. Please respin.

  1. fs/ceph/mds_client.c: ceph_mdsc_close_sessions() still unregisters before remove_session_caps()
     Max, 1/3 and 2/3 look right, but the series is incomplete.
     The teardown loop in `ceph_mdsc_close_sessions()` still does
     `__unregister_session()` then `remove_session_caps()`.
     That path is also reached from corrupt snap-trace handling in
     `ceph_handle_caps()`, `ceph_handle_reply()`, and `handle_snap()`.
     There `mdsc->stopping` may be false and the mount need not be
     fenced, so `__do_request()` can register a new session for the
     rank in that window — the same race this series is meant to close.
     Use the same CLOSED → remove caps → verify/unregister ordering
     as `handle_session()`, and wake `mdsc->session_close_wq` after
     unregister so `wait_for_mds_rank_not_closing()` waiters can
     proceed.

-- 
Alex Markuze

Re: [PATCH v4 0/3] ceph: don't unregister an MDS session before removing its caps

[chenyuxi (D)]

Hi Max,

Do you have any plans to move this series forward?

Patch 1/3 completes the fix for CVE-2026-89654 (use-after-free
in check_new_map()): the earlier fix ee611a750955 still leaves
the UAF in place.  We are tracking this CVE in our
downstream kernels, so we'd really like to see the series land
in mainline.  The thread, however, has been quiet since
Sep 8th, when 3/3 picked up Xiubo's Reviewed-by, and the
patches are in neither the ceph-client testing branch nor
mainline yet.

Is there anything still holding the series back that we could
help with?

Thanks,
Chen Yuxi

Re: [PATCH v4 0/3] ceph: don't unregister an MDS session before removing its caps

[Max Kellermann]

On Wed, Oct 14, 2026 at 11:15 AM chenyuxi (D) <chenyuxi19@huawei.com> wrote:
> Do you have any plans to move this series forward?
[...]
> Is there anything still holding the series back that we could
> help with?

It was rejected by Alex Markuze, and he never replied to me after I
questioned the two reviews. That's what's holding this back.

As for my plans - I have more Ceph unpublished fixes in our internal
Linux kernel fork, but I'm not submitting them, see
https://lore.kernel.org/ceph-devel/CAKPOu+8SiwxT_44e63-EZ4T_4d-tGjwmk_Lx8i4KgZOL5JnMtQ@mail.gmail.com/
for why.
I just looked it up: the stack of Ceph patches in our fork has grown
to 67 patches. They were either rejected or ignored. One of the
rejected patches fixes a security vulnerability.

-- 
Max Kellermann
Principal Architect
Hosting Technology

cm4all | Im Mediapark 6a | 50670 Köln | Germany
General information about the company can be found here:
https://www.cm4all.com/impressum
A member of the IONOS Group

Re: [PATCH v4 0/3] ceph: don't unregister an MDS session before removing its caps

[Alex Markuze]

Since the last merge with main Linux the testing branch has grown by
upwards of 100 acked patches.
Xiubo acked your patch, which is good enough for me, Xiubo is back on
the maintainers list for ceph client.
I'll take another look at the discussion for fairness' sake.

Feel free to submit and argue for any other patches you feel are necessary.
If a patch solves a problem or adds needed functionality it will be
reviewed and applied as always.

On Wed, Oct 14, 2026 at 12:50 PM Max Kellermann
<max.kellermann@ionos.com> wrote:
>
> On Wed, Oct 14, 2026 at 11:15 AM chenyuxi (D) <chenyuxi19@huawei.com> wrote:
> > Do you have any plans to move this series forward?
> [...]
> > Is there anything still holding the series back that we could
> > help with?
>
> It was rejected by Alex Markuze, and he never replied to me after I
> questioned the two reviews. That's what's holding this back.
>
> As for my plans - I have more Ceph unpublished fixes in our internal
> Linux kernel fork, but I'm not submitting them, see
> https://lore.kernel.org/ceph-devel/CAKPOu+8SiwxT_44e63-EZ4T_4d-tGjwmk_Lx8i4KgZOL5JnMtQ@mail.gmail.com/
> for why.
> I just looked it up: the stack of Ceph patches in our fork has grown
> to 67 patches. They were either rejected or ignored. One of the
> rejected patches fixes a security vulnerability.
>
> --
> Max Kellermann
> Principal Architect
> Hosting Technology
>
> cm4all | Im Mediapark 6a | 50670 Köln | Germany
> General information about the company can be found here:
> https://www.cm4all.com/impressum
> A member of the IONOS Group
>

Re: [PATCH v4 0/3] ceph: don't unregister an MDS session before removing its caps

[Alex Markuze]

Max, Please resend the patchset, the tree has diverged since this
patchset was sent back in September.

On Wed, Oct 14, 2026 at 1:46 PM Alex Markuze <amarkuze@redhat.com> wrote:
>
> Since the last merge with main Linux the testing branch has grown by
> upwards of 100 acked patches.
> Xiubo acked your patch, which is good enough for me, Xiubo is back on
> the maintainers list for ceph client.
> I'll take another look at the discussion for fairness' sake.
>
> Feel free to submit and argue for any other patches you feel are necessary.
> If a patch solves a problem or adds needed functionality it will be
> reviewed and applied as always.
>
> On Wed, Oct 14, 2026 at 12:50 PM Max Kellermann
> <max.kellermann@ionos.com> wrote:
> >
> > On Wed, Oct 14, 2026 at 11:15 AM chenyuxi (D) <chenyuxi19@huawei.com> wrote:
> > > Do you have any plans to move this series forward?
> > [...]
> > > Is there anything still holding the series back that we could
> > > help with?
> >
> > It was rejected by Alex Markuze, and he never replied to me after I
> > questioned the two reviews. That's what's holding this back.
> >
> > As for my plans - I have more Ceph unpublished fixes in our internal
> > Linux kernel fork, but I'm not submitting them, see
> > https://lore.kernel.org/ceph-devel/CAKPOu+8SiwxT_44e63-EZ4T_4d-tGjwmk_Lx8i4KgZOL5JnMtQ@mail.gmail.com/
> > for why.
> > I just looked it up: the stack of Ceph patches in our fork has grown
> > to 67 patches. They were either rejected or ignored. One of the
> > rejected patches fixes a security vulnerability.
> >
> > --
> > Max Kellermann
> > Principal Architect
> > Hosting Technology
> >
> > cm4all | Im Mediapark 6a | 50670 Köln | Germany
> > General information about the company can be found here:
> > https://www.cm4all.com/impressum
> > A member of the IONOS Group
> >

Re: [PATCH v4 0/3] ceph: don't unregister an MDS session before removing its caps

[Max Kellermann]

On Wed, Oct 14, 2026 at 1:28 PM Alex Markuze <amarkuze@redhat.com> wrote:
>
> Max, Please resend the patchset, the tree has diverged since this
> patchset was sent back in September.

I see no rebase conflicts with linus/master nor ceph-client/for-linus.
What other recent changes would make resending necessary?

-- 
Max Kellermann
Principal Architect
Hosting Technology

cm4all | Im Mediapark 6a | 50670 Köln | Germany
General information about the company can be found here:
https://www.cm4all.com/impressum
A member of the IONOS Group