From c650f8fd4bc3c0e0591f49c721779a55061581b2 Mon Sep 17 00:00:00 2001 From: Patrick Donnelly Date: Tue, 19 Aug 2025 21:42:14 -0400 Subject: [PATCH] mon/MonClient: add assertions for monc lock in MonConnection When handling auth, we want to be sure these methods hold the monc_lock which protects, in particular, the client authorizer. Signed-off-by: Patrick Donnelly --- src/mon/MonClient.cc | 25 +++++++++++++++++-------- src/mon/MonClient.h | 4 +++- 2 files changed, 20 insertions(+), 9 deletions(-) diff --git a/src/mon/MonClient.cc b/src/mon/MonClient.cc index 16839cfc78c..bc7097d1de1 100644 --- a/src/mon/MonClient.cc +++ b/src/mon/MonClient.cc @@ -286,7 +286,7 @@ int MonClient::ping_monitor(const string &mon_id, string *result_reply) ldout(cct, 10) << __func__ << " ping mon." << new_mon_id << " " << con->get_peer_addr() << dendl; - pinger->mc.reset(new MonConnection(cct, con, 0, &auth_registry)); + pinger->mc.reset(new MonConnection(cct, con, 0, &auth_registry, monc_lock)); pinger->mc->start(monmap.get_epoch(), entity_name); con->send_message(new MPing); @@ -637,6 +637,7 @@ int MonClient::authenticate(double timeout) void MonClient::_wipe_secrets_and_tickets() { + ceph_assert(ceph_mutex_is_locked_by_me(monc_lock)); ldout(cct, 5) << " wiping rotating secrets and invalidating tickets" << dendl; rotating_secrets->wipe(); auth->invalidate_all_tickets(); @@ -824,9 +825,10 @@ void MonClient::_reopen_session(int rank) void MonClient::_add_conn(unsigned rank) { + ceph_assert(ceph_mutex_is_locked_by_me(monc_lock)); auto peer = monmap.get_addrs(rank); auto conn = messenger->connect_to_mon(peer); - MonConnection mc(cct, conn, global_id, &auth_registry); + MonConnection mc(cct, conn, global_id, &auth_registry, monc_lock); if (auth) { mc.get_auth().reset(auth->clone()); } @@ -1245,7 +1247,7 @@ void MonClient::_send_command(MonCommand *r) } r->target_session.reset(new MonConnection(cct, r->target_con, 0, - &auth_registry)); + &auth_registry, monc_lock)); r->target_session->start(monmap.get_epoch(), entity_name); r->last_send_attempt = ceph_clock_now(); @@ -1564,8 +1566,8 @@ int MonClient::handle_auth_done( CryptoKey *session_key, std::string *connection_secret) { + std::lock_guard l(monc_lock); if (con->get_peer_type() == CEPH_ENTITY_TYPE_MON) { - std::lock_guard l(monc_lock); if (con->is_anon()) { for (auto& i : mon_commands) { if (i.second->target_con == con) { @@ -1620,9 +1622,8 @@ int MonClient::handle_auth_bad_method( const std::vector& allowed_methods, const std::vector& allowed_modes) { - auth_meta->allowed_methods = allowed_methods; - std::lock_guard l(monc_lock); + auth_meta->allowed_methods = allowed_methods; if (con->get_peer_type() == CEPH_ENTITY_TYPE_MON) { if (con->is_anon()) { for (auto& i : mon_commands) { @@ -1678,6 +1679,7 @@ int MonClient::handle_auth_request( const ceph::buffer::list& payload, ceph::buffer::list *reply) { + std::lock_guard l(monc_lock); if (payload.length() == 0) { // for some channels prior to nautilus (osd heartbeat), we // tolerate the lack of an authorizer. @@ -1753,8 +1755,8 @@ AuthAuthorizer* MonClient::build_authorizer(int service_id) const { MonConnection::MonConnection( CephContext *cct, ConnectionRef con, uint64_t global_id, - AuthRegistry *ar) - : cct(cct), con(con), global_id(global_id), auth_registry(ar) + AuthRegistry *ar, ceph::mutex& m) + : cct(cct), con(con), global_id(global_id), auth_registry(ar), monc_lock(m) {} MonConnection::~MonConnection() @@ -1812,6 +1814,7 @@ int MonConnection::get_auth_request( uint32_t want_keys, RotatingKeyRing* keyring) { + ceph_assert(ceph_mutex_is_locked_by_me(monc_lock)); using ceph::encode; // choose method if (auth_method < 0) { @@ -1850,6 +1853,7 @@ int MonConnection::handle_auth_reply_more( const ceph::buffer::list& bl, ceph::buffer::list *reply) { + ceph_assert(ceph_mutex_is_locked_by_me(monc_lock)); ldout(cct, 10) << __func__ << " payload " << bl.length() << dendl; ldout(cct, 30) << __func__ << " got\n"; bl.hexdump(*_dout); @@ -1882,6 +1886,7 @@ int MonConnection::handle_auth_done( CryptoKey *session_key, std::string *connection_secret) { + ceph_assert(ceph_mutex_is_locked_by_me(monc_lock)); ldout(cct,10) << __func__ << " global_id " << new_global_id << " payload " << bl.length() << dendl; @@ -1907,6 +1912,7 @@ int MonConnection::handle_auth_bad_method( const std::vector& allowed_methods, const std::vector& allowed_modes) { + ceph_assert(ceph_mutex_is_locked_by_me(monc_lock)); ldout(cct,10) << __func__ << " old_auth_method " << old_auth_method << " result " << cpp_strerror(result) << " allowed_methods " << allowed_methods << dendl; @@ -1940,6 +1946,7 @@ int MonConnection::handle_auth(MAuthReply* m, uint32_t want_keys, RotatingKeyRing* keyring) { + ceph_assert(ceph_mutex_is_locked_by_me(monc_lock)); if (state == State::NEGOTIATING) { int r = _negotiate(m, entity_name, want_keys, keyring); if (r) { @@ -1978,6 +1985,7 @@ int MonConnection::_init_auth( RotatingKeyRing* keyring, bool msgr2) { + ceph_assert(ceph_mutex_is_locked_by_me(monc_lock)); ldout(cct, 10) << __func__ << " method " << method << dendl; if (auth && auth->get_protocol() == (int)method) { ldout(cct, 10) << __func__ << " already have auth, reseting" << dendl; @@ -2012,6 +2020,7 @@ int MonConnection::_init_auth( int MonConnection::authenticate(MAuthReply *m) { + ceph_assert(ceph_mutex_is_locked_by_me(monc_lock)); ceph_assert(auth); if (!m->global_id) { ldout(cct, 1) << "peer sent an invalid global_id" << dendl; diff --git a/src/mon/MonClient.h b/src/mon/MonClient.h index ece08ccea11..01c3b60bba9 100644 --- a/src/mon/MonClient.h +++ b/src/mon/MonClient.h @@ -60,7 +60,8 @@ public: MonConnection(CephContext *cct, ConnectionRef conn, uint64_t global_id, - AuthRegistry *auth_registry); + AuthRegistry *auth_registry, + ceph::mutex& m); ~MonConnection(); MonConnection(MonConnection&& rhs) = default; MonConnection& operator=(MonConnection&&) = default; @@ -144,6 +145,7 @@ private: MessageRef pending_tell_command; AuthRegistry *auth_registry; + ceph::mutex& monc_lock; }; -- 2.39.5