parse-util: fix ineffective overflow guards in store_loadavg_fixed_point()

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;
 }
 

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);