From 88c11d28d891236df51229ff017dce26e8c5a479 Mon Sep 17 00:00:00 2001 From: Emil Velikov Date: Mon, 30 Sep 2024 22:11:40 +0100 Subject: [PATCH] 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 Link: https://github.com/kmod-project/kmod/pull/169 Signed-off-by: Lucas De Marchi --- libkmod/libkmod-builtin.c | 12 +++++++----- libkmod/libkmod-elf.c | 12 +++++++----- shared/array.c | 11 +++++++---- shared/util.h | 22 ++++++++++++++++++++++ 4 files changed, 43 insertions(+), 14 deletions(-) diff --git a/libkmod/libkmod-builtin.c b/libkmod/libkmod-builtin.c index 937fefc..dad82bb 100644 --- a/libkmod/libkmod-builtin.c +++ b/libkmod/libkmod-builtin.c @@ -15,6 +15,7 @@ #include #include +#include #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 a8dcdd8..cd91d8e 100644 --- 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 964ab2f..38c04b5 100644 --- a/shared/array.c +++ b/shared/array.c @@ -10,16 +10,18 @@ #include #include +#include /* 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 7045e65..974f1dd 100644 --- 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 +}