From: Sage Weil <sage@newdream.net>
Date: Wed, 23 Dec 2009 19:49:51 +0000 (-0800)
Subject: msgr: drop rank lock while verifying authorizer
X-Git-Tag: v0.19~209
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=076e0dd7986ef5fde403847acfe6c4dae6130f7c;p=ceph.git

msgr: drop rank lock while verifying authorizer

This easily causes a lock inversion if the verify callback takes any
locks.  e.g.

09.12.23 11:43:32.335042 1482578256 lockdep: ------------------------------------
09.12.23 11:43:32.335072 1482578256 lockdep: existing dependency MDS::mds_lock (10) -> SimpleMessenger::lock (6) at:
 1: (Mutex::Lock(bool)+0x3a) [0x6f581c]
 2: (SimpleMessenger::submit_message(Message*, entity_inst_t const&, bool)+0xc0) [0x6f900c]
 3: (SimpleMessenger::Endpoint::send_message(Message*, entity_inst_t)+0x1ab) [0x6f9c85]
 4: (MonClient::_send_mon_message(Message*, bool)+0xa2) [0x9682dc]
 5: (MonClient::send_mon_message(Message*)+0x37) [0x740e17]
 6: (MDS::beacon_send()+0x226) [0x710b02]
 7: (MDS::beacon_start()+0x15) [0x711fc5]
 8: (MDS::init()+0xd5) [0x71238b]
 9: (main()+0x2cd) [0x6eb6af]
 10: (__libc_start_main()+0xe6) [0x7f22954811a6]
 11: /tmp/cmds.20091223.114259 [0x6eb0d9]

09.12.23 11:43:32.335289 1482578256 lockdep: new dependency SimpleMessenger::lock (6) -> MDS::mds_lock (10) creates a cycle at
 1: (Mutex::Lock(bool)+0x3a) [0x6f581c]
 2: (Mutex::Locker::Locker(Mutex&)+0x2c) [0x705f2c]
 3: (MDS::ms_verify_authorizer(Connection*, int, int, buffer::list&, buffer::list&, bool&)+0x48) [0x70d6c6]
 4: (Messenger::ms_deliver_verify_authorizer(Connection*, int, int, buffer::list&, buffer::list&, bool&)+0x74) [0x702d5e]
 5: (SimpleMessenger::verify_authorizer(Connection*, int, int, buffer::list&, buffer::list&, bool&)+0x6b) [0x6f6279]
 6: (SimpleMessenger::Pipe::accept()+0xc7d) [0x6ffba5]
 7: (SimpleMessenger::Pipe::reader()+0x32) [0x700de8]
 8: (SimpleMessenger::Pipe::Reader::entry()+0x19) [0x6f4e2d]
 9: (Thread::_entry_func(void*)+0x20) [0x705944]
 10: /lib/libpthread.so.0 [0x7f22962effc7]
 11: (clone()+0x6d) [0x7f22955325ad]

09.12.23 11:43:32.335497 1482578256 lockdep: btw, i am holding these locks:
09.12.23 11:43:32.335511 1482578256 lockdep:   SimpleMessenger::lock (6)


common/lockdep.cc: In function 'int lockdep_will_lock(const char*, int)':
common/lockdep.cc:184: FAILED assert(0)
 1: (lockdep_will_lock(char const*, int)+0x765) [0x9776a9]
 2: (Mutex::_will_lock()+0x1f) [0x6f43bf]
 3: (Mutex::Lock(bool)+0x3a) [0x6f581c]
 4: (Mutex::Locker::Locker(Mutex&)+0x2c) [0x705f2c]
 5: (MDS::ms_verify_authorizer(Connection*, int, int, buffer::list&, buffer::list&, bool&)+0x48) [0x70d6c6]
 6: (Messenger::ms_deliver_verify_authorizer(Connection*, int, int, buffer::list&, buffer::list&, bool&)+0x74) [0x702d5e]
 7: (SimpleMessenger::verify_authorizer(Connection*, int, int, buffer::list&, buffer::list&, bool&)+0x6b) [0x6f6279]
 8: (SimpleMessenger::Pipe::accept()+0xc7d) [0x6ffba5]
 9: (SimpleMessenger::Pipe::reader()+0x32) [0x700de8]
 10: (SimpleMessenger::Pipe::Reader::entry()+0x19) [0x6f4e2d]
 11: (Thread::_entry_func(void*)+0x20) [0x705944]
 12: /lib/libpthread.so.0 [0x7f22962effc7]
 13: (clone()+0x6d) [0x7f22955325ad]
 NOTE: a copy of the executable, or `objdump -rdS <executable>` is needed to interpret this.
---

diff --git a/src/msg/SimpleMessenger.cc b/src/msg/SimpleMessenger.cc
index d338bd317ed2..15f50054891f 100644
--- a/src/msg/SimpleMessenger.cc
+++ b/src/msg/SimpleMessenger.cc
@@ -667,14 +667,15 @@ int SimpleMessenger::Pipe::accept()
       goto reply;
     }
     
+    rank->lock.Unlock();
     if (rank->verify_authorizer(connection_state, peer_type,
 				connect.authorizer_protocol, authorizer, authorizer_reply, authorizer_valid) &&
 	!authorizer_valid) {
       dout(0) << "accept bad authorizer" << dendl;
       reply.tag = CEPH_MSGR_TAG_BADAUTHORIZER;
-      rank->lock.Unlock();
       goto reply;
     }
+    rank->lock.Lock();
     
     // existing?
     if (rank->rank_pipe.count(peer_addr)) {