diff --git a/src/basic/parse-util.c b/src/basic/parse-util.c index f882124a0e..f72029c67f 100644 --- a/src/basic/parse-util.c +++ b/src/basic/parse-util.c @@ -779,16 +779,17 @@ int parse_oom_score_adjust(const char *s, int *ret) { int store_loadavg_fixed_point(unsigned long i, unsigned long f, loadavg_t *ret) { assert(ret); - if (i >= (~0UL << LOADAVG_PRECISION_BITS)) + /* Validate both parts *before* shifting them, so that the shifts themselves cannot overflow. + * Rejecting f >= 100 is equivalent to the old check of the shifted value against + * LOADAVG_FIXED_POINT_1_0, since DIV_ROUND_UP(99 << 11, 100) == 2028 and + * DIV_ROUND_UP(100 << 11, 100) == 2048. */ + if (i > (~0UL >> LOADAVG_PRECISION_BITS)) return -ERANGE; - i = i << LOADAVG_PRECISION_BITS; - f = DIV_ROUND_UP((f << LOADAVG_PRECISION_BITS), 100); - - if (f >= LOADAVG_FIXED_POINT_1_0) + if (f >= 100) return -ERANGE; - *ret = i | f; + *ret = (i << LOADAVG_PRECISION_BITS) | DIV_ROUND_UP(f << LOADAVG_PRECISION_BITS, 100); return 0; } diff --git a/src/test/test-parse-util.c b/src/test/test-parse-util.c index bc74856fde..cb3fafa94a 100644 --- a/src/test/test-parse-util.c +++ b/src/test/test-parse-util.c @@ -7,6 +7,8 @@ #include "capability-util.h" #include "locale-util.h" #include "parse-util.h" +#include "stdio-util.h" +#include "string-util.h" #include "tests.h" TEST(parse_boolean) { @@ -864,6 +866,26 @@ TEST(parse_loadavg_fixed_point) { ASSERT_ERROR(parse_loadavg_fixed_point("4096.4096", &fp), ERANGE); ASSERT_ERROR(parse_loadavg_fixed_point("-4000.5", &fp), ERANGE); ASSERT_ERROR(parse_loadavg_fixed_point("18446744073709551615.5", &fp), ERANGE); + + /* Neither part may be large enough to overflow the shift by LOADAVG_PRECISION_BITS. Derive the + * boundary from the width of unsigned long, so that this holds on 32 bit too: the largest integer + * part that still fits is ~0UL >> LOADAVG_PRECISION_BITS (2^53-1 on 64 bit), and one more than + * that used to shift clean out of the word and be reported as a load of zero. */ + unsigned long max_side = ~0UL >> LOADAVG_PRECISION_BITS; + char buf[DECIMAL_STR_MAX(unsigned long) + STRLEN(".99")]; + + xsprintf(buf, "%lu.99", max_side); + ASSERT_OK_ZERO(parse_loadavg_fixed_point(buf, &fp)); + ASSERT_EQ(LOADAVG_INT_SIDE(fp), max_side); + ASSERT_EQ(LOADAVG_DECIMAL_SIDE(fp), 99U); + + xsprintf(buf, "%lu.00", max_side + 1); + ASSERT_ERROR(parse_loadavg_fixed_point(buf, &fp), ERANGE); + + /* Same for the fractional part, which used to be shifted before it was range checked. */ + xsprintf(buf, "1.%lu", max_side + 1); + ASSERT_ERROR(parse_loadavg_fixed_point(buf, &fp), ERANGE); + ASSERT_ERROR(parse_loadavg_fixed_point("foobar", &fp), EINVAL); ASSERT_ERROR(parse_loadavg_fixed_point("3333", &fp), EINVAL); ASSERT_ERROR(parse_loadavg_fixed_point("1.2.3", &fp), EINVAL);