From 1f55b17594bc9063b35f52b76c562746005879cd Mon Sep 17 00:00:00 2001 From: Sage Weil Date: Thu, 29 Apr 2010 10:27:02 -0700 Subject: [PATCH] mon: m->get_session() may return null if session has closed because the session close clears connection->priv. We need to check at each site anyway, either for null session, or for session->closed. So check for null session. Conflicts: src/mon/Monitor.cc src/mon/OSDMonitor.cc src/mon/PGMonitor.cc --- src/messages/PaxosServiceMessage.h | 3 +- src/mon/LogMonitor.cc | 20 +++- src/mon/MDSMonitor.cc | 36 ++++-- src/mon/Monitor.cc | 44 ++++--- src/mon/OSDMonitor.cc | 181 +++++++++++++++++------------ src/mon/PGMonitor.cc | 28 +++-- 6 files changed, 195 insertions(+), 117 deletions(-) diff --git a/src/messages/PaxosServiceMessage.h b/src/messages/PaxosServiceMessage.h index 1b670bfa483dc..88695f3d7536f 100644 --- a/src/messages/PaxosServiceMessage.h +++ b/src/messages/PaxosServiceMessage.h @@ -48,7 +48,8 @@ class PaxosServiceMessage : public Message { */ MonSession *get_session() { MonSession *session = (MonSession *)get_connection()->get_priv(); - session->put(); + if (session) + session->put(); return session; } diff --git a/src/mon/LogMonitor.cc b/src/mon/LogMonitor.cc index 1cd6722bf562a..28b1f54e1e651 100644 --- a/src/mon/LogMonitor.cc +++ b/src/mon/LogMonitor.cc @@ -229,14 +229,17 @@ void LogMonitor::committed() bool LogMonitor::preprocess_log(MLog *m) { dout(10) << "preprocess_log " << *m << " from " << m->get_orig_source() << dendl; + int num_new = 0; - if (!m->get_session()->caps.check_privileges(PAXOS_LOG, MON_CAP_X)) { - dout(0) << "Received MLog from entity with insufficient privileges " - << m->get_session()->caps << dendl; - return true; //no reply expected + MonSession *session = m->get_session(); + if (!session) + goto done; + if (!session->caps.check_privileges(PAXOS_LOG, MON_CAP_X)) { + dout(0) << "preprocess_log got MLog from entity with insufficient privileges " + << session->caps << dendl; + goto done; } - int num_new = 0; for (deque::iterator p = m->entries.begin(); p != m->entries.end(); p++) { @@ -245,9 +248,14 @@ bool LogMonitor::preprocess_log(MLog *m) } if (!num_new) { dout(10) << " nothing new" << dendl; - return true; + goto done; } + return false; + + done: + delete m; + return true; } bool LogMonitor::prepare_log(MLog *m) diff --git a/src/mon/MDSMonitor.cc b/src/mon/MDSMonitor.cc index 850da42b43081..f7ebeab1e903b 100644 --- a/src/mon/MDSMonitor.cc +++ b/src/mon/MDSMonitor.cc @@ -158,10 +158,13 @@ bool MDSMonitor::preprocess_beacon(MMDSBeacon *m) version_t seq = m->get_seq(); MDSMap::mds_info_t info; - //check privileges, ignore if fails - if ( !m->get_session()->caps.check_privileges(PAXOS_MDSMAP, MON_CAP_X)) { - dout(0) << "received MMDSBeacon from entity with insufficient privileges " - << m->get_session()->caps << dendl; + // check privileges, ignore if fails + MonSession *session = m->get_session(); + if (!session) + goto out; + if (!session->caps.check_privileges(PAXOS_MDSMAP, MON_CAP_X)) { + dout(0) << "preprocess_beason got MMDSBeacon from entity with insufficient privileges " + << session->caps << dendl; goto out; } @@ -247,19 +250,28 @@ bool MDSMonitor::preprocess_beacon(MMDSBeacon *m) bool MDSMonitor::preprocess_offload_targets(MMDSLoadTargets* m) { dout(10) << "preprocess_offload_targets " << *m << " from " << m->get_orig_source() << dendl; - - //check privileges, ignore message if fails - if(!m->get_session()->caps.check_privileges(PAXOS_MDSMAP, MON_CAP_X)) { - dout(0) << "got MMDSLoadTargets from entity with insufficient caps " - << m->get_session()->caps << dendl; - return true; + __u64 gid; + + // check privileges, ignore message if fails + MonSession *session = m->get_session(); + if (!session) + goto done; + if (!session->caps.check_privileges(PAXOS_MDSMAP, MON_CAP_X)) { + dout(0) << "preprocess_offload_targets got MMDSLoadTargets from entity with insufficient caps " + << session->caps << dendl; + goto done; } - __u64 gid = m->global_id; + gid = m->global_id; if (mdsmap.mds_info.count(gid) && m->targets == mdsmap.mds_info[gid].export_targets) - return true; + goto done; + return false; + + done: + delete m; + return true; } diff --git a/src/mon/Monitor.cc b/src/mon/Monitor.cc index 8f61667e9c0dc..9475c767fc479 100644 --- a/src/mon/Monitor.cc +++ b/src/mon/Monitor.cc @@ -264,9 +264,12 @@ void Monitor::handle_command(MMonCommand *m) return; } - if (!m->get_session()->caps.check_privileges(PAXOS_MONMAP, MON_CAP_ALL)) { - string rs="Access denied"; - reply_command((MMonCommand *)m, -EACCES, rs, 0); + MonSession *session = m->get_session(); + if (!session || + !session->caps.check_privileges(PAXOS_MONMAP, MON_CAP_ALL)) { + string rs = "Access denied"; + reply_command(m, -EACCES, rs, 0); + return; } dout(0) << "handle_command " << *m << dendl; @@ -514,14 +517,17 @@ void Monitor::remove_session(MonSession *s) void Monitor::handle_observe(MMonObserve *m) { dout(10) << "handle_observe " << *m << " from " << m->get_source_inst() << dendl; - //check that there are perms. Send a response back if they aren't sufficient, - //and delete the message (if it's not deleted for us, which happens when - //we own the connection to the requested observer). - if (!m->get_session()->caps.check_privileges(PAXOS_MONMAP, MON_CAP_X)) { + // check that there are perms. Send a response back if they aren't sufficient, + // and delete the message (if it's not deleted for us, which happens when + // we own the connection to the requested observer). + MonSession *session = m->get_session(); + if (!session || !session->caps.check_privileges(PAXOS_MONMAP, MON_CAP_X)) { bool delete_m = false; - if (m->session_mon) delete_m = true; + if (m->session_mon) + delete_m = true; send_reply(m, m); - if (delete_m) delete m; + if (delete_m) + delete m; return; } if (m->machine_id >= PAXOS_NUM) { @@ -975,26 +981,32 @@ int Monitor::mkfs(bufferlist& osdmapbl) void Monitor::handle_class(MClass *m) { - if (!m->get_session()->caps.check_privileges(PAXOS_OSDMAP, MON_CAP_X)) { + MonSession *session = m->get_session(); + if (!session) + goto done; + if (!session->caps.check_privileges(PAXOS_OSDMAP, MON_CAP_X)) { dout(0) << "MClass received from entity without sufficient privileges " - << m->get_session()->caps << dendl; - delete m; - return; + << session->caps << dendl; + goto done; } + switch (m->action) { case CLASS_SET: case CLASS_GET: classmon()->handle_request(m); - break; + return; + case CLASS_RESPONSE: dout(0) << "got a class response (" << *m << ") ???" << dendl; - delete m; break; + default: dout(0) << "got an unknown class message (" << *m << ") ???" << dendl; - assert(0); break; } + + done: + delete m; } bool Monitor::ms_get_authorizer(int service_id, AuthAuthorizer **authorizer, bool force_new) diff --git a/src/mon/OSDMonitor.cc b/src/mon/OSDMonitor.cc index 10126515613b2..3f7c82cb3c587 100644 --- a/src/mon/OSDMonitor.cc +++ b/src/mon/OSDMonitor.cc @@ -291,15 +291,18 @@ bool OSDMonitor::preprocess_failure(MOSDFailure *m) // who is failed int badboy = m->get_failed().name.num(); - if (ceph_fsid_compare(&m->fsid, &mon->monmap->fsid)) { - dout(0) << "preprocess_failure on fsid " << m->fsid << " != " << mon->monmap->fsid << dendl; + // check permissions + MonSession *session = m->get_session(); + if (!session) + goto didit; + if (!session->caps.check_privileges(PAXOS_OSDMAP, MON_CAP_X)) { + dout(0) << "got MOSDFailure from entity with insufficient caps " + << session->caps << dendl; goto didit; } - //check permissions - if (!m->get_session()->caps.check_privileges(PAXOS_OSDMAP, MON_CAP_X)) { - dout(0) << "got MOSDFailure from entity with insufficient caps " - << m->get_session()->caps << dendl; + if (ceph_fsid_compare(&m->fsid, &mon->monmap->fsid)) { + dout(0) << "preprocess_failure on fsid " << m->fsid << " != " << mon->monmap->fsid << dendl; goto didit; } @@ -386,22 +389,24 @@ void OSDMonitor::_reported_failure(MOSDFailure *m) bool OSDMonitor::preprocess_boot(MOSDBoot *m) { - if (ceph_fsid_compare(&m->sb.fsid, &mon->monmap->fsid)) { - dout(0) << "preprocess_boot on fsid " << m->sb.fsid << " != " << mon->monmap->fsid << dendl; - delete m; - return true; - } + int from = m->get_orig_source_inst().name.num(); - //check permissions, ignore if failed (no response expected) - if (!m->get_session()->caps.check_privileges(PAXOS_OSDMAP, MON_CAP_X)) { + // check permissions, ignore if failed (no response expected) + MonSession *session = m->get_session(); + if (!session) + goto ignore; + if (!session->caps.check_privileges(PAXOS_OSDMAP, MON_CAP_X)) { dout(0) << "got preprocess_boot message from entity with insufficient caps" - << m->get_session()->caps << dendl; - delete m; - return true; + << session->caps << dendl; + goto ignore; + } + + if (ceph_fsid_compare(&m->sb.fsid, &mon->monmap->fsid)) { + dout(0) << "preprocess_boot on fsid " << m->sb.fsid << " != " << mon->monmap->fsid << dendl; + goto ignore; } assert(m->get_orig_source_inst().name.is_osd()); - int from = m->get_orig_source_inst().name.num(); // already booted? if (osdmap.is_up(from) && @@ -415,6 +420,10 @@ bool OSDMonitor::preprocess_boot(MOSDBoot *m) dout(10) << "preprocess_boot from " << m->get_orig_source_inst() << dendl; return false; + + ignore: + delete m; + return true; } bool OSDMonitor::prepare_boot(MOSDBoot *m) @@ -503,13 +512,18 @@ void OSDMonitor::_booted(MOSDBoot *m, bool logit) bool OSDMonitor::preprocess_alive(MOSDAlive *m) { - //check permissions, ignore if failed - if (!m->get_session()->caps.check_privileges(PAXOS_OSDMAP, MON_CAP_X)) { + int from = m->get_orig_source().num(); + + // check permissions, ignore if failed + MonSession *session = m->get_session(); + if (!session) + goto ignore; + if (!session->caps.check_privileges(PAXOS_OSDMAP, MON_CAP_X)) { dout(0) << "attempt to send MOSDAlive from entity with insufficient privileges:" - << m->get_session()->caps << dendl; - return true; + << session->caps << dendl; + goto ignore; } - int from = m->get_orig_source().num(); + if (osdmap.is_up(from) && osdmap.get_inst(from) == m->get_orig_source_inst() && osdmap.get_up_thru(from) >= m->map_epoch) { @@ -522,6 +536,10 @@ bool OSDMonitor::preprocess_alive(MOSDAlive *m) dout(10) << "preprocess_alive e" << m->map_epoch << " from " << m->get_orig_source_inst() << dendl; return false; + + ignore: + delete m; + return true; } bool OSDMonitor::prepare_alive(MOSDAlive *m) @@ -554,14 +572,18 @@ void OSDMonitor::_reply_map(PaxosServiceMessage *m, epoch_t e) bool OSDMonitor::preprocess_pgtemp(MOSDPGTemp *m) { dout(10) << "preprocess_pgtemp " << *m << dendl; + vector empty; - //check caps - if (!m->get_session()->caps.check_privileges(PAXOS_OSDMAP, MON_CAP_X)) { + // check caps + MonSession *session = m->get_session(); + if (!session) + goto ignore; + if (!session->caps.check_privileges(PAXOS_OSDMAP, MON_CAP_X)) { dout(0) << "attempt to send MOSDPGTemp from entity with insufficient caps " - << m->get_session()->caps << dendl; - return true; + << session->caps << dendl; + goto ignore; } - vector empty; + for (map >::iterator p = m->pg_temp.begin(); p != m->pg_temp.end(); p++) { dout(20) << " " << p->first << (osdmap.pg_temp.count(p->first) ? osdmap.pg_temp[p->first] : empty) @@ -578,6 +600,10 @@ bool OSDMonitor::preprocess_pgtemp(MOSDPGTemp *m) dout(7) << "preprocess_pgtemp e" << m->map_epoch << " no changes from " << m->get_orig_source_inst() << dendl; _reply_map(m, m->map_epoch); return true; + + ignore: + delete m; + return true; } bool OSDMonitor::prepare_pgtemp(MOSDPGTemp *m) @@ -598,13 +624,16 @@ bool OSDMonitor::preprocess_remove_snaps(MRemoveSnaps *m) { dout(7) << "preprocess_remove_snaps " << *m << dendl; - //check privilege, ignore if failed - if (!m->get_session()->caps.check_privileges(PAXOS_OSDMAP, MON_CAP_RW)) { + // check privilege, ignore if failed + MonSession *session = m->get_session(); + if (!session) + goto ignore; + if (!session->caps.check_privileges(PAXOS_OSDMAP, MON_CAP_RW)) { dout(0) << "got preprocess_remove_snaps from entity with insufficient caps " - << m->get_session()->caps << dendl; - delete m; - return true; + << session->caps << dendl; + goto ignore; } + for (map >::iterator q = m->snaps.begin(); q != m->snaps.end(); q++) { @@ -621,6 +650,8 @@ bool OSDMonitor::preprocess_remove_snaps(MRemoveSnaps *m) return false; } } + + ignore: delete m; return true; } @@ -1046,30 +1077,14 @@ bool OSDMonitor::preprocess_command(MMonCommand *m) int OSDMonitor::prepare_new_pool(MPoolOp *m) { - //check permissions for the auid, then pass off to next function - dout(10) << "prepare_new_pool from " - << (m->get_connection()) << dendl; - if (m->auid) { - if(m->get_session()-> - caps.check_privileges(PAXOS_OSDMAP, MON_CAP_W, m->auid)) { - return prepare_new_pool(m->name, m->auid); - } else { - dout(5) << "attempt to create new pool without sufficient auid privileges!" - << "message: " << *m << std::endl - << "caps: " << m->get_session()->caps << dendl; - return -EPERM; - } - } else { - if (m->get_session()->caps.check_privileges(PAXOS_OSDMAP, MON_CAP_W)) { - return prepare_new_pool(m->name, m->get_session()->caps.auid); - } else { - dout(5) << "attempt to create new pool without sufficient caps!" - << "message: " << *m << std::endl - << "caps: " << m->get_session()->caps << dendl; - return -EPERM; - } - } - return -1; //can't get here! + dout(10) << "prepare_new_pool from " << m->get_connection() << dendl; + MonSession *session = m->get_session(); + if (!session) + return -EPERM; + if (m->auid) + return prepare_new_pool(m->name, m->auid); + else + return prepare_new_pool(m->name, session->caps.auid); } int OSDMonitor::prepare_new_pool(string& name, __u64 auid) @@ -1353,7 +1368,8 @@ out: return false; } -bool OSDMonitor::preprocess_pool_op ( MPoolOp *m) { +bool OSDMonitor::preprocess_pool_op(MPoolOp *m) +{ dout(0) << "m->op=" << m->op << dendl; if (m->op == POOL_OP_CREATE) { return preprocess_pool_op_create(m); @@ -1403,15 +1419,31 @@ bool OSDMonitor::preprocess_pool_op ( MPoolOp *m) { bool OSDMonitor::preprocess_pool_op_create ( MPoolOp *m) { + MonSession *session = m->get_session(); + if (!session) { + _pool_op(m, -EPERM, pending_inc.epoch); + return true; + } + if ((m->auid && !session->caps.check_privileges(PAXOS_OSDMAP, MON_CAP_W, m->auid)) || + (!m->auid && !session->caps.check_privileges(PAXOS_OSDMAP, MON_CAP_W))) { + if (session) + dout(5) << "attempt to create new pool without sufficient auid privileges!" + << "message: " << *m << std::endl + << "caps: " << m->get_session()->caps << dendl; + _pool_op(m, -EPERM, pending_inc.epoch); + return true; + } + int pool = osdmap.lookup_pg_pool_name(m->name.c_str()); - if (pool < 0) - return false; + if (pool >= 0) { + _pool_op(m, -EEXIST, pending_inc.epoch); + return true; + } - _pool_op(m, -EEXIST, pending_inc.epoch); - return true; + return false; } -bool OSDMonitor::prepare_pool_op (MPoolOp *m) +bool OSDMonitor::prepare_pool_op(MPoolOp *m) { dout(10) << "prepare_pool_op " << *m << dendl; if (m->op == POOL_OP_CREATE) { @@ -1446,7 +1478,7 @@ bool OSDMonitor::prepare_pool_op (MPoolOp *m) return true; } -bool OSDMonitor::prepare_pool_op_create (MPoolOp *m) +bool OSDMonitor::prepare_pool_op_create(MPoolOp *m) { int err = prepare_new_pool(m); if (!err) { @@ -1458,30 +1490,33 @@ bool OSDMonitor::prepare_pool_op_create (MPoolOp *m) return true; } -bool OSDMonitor::prepare_pool_op_delete (MPoolOp *m) +bool OSDMonitor::prepare_pool_op_delete(MPoolOp *m) { pending_inc.old_pools.insert(m->pool); paxos->wait_for_commit(new OSDMonitor::C_PoolOp(this, m, 0, pending_inc.epoch)); return true; } -bool OSDMonitor::prepare_pool_op_auid (MPoolOp *m) +bool OSDMonitor::prepare_pool_op_auid(MPoolOp *m) { - //check that current user can write to new auid - if(m->get_session()-> - caps.check_privileges(PAXOS_OSDMAP, MON_CAP_W, m->auid)) { - //check that current user can write to old auid + // check that current user can write to new auid + MonSession *session = m->get_session(); + if (!session) + goto fail; + if (session->caps.check_privileges(PAXOS_OSDMAP, MON_CAP_W, m->auid)) { + // check that current user can write to old auid int old_auid = osdmap.get_pg_pool(m->pool)->v.auid; - if(m->get_session()-> - caps.check_privileges(PAXOS_OSDMAP, MON_CAP_W, old_auid)) { - //update pg_pool_t with new auid + if (session->caps.check_privileges(PAXOS_OSDMAP, MON_CAP_W, old_auid)) { + // update pg_pool_t with new auid pending_inc.new_pools[m->pool] = *(osdmap.get_pg_pool(m->pool)); pending_inc.new_pools[m->pool].v.auid = m->auid; paxos->wait_for_commit(new OSDMonitor::C_PoolOp(this, m, 0, pending_inc.epoch)); return true; } } - //if it gets here it failed a permissions check + + fail: + // if it gets here it failed a permissions check _pool_op(m, -EPERM, pending_inc.epoch); return true; } diff --git a/src/mon/PGMonitor.cc b/src/mon/PGMonitor.cc index 844398b75dc09..054e754f85137 100644 --- a/src/mon/PGMonitor.cc +++ b/src/mon/PGMonitor.cc @@ -190,10 +190,13 @@ void PGMonitor::committed() void PGMonitor::handle_statfs(MStatfs *statfs) { - //check caps - if(!statfs->get_session()->caps.check_privileges(PAXOS_PGMAP, MON_CAP_R)) { + // check caps + MonSession *session = statfs->get_session(); + if (!session) + goto out; + if (!session->caps.check_privileges(PAXOS_PGMAP, MON_CAP_R)) { dout(0) << "MStatfs received from entity with insufficient privileges " - << statfs->get_session()->caps << dendl; + << session->caps << dendl; goto out; } MStatfsReply *reply; @@ -224,9 +227,12 @@ bool PGMonitor::preprocess_getpoolstats(MGetPoolStats *m) { MGetPoolStatsReply *reply; - if (!m->get_session()->caps.check_privileges(PAXOS_PGMAP, MON_CAP_R)) { + MonSession *session = m->get_session(); + if (!session) + goto out; + if (!session->caps.check_privileges(PAXOS_PGMAP, MON_CAP_R)) { dout(0) << "MGetPoolStats received from entity with insufficient caps " - << m->get_session()->caps << dendl; + << session->caps << dendl; goto out; } @@ -252,7 +258,6 @@ bool PGMonitor::preprocess_getpoolstats(MGetPoolStats *m) out: delete m; - return true; } @@ -261,12 +266,17 @@ bool PGMonitor::preprocess_pg_stats(MPGStats *stats) { int from = stats->get_orig_source().num(); MPGStatsAck *ack; - //check caps - if (!stats->get_session()->caps.check_privileges(PAXOS_PGMAP, MON_CAP_R)) { + + // check caps + MonSession *session = stats->get_session(); + if (!session) + goto out; + if (!session->caps.check_privileges(PAXOS_PGMAP, MON_CAP_R)) { dout(0) << "MPGStats received from entity with insufficient privileges " - << stats->get_session()->caps << dendl; + << session->caps << dendl; goto out; } + // first, just see if they need a new osdmap. but // only if they've had the map for a while. if (stats->had_map_for > 30.0 && -- 2.39.5