[PATCH v2 1/3] config: use unsigned_mult_overflows to check for overflows
- From
René Scharfe <l.s.r@web.de>
- Date
- Jun 22, 2019, 10:03 UTC
- Message-ID
- <7a580777-2ccc-6097-477a-e66eb28d5f9b@web.de>
- In-Reply-To
- <xmqq7e9ltbe2.fsf@gitster-ct.c.googlers.com>
parse_unit_factor() checks if a K, M or G is present after a number and multiplies it by 2^10, 2^20 or 2^30, respectively. One of its callers checks if the result is smaller than the number alone to detect overflows. The other one passes 1 as the number and does multiplication and overflow check itself in a similar manner.
This works, but is inconsistent, and it would break if we added support for a bigger unit factor. E.g. 16777217T is 2^64 + 2^40, i.e. too big for a 64-bit number. Modulo 2^64 we get 2^40 == 1TB, which is bigger than the raw number 16777217 == 2^24 + 1, so the overflow would go undetected by that method.
Let both callers pass 1 and handle overflow check and multiplication themselves. Do the check before the multiplication, using unsigned_mult_overflows, which is simpler and can deal with larger unit factors.
Signed-off-by: Rene Scharfe <l.s.r@web.de> --- Patch generated with --function-context for easier review (e.g. to see why we can stop updating uval in place).
config.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/config.c b/config.c index 01c6e9df23..3c00369ba8 100644 --- a/config.c +++ b/config.c @@ -856,29 +856,29 @@ static int parse_unit_factor(const char *end, uintmax_t *val) static int git_parse_signed(const char *value, intmax_t *ret, intmax_t max) { if (value && *value) { char *end; intmax_t val; uintmax_t uval; uintmax_t factor = 1; errno = 0; val = strtoimax(value, &end, 0); if (errno == ERANGE) return 0; if (!parse_unit_factor(end, &factor)) { errno = EINVAL; return 0; } uval = val < 0 ? -val : val; - uval *= factor; - if (uval > max || (val < 0 ? -val : val) > uval) { + if (unsigned_mult_overflows(factor, uval) || + factor * uval > max) { errno = ERANGE; return 0; } val *= factor; *ret = val; return 1; } errno = EINVAL; return 0; } @@ -886,26 +886,27 @@ static int git_parse_signed(const char *value, intmax_t *ret, intmax_t max) static int git_parse_unsigned(const char *value, uintmax_t *ret, uintmax_t max) { if (value && *value) { char *end; uintmax_t val; - uintmax_t oldval; + uintmax_t factor = 1; errno = 0; val = strtoumax(value, &end, 0); if (errno == ERANGE) return 0; - oldval = val; - if (!parse_unit_factor(end, &val)) { + if (!parse_unit_factor(end, &factor)) { errno = EINVAL; return 0; } - if (val > max || oldval > val) { + if (unsigned_mult_overflows(factor, val) || + factor * val > max) { errno = ERANGE; return 0; } + val *= factor; *ret = val; return 1; } errno = EINVAL; return 0; } -- 2.22.0