* [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
* 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
* [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
* 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
* [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 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®