]> git.ipfire.org Git - thirdparty/lxc.git/commitdiff
string_utils: handle overflow correct in parse_byte_size_string() 3743/head
authorChristian Brauner <christian.brauner@ubuntu.com>
Sat, 27 Mar 2021 20:14:02 +0000 (21:14 +0100)
committerChristian Brauner <christian.brauner@ubuntu.com>
Sun, 28 Mar 2021 09:55:13 +0000 (11:55 +0200)
This takes the overflow handling code from the kernel.

Link: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=32549
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
src/lxc/compiler.h
src/lxc/confile.c
src/lxc/string_utils.c
src/lxc/string_utils.h
src/tests/lxc-test-utils.c

index 81cab5c43b15fb29fe77c2b9b94d06a994718035..5d45955d0afa4ab6d1498e494351530edc5d81d1 100644 (file)
@@ -7,6 +7,9 @@
 #define _GNU_SOURCE 1
 #endif
 
+#include <stdbool.h>
+#include <linux/types.h>
+
 #include "config.h"
 
 #ifndef thread_local
 #define __fallthrough /* fall through */
 #endif
 
+#if defined(__GNUC__) && !defined(__clang__)
+       #if GCC_VERSION >= 50100
+       #define COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW 1
+       #endif
+#endif
+
+#define likely(x) __builtin_expect(!!(x), 1)
+#define unlikely(x) __builtin_expect(!!(x), 0)
+#define __must_check __attribute__((__warn_unused_result__))
+
+static inline bool __must_check __must_check_overflow(bool overflow)
+{
+       return unlikely(overflow);
+}
+
+#define is_signed_type(type)       (((type)(-1)) < (type)1)
+#define __type_half_max(type) ((type)1 << (8*sizeof(type) - 1 - is_signed_type(type)))
+#define type_max(T) ((T)((__type_half_max(T) - 1) + __type_half_max(T)))
+#define type_min(T) ((T)((T)-type_max(T)-(T)1))
+
+/*
+ * Avoids triggering -Wtype-limits compilation warning,
+ * while using unsigned data types to check a < 0.
+ */
+#define is_non_negative(a) ((a) > 0 || (a) == 0)
+#define is_negative(a) (!(is_non_negative(a)))
+
+#ifdef COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW
+/*
+ * For simplicity and code hygiene, the fallback code below insists on
+ * a, b and *d having the same type (similar to the min() and max()
+ * macros), whereas gcc's type-generic overflow checkers accept
+ * different types. Hence we don't just make check_add_overflow an
+ * alias for __builtin_add_overflow, but add type checks similar to
+ * below.
+ */
+#define check_add_overflow(a, b, d) __must_check_overflow(({   \
+       typeof(a) __a = (a);                    \
+       typeof(b) __b = (b);                    \
+       typeof(d) __d = (d);                    \
+       (void) (&__a == &__b);                  \
+       (void) (&__a == __d);                   \
+       __builtin_add_overflow(__a, __b, __d);  \
+}))
+
+#define check_sub_overflow(a, b, d) __must_check_overflow(({   \
+       typeof(a) __a = (a);                    \
+       typeof(b) __b = (b);                    \
+       typeof(d) __d = (d);                    \
+       (void) (&__a == &__b);                  \
+       (void) (&__a == __d);                   \
+       __builtin_sub_overflow(__a, __b, __d);  \
+}))
+
+#define check_mul_overflow(a, b, d) __must_check_overflow(({   \
+       typeof(a) __a = (a);                    \
+       typeof(b) __b = (b);                    \
+       typeof(d) __d = (d);                    \
+       (void) (&__a == &__b);                  \
+       (void) (&__a == __d);                   \
+       __builtin_mul_overflow(__a, __b, __d);  \
+}))
+#else /* !COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW */
+
+/* Checking for unsigned overflow is relatively easy without causing UB. */
+#define __unsigned_add_overflow(a, b, d) ({    \
+       typeof(a) __a = (a);                    \
+       typeof(b) __b = (b);                    \
+       typeof(d) __d = (d);                    \
+       (void) (&__a == &__b);                  \
+       (void) (&__a == __d);                   \
+       *__d = __a + __b;                       \
+       *__d < __a;                             \
+})
+#define __unsigned_sub_overflow(a, b, d) ({    \
+       typeof(a) __a = (a);                    \
+       typeof(b) __b = (b);                    \
+       typeof(d) __d = (d);                    \
+       (void) (&__a == &__b);                  \
+       (void) (&__a == __d);                   \
+       *__d = __a - __b;                       \
+       __a < __b;                              \
+})
+
+/*
+ * If one of a or b is a compile-time constant, this avoids a division.
+ */
+#define __unsigned_mul_overflow(a, b, d) ({            \
+       typeof(a) __a = (a);                            \
+       typeof(b) __b = (b);                            \
+       typeof(d) __d = (d);                            \
+       (void) (&__a == &__b);                          \
+       (void) (&__a == __d);                           \
+       *__d = __a * __b;                               \
+       __builtin_constant_p(__b) ?                     \
+         __b > 0 && __a > type_max(typeof(__a)) / __b : \
+         __a > 0 && __b > type_max(typeof(__b)) / __a;  \
+})
+
+/*
+ * For signed types, detecting overflow is much harder, especially if
+ * we want to avoid UB. But the interface of these macros is such that
+ * we must provide a result in *d, and in fact we must produce the
+ * result promised by gcc's builtins, which is simply the possibly
+ * wrapped-around value. Fortunately, we can just formally do the
+ * operations in the widest relevant unsigned type (u64) and then
+ * truncate the result - gcc is smart enough to generate the same code
+ * with and without the (u64) casts.
+ */
+
+/*
+ * Adding two signed integers can overflow only if they have the same
+ * sign, and overflow has happened iff the result has the opposite
+ * sign.
+ */
+#define __signed_add_overflow(a, b, d) ({      \
+       typeof(a) __a = (a);                    \
+       typeof(b) __b = (b);                    \
+       typeof(d) __d = (d);                    \
+       (void) (&__a == &__b);                  \
+       (void) (&__a == __d);                   \
+       *__d = (__u64)__a + (__u64)__b;         \
+       (((~(__a ^ __b)) & (*__d ^ __a))        \
+               & type_min(typeof(__a))) != 0;  \
+})
+
+/*
+ * Subtraction is similar, except that overflow can now happen only
+ * when the signs are opposite. In this case, overflow has happened if
+ * the result has the opposite sign of a.
+ */
+#define __signed_sub_overflow(a, b, d) ({      \
+       typeof(a) __a = (a);                    \
+       typeof(b) __b = (b);                    \
+       typeof(d) __d = (d);                    \
+       (void) (&__a == &__b);                  \
+       (void) (&__a == __d);                   \
+       *__d = (__u64)__a - (__u64)__b;         \
+       ((((__a ^ __b)) & (*__d ^ __a))         \
+               & type_min(typeof(__a))) != 0;  \
+})
+
+/*
+ * Signed multiplication is rather hard. gcc always follows C99, so
+ * division is truncated towards 0. This means that we can write the
+ * overflow check like this:
+ *
+ * (a > 0 && (b > MAX/a || b < MIN/a)) ||
+ * (a < -1 && (b > MIN/a || b < MAX/a) ||
+ * (a == -1 && b == MIN)
+ *
+ * The redundant casts of -1 are to silence an annoying -Wtype-limits
+ * (included in -Wextra) warning: When the type is u8 or u16, the
+ * __b_c_e in check_mul_overflow obviously selects
+ * __unsigned_mul_overflow, but unfortunately gcc still parses this
+ * code and warns about the limited range of __b.
+ */
+
+#define __signed_mul_overflow(a, b, d) ({                              \
+       typeof(a) __a = (a);                                            \
+       typeof(b) __b = (b);                                            \
+       typeof(d) __d = (d);                                            \
+       typeof(a) __tmax = type_max(typeof(a));                         \
+       typeof(a) __tmin = type_min(typeof(a));                         \
+       (void) (&__a == &__b);                                          \
+       (void) (&__a == __d);                                           \
+       *__d = (__u64)__a * (__u64)__b;                                 \
+       (__b > 0   && (__a > __tmax/__b || __a < __tmin/__b)) ||        \
+       (__b < (typeof(__b))-1  && (__a > __tmin/__b || __a < __tmax/__b)) || \
+       (__b == (typeof(__b))-1 && __a == __tmin);                      \
+})
+
+
+#define check_add_overflow(a, b, d)    __must_check_overflow(          \
+       __builtin_choose_expr(is_signed_type(typeof(a)),                \
+                       __signed_add_overflow(a, b, d),                 \
+                       __unsigned_add_overflow(a, b, d)))
+
+#define check_sub_overflow(a, b, d)    __must_check_overflow(          \
+       __builtin_choose_expr(is_signed_type(typeof(a)),                \
+                       __signed_sub_overflow(a, b, d),                 \
+                       __unsigned_sub_overflow(a, b, d)))
+
+#define check_mul_overflow(a, b, d)    __must_check_overflow(          \
+       __builtin_choose_expr(is_signed_type(typeof(a)),                \
+                       __signed_mul_overflow(a, b, d),                 \
+                       __unsigned_mul_overflow(a, b, d)))
+
+#endif /* COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW */
+
 #ifndef __noreturn
 #      if __STDC_VERSION__ >= 201112L
 #              if !IS_BIONIC
 #define __public __attribute__((visibility("default")))
 #endif
 
-#define likely(x) __builtin_expect(!!(x), 1)
-#define unlikely(x) __builtin_expect(!!(x), 0)
-
 #endif /* __LXC_COMPILER_H */
index 1decb0bc58d4a9fe45e6c1fbde10eebee7f54f5a..59fc5704d000e9e99139507e9bc2bcc4c5f0e4f4 100644 (file)
@@ -2421,7 +2421,7 @@ static int set_config_console_buffer_size(const char *key, const char *value,
                                          struct lxc_conf *lxc_conf, void *data)
 {
        int ret;
-       int64_t size;
+       long long int size;
        uint64_t buffer_size, pgsz;
 
        if (lxc_config_value_empty(value)) {
@@ -2445,7 +2445,7 @@ static int set_config_console_buffer_size(const char *key, const char *value,
        /* must be at least a page size */
        pgsz = lxc_getpagesize();
        if ((uint64_t)size < pgsz) {
-               NOTICE("Requested ringbuffer size for the console is %" PRId64 " but must be at least %" PRId64 " bytes. Setting ringbuffer size to %" PRId64 " bytes",
+               NOTICE("Requested ringbuffer size for the console is %lld but must be at least %" PRId64 " bytes. Setting ringbuffer size to %" PRId64 " bytes",
                       size, pgsz, pgsz);
                size = pgsz;
        }
@@ -2466,7 +2466,7 @@ static int set_config_console_size(const char *key, const char *value,
                                   struct lxc_conf *lxc_conf, void *data)
 {
        int ret;
-       int64_t size;
+       long long int size;
        uint64_t log_size, pgsz;
 
        if (lxc_config_value_empty(value)) {
@@ -2490,7 +2490,7 @@ static int set_config_console_size(const char *key, const char *value,
        /* must be at least a page size */
        pgsz = lxc_getpagesize();
        if ((uint64_t)size < pgsz) {
-               NOTICE("Requested ringbuffer size for the console is %" PRId64 " but must be at least %" PRId64 " bytes. Setting ringbuffer size to %" PRId64 " bytes",
+               NOTICE("Requested ringbuffer size for the console is %lld but must be at least %" PRId64 " bytes. Setting ringbuffer size to %" PRId64 " bytes",
                       size, pgsz, pgsz);
                size = pgsz;
        }
index 1d408b97784f50d221e4f95f64d57a27a3b385bc..933fb434dd6c50f073b78b09c6d0cafb4363c48e 100644 (file)
@@ -897,13 +897,12 @@ void *must_realloc(void *orig, size_t sz)
        return ret;
 }
 
-int parse_byte_size_string(const char *s, int64_t *converted)
+int parse_byte_size_string(const char *s, long long int *converted)
 {
        int ret, suffix_len;
-       long long int conv;
-       int64_t mltpl, overflow;
+       long long int conv, mltpl;
        char *end;
-       char dup[INTTYPE_TO_STRLEN(int64_t)];
+       char dup[INTTYPE_TO_STRLEN(long long int)];
        char suffix[3] = {0};
 
        if (is_empty_string(s))
@@ -960,11 +959,9 @@ int parse_byte_size_string(const char *s, int64_t *converted)
        else
                return ret_errno(EINVAL);
 
-       overflow = conv * mltpl;
-       if (conv != 0 && (overflow / conv) != mltpl)
+       if (check_mul_overflow(conv, mltpl, converted))
                return ret_errno(ERANGE);
 
-       *converted = overflow;
        return 0;
 }
 
index 688587de155c4f5ff986d9daf68881fcbff4a378..76c765d3c774ee00fac6af29ec7759af0b86ba61 100644 (file)
@@ -81,7 +81,7 @@ __hidden extern int lxc_safe_uint64(const char *numstr, uint64_t *converted, int
 __hidden extern int lxc_safe_int64_residual(const char *numstr, int64_t *converted, int base,
                                            char *residual, size_t residual_len);
 /* Handles B, kb, MB, GB. Detects overflows and reports -ERANGE. */
-__hidden extern int parse_byte_size_string(const char *s, int64_t *converted);
+__hidden extern int parse_byte_size_string(const char *s, long long int *converted);
 
 /*
  * Concatenate all passed-in strings into one path. Do not fail. If any piece
index 841198d9ef6c5ee0c4737f18c938c4b72a4a4615..9b3e8cd1917f0b7a672abd05503e1f669ec7eb2b 100644 (file)
@@ -403,7 +403,7 @@ void test_lxc_string_in_array(void)
 void test_parse_byte_size_string(void)
 {
        int ret;
-       int64_t n;
+       long long int n;
 
        ret = parse_byte_size_string("0", &n);
        if (ret < 0) {