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

diff --git a/src/messages/PaxosServiceMessage.h b/src/messages/PaxosServiceMessage.h
index 1b670bfa483dcb6308624643dc85662305c39ce8..88695f3d7536f3f16700bf525747a9a48b99a656 100644 (file)
--- 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 1cd6722bf562a8c083072ebca51e92cd1a634bdd..28b1f54e1e651e4d62d6ff14c6af13c3124ec527 100644 (file)
--- 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<LogEntry>::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 850da42b43081e7825b3c96cac8f5d2c35914095..f7ebeab1e903bf2c49ec0e3b1aa90a72463d2f14 100644 (file)
--- 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 8f61667e9c0dc17aff0cd8d557d86a987db9488e..9475c767fc47993e5eaac2e89090f0fd471acd13 100644 (file)
--- 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 10126515613b2ea62992774095c6f98433fa9705..3f7c82cb3c5876ef320968e3037f212e4bbc0198 100644 (file)
--- 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<int> 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<int> empty;
+
   for (map<pg_t,vector<int> >::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<int, vector<snapid_t> >::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 844398b75dc09598aa1adb74831a0ecf1b738c64..054e754f851375344d3f0fcbc736d6d54ba30b56 100644 (file)
--- 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 &&