Introduce and use u{add,mul}sz_overflow() helpers

Instead of doing things manually add a few helpers and use them. As a
bonus point, fix the potential overflow in kmod_elf_get_strings().

Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com>
Link: https://github.com/kmod-project/kmod/pull/169
Signed-off-by: Lucas De Marchi <lucas.de.marchi@gmail.com>

diff --git a/libkmod/libkmod-builtin.c b/libkmod/libkmod-builtin.c
index 937fefc8cfc67d7cb9c37f943908a4b4d070acb5..dad82bb49b075aa5df90cd36740064c65ee671c1 100644 (file)
--- a/libkmod/libkmod-builtin.c
+++ b/libkmod/libkmod-builtin.c
@@ -15,6 +15,7 @@
 #include <unistd.h>
 
 #include <shared/strbuf.h>
+#include <shared/util.h>
 
 #include "libkmod.h"
 #include "libkmod-internal.h"
@@ -133,19 +134,20 @@ static ssize_t get_strings(struct kmod_builtin_info *info, const char *modname,
 
 static char **strbuf_to_vector(struct strbuf *buf, size_t count)
 {
-       /* size required for string vector + terminating NULL */
-       const size_t vecsz = sizeof(char *) * (count + 1);
+       size_t vec_size, total_size;
        char **vector;
        char *s;
        size_t n;
 
-       /* make sure that vector and strings fit into memory constraints */
-       if (SIZE_MAX / sizeof(char *) - 1 < count || SIZE_MAX - buf->used < vecsz) {
+       /* (string vector + NULL) * sizeof(char *) + buf->used */
+       if (uaddsz_overflow(count, 1, &n) ||
+           umulsz_overflow(sizeof(char *), n, &vec_size) ||
+           uaddsz_overflow(buf->used, vec_size, &total_size)) {
                errno = ENOMEM;
                return NULL;
        }
 
-       vector = realloc(buf->bytes, vecsz + buf->used);
+       vector = realloc(buf->bytes, total_size);
        if (vector == NULL)
                return NULL;
        buf->bytes = NULL;

diff --git a/libkmod/libkmod-elf.c b/libkmod/libkmod-elf.c
index a8dcdd83753ac375d4e771fe31ab56393f0e595e..cd91d8e5e19fca609d7be0fb7a9b155fe17b7054 100644 (file)
--- a/libkmod/libkmod-elf.c
+++ b/libkmod/libkmod-elf.c
@@ -429,7 +429,7 @@ int kmod_elf_get_section(const struct kmod_elf *elf, const char *section,
 int kmod_elf_get_strings(const struct kmod_elf *elf, const char *section, char ***array)
 {
        size_t i, j, count;
-       size_t vecsz;
+       size_t tmp_size, vec_size, total_size;
        uint64_t size;
        const void *buf;
        const char *strings;
@@ -470,13 +470,15 @@ int kmod_elf_get_strings(const struct kmod_elf *elf, const char *section, char *
        if (strings[i - 1] != '\0')
                count++;
 
-       /* make sure that vector and strings fit into memory constraints */
-       vecsz = sizeof(char *) * (count + 1);
-       if (SIZE_MAX / sizeof(char *) - 1 < count || SIZE_MAX - size <= vecsz) {
+       /* (string vector + NULL) * sizeof(char *) + size + NUL */
+       if (uaddsz_overflow(count, 1, &tmp_size) ||
+           umulsz_overflow(sizeof(char *), tmp_size, &vec_size) ||
+           uaddsz_overflow(size, vec_size, &tmp_size) ||
+           uaddsz_overflow(1, tmp_size, &total_size)) {
                return -ENOMEM;
        }
 
-       *array = a = malloc(vecsz + size + 1);
+       *array = a = malloc(total_size);
        if (*array == NULL)
                return -errno;
 

diff --git a/shared/array.c b/shared/array.c
index 964ab2f29016bed7bb1fa0e06ae03064ba705516..38c04b50490a4613d1e7ff58c91d69992ae35f00 100644 (file)
--- a/shared/array.c
+++ b/shared/array.c
@@ -10,16 +10,18 @@
 #include <string.h>
 
 #include <shared/array.h>
+#include <shared/util.h>
 
 /* basic pointer array growing in steps */
 
 static int array_realloc(struct array *array, size_t new_total)
 {
+       size_t total_size;
        void *tmp;
 
-       if (SIZE_MAX / sizeof(void *) < new_total)
+       if (umulsz_overflow(sizeof(void *), new_total, &total_size))
                return -ENOMEM;
-       tmp = realloc(array->array, sizeof(void *) * new_total);
+       tmp = realloc(array->array, total_size);
        if (tmp == NULL)
                return -ENOMEM;
        array->array = tmp;
@@ -49,11 +51,12 @@ int array_append(struct array *array, const void *element)
        size_t idx;
 
        if (array->count + 1 >= array->total) {
+               size_t new_size;
                int r;
 
-               if (SIZE_MAX - array->total < array->step)
+               if (uaddsz_overflow(array->total, array->step, &new_size))
                        return -ENOMEM;
-               r = array_realloc(array, array->total + array->step);
+               r = array_realloc(array, new_size);
                if (r < 0)
                        return r;
        }

diff --git a/shared/util.h b/shared/util.h
index 7045e65b4f6eae70e4821060a7b73a66514d47d6..974f1ddbc8b28ab8e73e2db60c39bafadf16ccb7 100644 (file)
--- a/shared/util.h
+++ b/shared/util.h
@@ -114,6 +114,17 @@ static inline bool uadd64_overflow(uint64_t a, uint64_t b, uint64_t *res)
 #endif
 }
 
+static inline bool uaddsz_overflow(size_t a, size_t b, size_t *res)
+{
+#if __SIZEOF_SIZE_T__ == 8
+       return uadd64_overflow(a, b, res);
+#elif __SIZEOF_SIZE_T__ == 4
+       return uadd32_overflow(a, b, res);
+#else
+#error "Unknown sizeof(size_t)"
+#endif
+}
+
 static inline bool umul32_overflow(uint32_t a, uint32_t b, uint32_t *res)
 {
 #if (HAVE___BUILTIN_UMUL_OVERFLOW && __SIZEOF_INT__ == 4)
@@ -135,3 +146,14 @@ static inline bool umul64_overflow(uint64_t a, uint64_t b, uint64_t *res)
        return UINT64_MAX / a < b;
 #endif
 }
+
+static inline bool umulsz_overflow(size_t a, size_t b, size_t *res)
+{
+#if __SIZEOF_SIZE_T__ == 8
+       return umul64_overflow(a, b, res);
+#elif __SIZEOF_SIZE_T__ == 4
+       return umul32_overflow(a, b, res);
+#else
+#error "Unknown sizeof(size_t)"
+#endif
+}