From 723d691f7a1f53888618dfc311868d1988f61f56 Mon Sep 17 00:00:00 2001 From: Sage Weil Date: Thu, 18 Jul 2013 09:55:43 -0700 Subject: [PATCH] msg/Pipe: do not hold pipe_lock for verify_authorizer() We shouldn't hold the pipe_lock while doing the ms_verify_authorizer upcalls. Fix by unlocking a bit earlier, and verifying our state is still correct in the failure path. This regression was introduced by ecab4bb9513385bd765cca23e4e2fadb7ac4bac2. Signed-off-by: Sage Weil Reviewed-by: Greg Farnum --- src/msg/Pipe.cc | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/msg/Pipe.cc b/src/msg/Pipe.cc index bff0b996b20..03ee79a15c2 100644 --- a/src/msg/Pipe.cc +++ b/src/msg/Pipe.cc @@ -385,10 +385,15 @@ int Pipe::accept() // Check the authorizer. If not good, bail out. + pipe_lock.Unlock(); + if (!msgr->verify_authorizer(connection_state.get(), peer_type, connect.authorizer_protocol, authorizer, authorizer_reply, authorizer_valid, session_key) || !authorizer_valid) { ldout(msgr->cct,0) << "accept: got bad authorizer" << dendl; + pipe_lock.Lock(); + if (state != STATE_ACCEPTING) + goto shutting_down_msgr_unlocked; reply.tag = CEPH_MSGR_TAG_BADAUTHORIZER; delete session_security; session_security = NULL; @@ -399,7 +404,6 @@ int Pipe::accept() ldout(msgr->cct,10) << "accept: setting up session_security." << dendl; - pipe_lock.Unlock(); msgr->lock.Lock(); pipe_lock.Lock(); if (msgr->dispatch_queue.stop) @@ -711,8 +715,9 @@ int Pipe::accept() return -1; shutting_down: - assert(pipe_lock.is_locked()); msgr->lock.Unlock(); + shutting_down_msgr_unlocked: + assert(pipe_lock.is_locked()); if (msgr->cct->_conf->ms_inject_internal_delays) { ldout(msgr->cct, 10) << " sleep for " << msgr->cct->_conf->ms_inject_internal_delays << dendl;