From: Kefu Chai Date: Wed, 29 Apr 2015 07:41:08 +0000 (+0800) Subject: common/config: detect overflow of int values X-Git-Tag: v9.0.2~195^2~1 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=d62f80dc7b25d312ff05b65b7be854aae15b66a8;p=ceph.git common/config: detect overflow of int values * #include "strtol.h" in strtol.cc, to ensure the function defintions are consistent. * add a test accordingly * fix the testcase of converting 1024E. * do not accept integers overflow after adding SI suffix * do not accept integers underflow (i.e. negative values) Fixes: #11484 Signed-off-by: Kefu Chai --- diff --git a/src/common/config.cc b/src/common/config.cc index 41bb6a85f8c0..949c9e4073a5 100644 --- a/src/common/config.cc +++ b/src/common/config.cc @@ -885,7 +885,7 @@ int md_config_t::set_val_raw(const char *val, const config_option *opt) switch (opt->type) { case OPT_INT: { std::string err; - int f = strict_sistrtoll(val, &err); + int f = strict_si_cast(val, &err); if (!err.empty()) return -EINVAL; *(int*)opt->conf_ptr(this) = f; @@ -893,7 +893,7 @@ int md_config_t::set_val_raw(const char *val, const config_option *opt) } case OPT_LONGLONG: { std::string err; - long long f = strict_sistrtoll(val, &err); + long long f = strict_si_cast(val, &err); if (!err.empty()) return -EINVAL; *(long long*)opt->conf_ptr(this) = f; @@ -923,7 +923,7 @@ int md_config_t::set_val_raw(const char *val, const config_option *opt) return 0; case OPT_U32: { std::string err; - int f = strict_sistrtoll(val, &err); + int f = strict_si_cast(val, &err); if (!err.empty()) return -EINVAL; *(uint32_t*)opt->conf_ptr(this) = f; @@ -931,7 +931,7 @@ int md_config_t::set_val_raw(const char *val, const config_option *opt) } case OPT_U64: { std::string err; - long long f = strict_sistrtoll(val, &err); + uint64_t f = strict_si_cast(val, &err); if (!err.empty()) return -EINVAL; *(uint64_t*)opt->conf_ptr(this) = f; diff --git a/src/common/strtol.cc b/src/common/strtol.cc index ca15e2fad716..ea39ba0b0f47 100644 --- a/src/common/strtol.cc +++ b/src/common/strtol.cc @@ -12,14 +12,12 @@ * */ +#include "strtol.h" + #include #include #include #include -#include -extern "C" { -#include -} using std::ostringstream; @@ -134,10 +132,8 @@ float strict_strtof(const char *str, std::string *err) uint64_t strict_sistrtoll(const char *str, std::string *err) { std::string s(str); - if (s.size() == 0) { - ostringstream oss; - oss << "strict_sistrtoll: value not specified"; - *err = oss.str(); + if (s.empty()) { + *err = "strict_sistrtoll: value not specified"; return 0; } const char &u = s.at(s.size()-1); //str[std::strlen(str)-1]; @@ -164,9 +160,25 @@ uint64_t strict_sistrtoll(const char *str, std::string *err) s = std::string(str, s.size()-1); v = s.c_str(); - uint64_t r = strict_strtoll(v, 10, err); + long long r_ll = strict_strtoll(v, 10, err); + + if (r_ll < 0) { + *err = "strict_sistrtoll: value should not be negative"; + return 0; + } + + uint64_t r = r_ll; if (err->empty() && m > 0) { - r = (r << m); + if (r > (std::numeric_limits::max() >> m)) { + *err = "strict_sistrtoll: value seems to be too large"; + return 0; + } + r <<= m; } return r; } + +template <> +uint64_t strict_si_cast(const char *str, std::string *err) { + return strict_sistrtoll(str, err); +} diff --git a/src/common/strtol.h b/src/common/strtol.h index ea0a469a777e..5575ed7b390b 100644 --- a/src/common/strtol.h +++ b/src/common/strtol.h @@ -16,6 +16,7 @@ #define CEPH_COMMON_STRTOL_H #include +#include extern "C" { #include } @@ -30,4 +31,21 @@ float strict_strtof(const char *str, std::string *err); uint64_t strict_sistrtoll(const char *str, std::string *err); +template +Target strict_si_cast(const char *str, std::string *err) { + uint64_t ret = strict_sistrtoll(str, err); + if (!err->empty()) + return ret; + if (ret > (uint64_t)std::numeric_limits::max()) { + err->append("The option value '"); + err->append(str); + err->append("' seems to be too large"); + return 0; + } + return ret; +} + +template <> +uint64_t strict_si_cast(const char *str, std::string *err); + #endif diff --git a/src/test/daemon_config.cc b/src/test/daemon_config.cc index 1361ddc22477..2968b3514a2f 100644 --- a/src/test/daemon_config.cc +++ b/src/test/daemon_config.cc @@ -23,6 +23,9 @@ #include #include +#include + + using std::string; TEST(DaemonConfig, SimpleSet) { @@ -333,6 +336,30 @@ TEST(DaemonConfig, ThreadSafety1) { "false")); ASSERT_EQ(ret, 0); } + +TEST(DaemonConfig, InvalidIntegers) { + { + int ret = g_ceph_context->_conf->set_val("num_client", "-1"); + ASSERT_EQ(ret, -EINVAL); + } + { + int ret = g_ceph_context->_conf->set_val("num_client", "-1K"); + ASSERT_EQ(ret, -EINVAL); + } + { + long long bad_value = (long long)std::numeric_limits::max() + 1; + string str = boost::lexical_cast(bad_value); + int ret = g_ceph_context->_conf->set_val("num_client", str); + ASSERT_EQ(ret, -EINVAL); + } + { + // 4G must be greater than INT_MAX + ASSERT_GT(4LL * 1024 * 1024 * 1024, (long long)std::numeric_limits::max()); + int ret = g_ceph_context->_conf->set_val("num_client", "4G"); + ASSERT_EQ(ret, -EINVAL); + } +} + /* * Local Variables: * compile-command: "cd .. ; make unittest_daemon_config && ./unittest_daemon_config" diff --git a/src/test/strtol.cc b/src/test/strtol.cc index 08ba081ac2bb..93d6e68fb829 100644 --- a/src/test/strtol.cc +++ b/src/test/strtol.cc @@ -174,7 +174,8 @@ TEST(SIStrToLL, WithUnits) { for (std::map::iterator p = units.begin(); p != units.end(); ++p) { - test_strict_sistrtoll_units("1024", p->first, p->second); + // the upper bound of uint64_t is 2^64 = 4E + test_strict_sistrtoll_units("4", p->first, p->second); test_strict_sistrtoll_units("1", p->first, p->second); test_strict_sistrtoll_units("0", p->first, p->second); } @@ -208,4 +209,14 @@ TEST(SIStrToLL, Error) { test_strict_sistrtoll_err("BM"); test_strict_sistrtoll_err("B0wef"); test_strict_sistrtoll_err("0m"); + test_strict_sistrtoll_err("-1"); // it returns uint64_t + test_strict_sistrtoll_err("-1K"); + // the upper bound of uint64_t is 2^64 = 4E, so 1024E overflows + test_strict_sistrtoll_err("1024E"); // overflows after adding the suffix } + +/* + * Local Variables: + * compile-command: "cd .. ; make unittest_strtol && ./unittest_strtol" + * End: + */