From: Joao Eduardo Luis Date: Fri, 22 Nov 2013 20:58:57 +0000 (+0000) Subject: mon: Properly handle errors from 'cmd_getval()' when needed X-Git-Tag: v0.78~7^2~2 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=652056edc04a36bc41320347f386fcbccb5b0401;p=ceph.git mon: Properly handle errors from 'cmd_getval()' when needed Not handling the error return from cmd_getval() may leave uninitialzied values, which can cause issues, specially with non-string values. Fixes: 6806 Signed-off-by: Joao Eduardo Luis --- diff --git a/src/mon/MDSMonitor.cc b/src/mon/MDSMonitor.cc index 1e81eba9ad8..d25b0cb7778 100644 --- a/src/mon/MDSMonitor.cc +++ b/src/mon/MDSMonitor.cc @@ -919,10 +919,19 @@ bool MDSMonitor::prepare_command(MMonCommand *m) } else if (prefix == "mds set_state") { int64_t gid; - if (!cmd_getval(g_ceph_context, cmdmap, "gid", gid)) + if (!cmd_getval(g_ceph_context, cmdmap, "gid", gid)) { + ss << "error parsing 'gid' value '" + << cmd_vartype_stringify(cmdmap["gid"]) << "'"; + r = -EINVAL; goto out; + } int64_t state; - cmd_getval(g_ceph_context, cmdmap, "state", state); + if (!cmd_getval(g_ceph_context, cmdmap, "state", state)) { + ss << "error parsing 'state' string value '" + << cmd_vartype_stringify(cmdmap["state"]) << "'"; + r = -EINVAL; + goto out; + } if (!pending_mdsmap.is_dne_gid(gid)) { MDSMap::mds_info_t& info = pending_mdsmap.get_info_gid(gid); info.state = state; @@ -946,7 +955,12 @@ bool MDSMonitor::prepare_command(MMonCommand *m) } else if (prefix == "mds rm") { int64_t gid; - cmd_getval(g_ceph_context, cmdmap, "gid", gid); + if (!cmd_getval(g_ceph_context, cmdmap, "gid", gid)) { + ss << "error parsing 'gid' value '" + << cmd_vartype_stringify(cmdmap["gid"]) << "'"; + r = -EINVAL; + goto out; + } int state = pending_mdsmap.get_state_gid(gid); if (state == 0) { ss << "mds gid " << gid << " dne"; @@ -967,7 +981,12 @@ bool MDSMonitor::prepare_command(MMonCommand *m) } } else if (prefix == "mds rmfailed") { int64_t w; - cmd_getval(g_ceph_context, cmdmap, "who", w); + if (!cmd_getval(g_ceph_context, cmdmap, "who", w)) { + ss << "error parsing 'who' value '" + << cmd_vartype_stringify(cmdmap["who"]) << "'"; + r = -EINVAL; + goto out; + } pending_mdsmap.failed.erase(w); stringstream ss; ss << "removed failed mds." << w; @@ -994,7 +1013,12 @@ bool MDSMonitor::prepare_command(MMonCommand *m) r = 0; } else if (prefix == "mds compat rm_compat") { int64_t f; - cmd_getval(g_ceph_context, cmdmap, "feature", f); + if (!cmd_getval(g_ceph_context, cmdmap, "feature", f)) { + ss << "error parsing feature value '" + << cmd_vartype_stringify(cmdmap["feature"]) << "'"; + r = -EINVAL; + goto out; + } if (pending_mdsmap.compat.compat.contains(f)) { ss << "removing compat feature " << f; pending_mdsmap.compat.compat.remove(f); @@ -1005,7 +1029,12 @@ bool MDSMonitor::prepare_command(MMonCommand *m) } } else if (prefix == "mds compat rm_incompat") { int64_t f; - cmd_getval(g_ceph_context, cmdmap, "feature", f); + if (!cmd_getval(g_ceph_context, cmdmap, "feature", f)) { + ss << "error parsing feature value '" + << cmd_vartype_stringify(cmdmap["feature"]) << "'"; + r = -EINVAL; + goto out; + } if (pending_mdsmap.compat.incompat.contains(f)) { ss << "removing incompat feature " << f; pending_mdsmap.compat.incompat.remove(f); @@ -1064,8 +1093,18 @@ bool MDSMonitor::prepare_command(MMonCommand *m) } else if (prefix == "mds newfs") { MDSMap newmap; int64_t metadata, data; - cmd_getval(g_ceph_context, cmdmap, "metadata", metadata); - cmd_getval(g_ceph_context, cmdmap, "data", data); + if (!cmd_getval(g_ceph_context, cmdmap, "metadata", metadata)) { + ss << "error parsing 'metadata' value '" + << cmd_vartype_stringify(cmdmap["metadata"]) << "'"; + r = -EINVAL; + goto out; + } + if (!cmd_getval(g_ceph_context, cmdmap, "data", data)) { + ss << "error parsing 'data' value '" + << cmd_vartype_stringify(cmdmap["data"]) << "'"; + r = -EINVAL; + goto out; + } string sure; cmd_getval(g_ceph_context, cmdmap, "sure", sure); if (sure != "--yes-i-really-mean-it") { diff --git a/src/mon/Monitor.cc b/src/mon/Monitor.cc index a2d11120625..62533c53dd0 100644 --- a/src/mon/Monitor.cc +++ b/src/mon/Monitor.cc @@ -776,7 +776,11 @@ void Monitor::_osdmonitor_prepare_command(cmdmap_t& cmdmap, ostream& ss) void Monitor::_add_bootstrap_peer_hint(string cmd, cmdmap_t& cmdmap, ostream& ss) { string addrstr; - cmd_getval(g_ceph_context, cmdmap, "addr", addrstr); + if (!cmd_getval(g_ceph_context, cmdmap, "addr", addrstr)) { + ss << "unable to parse address string value '" + << cmd_vartype_stringify(cmdmap["addr"]) << "'"; + return; + } dout(10) << "_add_bootstrap_peer_hint '" << cmd << "' '" << addrstr << "'" << dendl; diff --git a/src/mon/OSDMonitor.cc b/src/mon/OSDMonitor.cc index b03144144a6..e757cfd798b 100644 --- a/src/mon/OSDMonitor.cc +++ b/src/mon/OSDMonitor.cc @@ -2209,7 +2209,12 @@ bool OSDMonitor::preprocess_command(MMonCommand *m) } } else if (prefix == "osd find") { int64_t osd; - cmd_getval(g_ceph_context, cmdmap, "id", osd); + if (!cmd_getval(g_ceph_context, cmdmap, "id", osd)) { + ss << "unable to parse osd id value '" + << cmd_vartype_stringify(cmdmap["id"]) << "'"; + r = -EINVAL; + goto reply; + } if (!osdmap.exists(osd)) { ss << "osd." << osd << " does not exist"; r = -ENOENT; @@ -2231,7 +2236,12 @@ bool OSDMonitor::preprocess_command(MMonCommand *m) f->flush(rdata); } else if (prefix == "osd metadata") { int64_t osd; - cmd_getval(g_ceph_context, cmdmap, "id", osd); + if (!cmd_getval(g_ceph_context, cmdmap, "id", osd)) { + ss << "unable to parse osd id value '" + << cmd_vartype_stringify(cmdmap["id"]) << "'"; + r = -EINVAL; + goto reply; + } if (!osdmap.exists(osd)) { ss << "osd." << osd << " does not exist"; r = -ENOENT; @@ -3484,7 +3494,12 @@ bool OSDMonitor::prepare_command_impl(MMonCommand *m, } double weight; - cmd_getval(g_ceph_context, cmdmap, "weight", weight); + if (!cmd_getval(g_ceph_context, cmdmap, "weight", weight)) { + ss << "unable to parse weight value '" + << cmd_vartype_stringify(cmdmap["weight"]) << "'"; + err = -EINVAL; + goto reply; + } string args; vector argvec; @@ -3547,7 +3562,12 @@ bool OSDMonitor::prepare_command_impl(MMonCommand *m, } double weight; - cmd_getval(g_ceph_context, cmdmap, "weight", weight); + if (!cmd_getval(g_ceph_context, cmdmap, "weight", weight)) { + ss << "unable to parse weight value '" + << cmd_vartype_stringify(cmdmap["weight"]) << "'"; + err = -EINVAL; + goto reply; + } string args; vector argvec; @@ -3741,7 +3761,12 @@ bool OSDMonitor::prepare_command_impl(MMonCommand *m, break; } double w; - cmd_getval(g_ceph_context, cmdmap, "weight", w); + if (!cmd_getval(g_ceph_context, cmdmap, "weight", w)) { + ss << "unable to parse weight value '" + << cmd_vartype_stringify(cmdmap["weight"]) << "'"; + err = -EINVAL; + break; + } err = newcrush.adjust_item_weightf(g_ceph_context, id, w); if (err >= 0) { @@ -3917,7 +3942,12 @@ bool OSDMonitor::prepare_command_impl(MMonCommand *m, } else if (prefix == "osd setmaxosd") { int64_t newmax; - cmd_getval(g_ceph_context, cmdmap, "newmax", newmax); + if (!cmd_getval(g_ceph_context, cmdmap, "newmax", newmax)) { + ss << "unable to parse 'newmax' value '" + << cmd_vartype_stringify(cmdmap["newmax"]) << "'"; + err = -EINVAL; + goto reply; + } if (newmax > g_conf->mon_max_osd) { err = -ERANGE; @@ -4063,12 +4093,18 @@ bool OSDMonitor::prepare_command_impl(MMonCommand *m, } else if (prefix == "osd primary-affinity") { int64_t id; if (!cmd_getval(g_ceph_context, cmdmap, "id", id)) { - ss << "invalid osd id"; + ss << "invalid osd id value '" + << cmd_vartype_stringify(cmdmap["id"]) << "'"; err = -EINVAL; goto reply; } double w; - cmd_getval(g_ceph_context, cmdmap, "weight", w); + if (!cmd_getval(g_ceph_context, cmdmap, "weight", w)) { + ss << "unable to parse 'weight' value '" + << cmd_vartype_stringify(cmdmap["weight"]) << "'"; + err = -EINVAL; + goto reply; + } long ww = (int)((double)CEPH_OSD_MAX_PRIMARY_AFFINITY*w); if (ww < 0L) { ss << "weight must be >= 0"; @@ -4094,9 +4130,19 @@ bool OSDMonitor::prepare_command_impl(MMonCommand *m, } } else if (prefix == "osd reweight") { int64_t id; - cmd_getval(g_ceph_context, cmdmap, "id", id); + if (!cmd_getval(g_ceph_context, cmdmap, "id", id)) { + ss << "unable to parse osd id value '" + << cmd_vartype_stringify(cmdmap["id"]) << "'"; + err = -EINVAL; + goto reply; + } double w; - cmd_getval(g_ceph_context, cmdmap, "weight", w); + if (!cmd_getval(g_ceph_context, cmdmap, "weight", w)) { + ss << "unable to parse weight value '" + << cmd_vartype_stringify(cmdmap["weight"]) << "'"; + err = -EINVAL; + goto reply; + } long ww = (int)((double)CEPH_OSD_IN*w); if (ww < 0L) { ss << "weight must be >= 0"; @@ -4114,7 +4160,12 @@ bool OSDMonitor::prepare_command_impl(MMonCommand *m, } else if (prefix == "osd lost") { int64_t id; - cmd_getval(g_ceph_context, cmdmap, "id", id); + if (!cmd_getval(g_ceph_context, cmdmap, "id", id)) { + ss << "unable to parse osd id value '" + << cmd_vartype_stringify(cmdmap["id"]) << "'"; + err = -EINVAL; + goto reply; + } string sure; if (!cmd_getval(g_ceph_context, cmdmap, "sure", sure) || sure != "--yes-i-really-mean-it") { ss << "are you SURE? this might mean real, permanent data loss. pass " @@ -4725,7 +4776,12 @@ done: goto reply; } int64_t size = 0; - cmd_getval(g_ceph_context, cmdmap, "size", size); + if (!cmd_getval(g_ceph_context, cmdmap, "size", size)) { + ss << "unable to parse 'size' value '" + << cmd_vartype_stringify(cmdmap["size"]) << "'"; + err = -EINVAL; + goto reply; + } // make sure new tier is empty const pool_stat_t& tier_stats = mon->pgmon()->pg_map.get_pg_pool_sum_stat(tierpool_id); diff --git a/src/mon/PGMonitor.cc b/src/mon/PGMonitor.cc index 567be518938..5adf5357d1e 100644 --- a/src/mon/PGMonitor.cc +++ b/src/mon/PGMonitor.cc @@ -1660,7 +1660,12 @@ bool PGMonitor::prepare_command(MMonCommand *m) } else if (prefix == "pg set_full_ratio" || prefix == "pg set_nearfull_ratio") { double n; - cmd_getval(g_ceph_context, cmdmap, "ratio", n); + if (!cmd_getval(g_ceph_context, cmdmap, "ratio", n)) { + ss << "unable to parse 'ratio' value '" + << cmd_vartype_stringify(cmdmap["who"]) << "'"; + r = -EINVAL; + goto reply; + } string op = prefix.substr(3, string::npos); if (op == "set_full_ratio") pending_inc.full_ratio = n;