Skip to content

Commit 62575e2

Browse files
jtlaytonidryomov
authored andcommitted
ceph: check session state after bumping session->s_seq
Some messages sent by the MDS entail a session sequence number increment, and the MDS will drop certain types of requests on the floor when the sequence numbers don't match. In particular, a REQUEST_CLOSE message can cross with one of the sequence morphing messages from the MDS which can cause the client to stall, waiting for a response that will never come. Originally, this meant an up to 5s delay before the recurring workqueue job kicked in and resent the request, but a recent change made it so that the client would never resend, causing a 60s stall unmounting and sometimes a blockisting event. Add a new helper for incrementing the session sequence and then testing to see whether a REQUEST_CLOSE needs to be resent, and move the handling of CEPH_MDS_SESSION_CLOSING into that function. Change all of the bare sequence counter increments to use the new helper. Reorganize check_session_state with a switch statement. It should no longer be called when the session is CLOSING, so throw a warning if it ever is (but still handle that case sanely). [ idryomov: whitespace, pr_err() call fixup ] URL: https://tracker.ceph.com/issues/47563 Fixes: fa99677 ("ceph: fix potential mdsc use-after-free crash") Reported-by: Patrick Donnelly <pdonnell@redhat.com> Signed-off-by: Jeff Layton <jlayton@kernel.org> Reviewed-by: Ilya Dryomov <idryomov@gmail.com> Reviewed-by: Xiubo Li <xiubli@redhat.com> Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
1 parent 3cea11c commit 62575e2

5 files changed

Lines changed: 39 additions & 18 deletions

File tree

fs/ceph/caps.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4074,7 +4074,7 @@ void ceph_handle_caps(struct ceph_mds_session *session,
40744074
vino.snap, inode);
40754075

40764076
mutex_lock(&session->s_mutex);
4077-
session->s_seq++;
4077+
inc_session_sequence(session);
40784078
dout(" mds%d seq %lld cap seq %u\n", session->s_mds, session->s_seq,
40794079
(unsigned)seq);
40804080

fs/ceph/mds_client.c

Lines changed: 35 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -4231,7 +4231,7 @@ static void handle_lease(struct ceph_mds_client *mdsc,
42314231
dname.len, dname.name);
42324232

42334233
mutex_lock(&session->s_mutex);
4234-
session->s_seq++;
4234+
inc_session_sequence(session);
42354235

42364236
if (!inode) {
42374237
dout("handle_lease no inode %llx\n", vino.ino);
@@ -4385,28 +4385,48 @@ static void maybe_recover_session(struct ceph_mds_client *mdsc)
43854385

43864386
bool check_session_state(struct ceph_mds_session *s)
43874387
{
4388-
if (s->s_state == CEPH_MDS_SESSION_CLOSING) {
4389-
dout("resending session close request for mds%d\n",
4390-
s->s_mds);
4391-
request_close_session(s);
4392-
return false;
4393-
}
4394-
if (s->s_ttl && time_after(jiffies, s->s_ttl)) {
4395-
if (s->s_state == CEPH_MDS_SESSION_OPEN) {
4388+
switch (s->s_state) {
4389+
case CEPH_MDS_SESSION_OPEN:
4390+
if (s->s_ttl && time_after(jiffies, s->s_ttl)) {
43964391
s->s_state = CEPH_MDS_SESSION_HUNG;
43974392
pr_info("mds%d hung\n", s->s_mds);
43984393
}
4399-
}
4400-
if (s->s_state == CEPH_MDS_SESSION_NEW ||
4401-
s->s_state == CEPH_MDS_SESSION_RESTARTING ||
4402-
s->s_state == CEPH_MDS_SESSION_CLOSED ||
4403-
s->s_state == CEPH_MDS_SESSION_REJECTED)
4404-
/* this mds is failed or recovering, just wait */
4394+
break;
4395+
case CEPH_MDS_SESSION_CLOSING:
4396+
/* Should never reach this when we're unmounting */
4397+
WARN_ON_ONCE(true);
4398+
fallthrough;
4399+
case CEPH_MDS_SESSION_NEW:
4400+
case CEPH_MDS_SESSION_RESTARTING:
4401+
case CEPH_MDS_SESSION_CLOSED:
4402+
case CEPH_MDS_SESSION_REJECTED:
44054403
return false;
4404+
}
44064405

44074406
return true;
44084407
}
44094408

4409+
/*
4410+
* If the sequence is incremented while we're waiting on a REQUEST_CLOSE reply,
4411+
* then we need to retransmit that request.
4412+
*/
4413+
void inc_session_sequence(struct ceph_mds_session *s)
4414+
{
4415+
lockdep_assert_held(&s->s_mutex);
4416+
4417+
s->s_seq++;
4418+
4419+
if (s->s_state == CEPH_MDS_SESSION_CLOSING) {
4420+
int ret;
4421+
4422+
dout("resending session close request for mds%d\n", s->s_mds);
4423+
ret = request_close_session(s);
4424+
if (ret < 0)
4425+
pr_err("unable to close session to mds%d: %d\n",
4426+
s->s_mds, ret);
4427+
}
4428+
}
4429+
44104430
/*
44114431
* delayed work -- periodically trim expired leases, renew caps with mds
44124432
*/

fs/ceph/mds_client.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -480,6 +480,7 @@ struct ceph_mds_client {
480480
extern const char *ceph_mds_op_name(int op);
481481

482482
extern bool check_session_state(struct ceph_mds_session *s);
483+
void inc_session_sequence(struct ceph_mds_session *s);
483484

484485
extern struct ceph_mds_session *
485486
__ceph_lookup_mds_session(struct ceph_mds_client *, int mds);

fs/ceph/quota.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ void ceph_handle_quota(struct ceph_mds_client *mdsc,
5353

5454
/* increment msg sequence number */
5555
mutex_lock(&session->s_mutex);
56-
session->s_seq++;
56+
inc_session_sequence(session);
5757
mutex_unlock(&session->s_mutex);
5858

5959
/* lookup inode */

fs/ceph/snap.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -873,7 +873,7 @@ void ceph_handle_snap(struct ceph_mds_client *mdsc,
873873
ceph_snap_op_name(op), split, trace_len);
874874

875875
mutex_lock(&session->s_mutex);
876-
session->s_seq++;
876+
inc_session_sequence(session);
877877
mutex_unlock(&session->s_mutex);
878878

879879
down_write(&mdsc->snap_rwsem);

0 commit comments

Comments
 (0)