Project

General

Profile

Bug #13829

Updated by Kefu Chai about 7 years ago

In the filestore-config-ref we say...

> filestore merge threshold
> Description: Min number of files in a subdir before merging into parent NOTE: A > negative value means to disable subdir merging

But running the following on 0.94.3 (95cefea9fd9ab740263bf8bb4796fd864d9afe2b) gives an error.

<pre>
# ceph --admin-daemon /var/run/ceph/ceph-osd.312.asok config set filestore_merge_threshold -40
{
"error": "error setting 'filestore_merge_threshold' to '-40': (22) Invalid argument"
}
</pre>


0.94.1 (e4bfad3a3c51054df7e537a724c8d0bf9be972ff) is not affected so I did a git bisect to find the commit that introduced this behavior.

<pre>
$ git bisect bad
9b947fa320b77e0055a581005353c2561a12a198 is the first bad commit
commit 9b947fa320b77e0055a581005353c2561a12a198
Author: Kefu Chai <kchai@redhat.com>
Date: Wed Apr 29 15:41:08 2015 +0800

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
</pre>


That patch includes the following code to src/common/strtol.cc.

<pre>
163 long long r_ll = strict_strtoll(v, 10, err);
164
165 if (r_ll < 0) {
166 *err = "strict_sistrtoll: value should not be negative";
167 return 0;
168 }
169
170 uint64_t r = r_ll;
171 if (err->empty() && m > 0) {
172 if (r > (std::numeric_limits<uint64_t>::max() >> m)) {
173 *err = "strict_sistrtoll: value seems to be too large";
174 return 0;
175 }
176 r <<= m;
177 }
</pre>


and this to @src/common/strtol.h@

<pre>
src/common/strtol.h

34 template <typename Target>
35 Target strict_si_cast(const char *str, std::string *err) {
36 uint64_t ret = strict_sistrtoll(str, err);
37 if (!err->empty())
38 return ret;
39 if (ret > (uint64_t)std::numeric_limits<Target>::max()) {
40 err->append("The option value '");
41 err->append(str);
42 err->append("' seems to be too large");
43 return 0;
44 }
45 return ret;
46 }
</pre>


Negative values fail both the test for a negative value (of course) and the tests
for uint64_t overflow.

Working with kefu on a patch so assigning this to myself.

Back