From 9b947fa320b77e0055a581005353c2561a12a198 Mon Sep 17 00:00:00 2001 From: Kefu Chai Date: Wed, 29 Apr 2015 15:41:08 +0800 Subject: [PATCH] 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 (cherry picked from commit d62f80dc7b25d312ff05b65b7be854aae15b66a8) --- src/common/config.cc | 8 ++++---- src/common/strtol.cc | 32 ++++++++++++++++++++++---------- src/common/strtol.h | 18 ++++++++++++++++++ src/test/daemon_config.cc | 27 +++++++++++++++++++++++++++ src/test/strtol.cc | 13 ++++++++++++- 5 files changed, 83 insertions(+), 15 deletions(-) diff --git a/src/common/config.cc b/src/common/config.cc index 88f443d86e06c..f2460c636e279 100644 --- a/src/common/config.cc +++ b/src/common/config.cc @@ -877,7 +877,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; @@ -885,7 +885,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; @@ -915,7 +915,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; @@ -923,7 +923,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 840b3d9c6a046..8a43eb56be2a7 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; @@ -131,10 +129,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]; @@ -161,9 +157,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 ea0a469a777e6..5575ed7b390be 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 1361ddc22477c..2968b3514a2fa 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 08ba081ac2bbf..93d6e68fb8295 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: + */ -- 2.39.5