From 3603cb0978b7ef21ff9cd63693ebd6d27bc2bc53 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Tue, 10 Feb 2015 23:13:49 -0500 Subject: git__*allocarray: safer realloc and malloc Introduce git__reallocarray that checks the product of the number of elements and element size for overflow before allocation. Also introduce git__mallocarray that behaves like calloc, but without the `c`. (It does not zero memory, for those truly worried about every cycle.) --- src/array.h | 3 +-- src/oid.c | 3 +-- src/pack-objects.c | 14 +++++--------- src/tsort.c | 3 +-- src/util.h | 22 ++++++++++++++++++++++ src/vector.c | 7 +------ src/win32/utf-conv.c | 3 +-- 7 files changed, 32 insertions(+), 23 deletions(-) diff --git a/src/array.h b/src/array.h index ca5a35ffe..0d0795370 100644 --- a/src/array.h +++ b/src/array.h @@ -57,8 +57,7 @@ GIT_INLINE(void *) git_array_grow(void *_a, size_t item_size) new_size = a->size * 3 / 2; } - if (GIT_ALLOC_OVERFLOW_MULTIPLY(new_size, item_size) || - (new_array = git__realloc(a->ptr, new_size * item_size)) == NULL) + if ((new_array = git__reallocarray(a->ptr, new_size, item_size)) == NULL) goto on_oom; a->ptr = new_array; a->asize = new_size; a->size++; diff --git a/src/oid.c b/src/oid.c index 9aac47db6..9fe2ebb65 100644 --- a/src/oid.c +++ b/src/oid.c @@ -261,8 +261,7 @@ struct git_oid_shorten { static int resize_trie(git_oid_shorten *self, size_t new_size) { - GITERR_CHECK_ALLOC_MULTIPLY(new_size, sizeof(trie_node)); - self->nodes = git__realloc(self->nodes, new_size * sizeof(trie_node)); + self->nodes = git__reallocarray(self->nodes, new_size, sizeof(trie_node)); GITERR_CHECK_ALLOC(self->nodes); if (new_size > self->size) { diff --git a/src/pack-objects.c b/src/pack-objects.c index aea4770af..cd0deb29a 100644 --- a/src/pack-objects.c +++ b/src/pack-objects.c @@ -205,9 +205,8 @@ int git_packbuilder_insert(git_packbuilder *pb, const git_oid *oid, GITERR_CHECK_ALLOC_MULTIPLY(pb->nr_alloc + 1024, 3 / 2); pb->nr_alloc = (pb->nr_alloc + 1024) * 3 / 2; - GITERR_CHECK_ALLOC_MULTIPLY(pb->nr_alloc, sizeof(*po)); - pb->object_list = git__realloc(pb->object_list, - pb->nr_alloc * sizeof(*po)); + pb->object_list = git__reallocarray(pb->object_list, + pb->nr_alloc, sizeof(*po)); GITERR_CHECK_ALLOC(pb->object_list); rehash(pb); } @@ -505,8 +504,7 @@ static git_pobject **compute_write_order(git_packbuilder *pb) unsigned int i, wo_end, last_untagged; git_pobject **wo; - if (GIT_ALLOC_OVERFLOW_MULTIPLY(pb->nr_objects, sizeof(*wo)) || - (wo = git__malloc(pb->nr_objects * sizeof(*wo))) == NULL) { + if ((wo = git__mallocarray(pb->nr_objects, sizeof(*wo))) == NULL) { giterr_set_oom(); return NULL; } @@ -1102,8 +1100,7 @@ static int ll_find_deltas(git_packbuilder *pb, git_pobject **list, return 0; } - GITERR_CHECK_ALLOC_MULTIPLY(pb->nr_threads, sizeof(*p)); - p = git__malloc(pb->nr_threads * sizeof(*p)); + p = git__mallocarray(pb->nr_threads, sizeof(*p)); GITERR_CHECK_ALLOC(p); /* Partition the work among the threads */ @@ -1254,8 +1251,7 @@ static int prepare_pack(git_packbuilder *pb) if (pb->progress_cb) pb->progress_cb(GIT_PACKBUILDER_DELTAFICATION, 0, pb->nr_objects, pb->progress_cb_payload); - GITERR_CHECK_ALLOC_MULTIPLY(pb->nr_objects, sizeof(*delta_list)); - delta_list = git__malloc(pb->nr_objects * sizeof(*delta_list)); + delta_list = git__mallocarray(pb->nr_objects, sizeof(*delta_list)); GITERR_CHECK_ALLOC(delta_list); for (i = 0; i < pb->nr_objects; ++i) { diff --git a/src/tsort.c b/src/tsort.c index b92e52056..e59819204 100644 --- a/src/tsort.c +++ b/src/tsort.c @@ -186,8 +186,7 @@ static int resize(struct tsort_store *store, size_t new_size) if (store->alloc < new_size) { void **tempstore; - GITERR_CHECK_ALLOC_MULTIPLY(new_size, sizeof(void *)); - tempstore = git__realloc(store->storage, new_size * sizeof(void *)); + tempstore = git__reallocarray(store->storage, new_size, sizeof(void *)); /** * Do not propagate on OOM; this will abort the sort and diff --git a/src/util.h b/src/util.h index 6c94a5aa0..7693ef809 100644 --- a/src/util.h +++ b/src/util.h @@ -104,6 +104,28 @@ GIT_INLINE(void *) git__realloc(void *ptr, size_t size) return new_ptr; } +/** + * Similar to `git__realloc`, except that it is suitable for reallocing an + * array to a new number of elements of `nelem`, each of size `elsize`. + * The total size calculation is checked for overflow. + */ +GIT_INLINE(void *) git__reallocarray(void *ptr, size_t nelem, size_t elsize) +{ + void *new_ptr = NULL; + if (GIT_ALLOC_OVERFLOW_MULTIPLY(nelem, elsize) || + !(new_ptr = realloc(ptr, nelem * elsize))) + giterr_set_oom(); + return new_ptr; +} + +/** + * Similar to `git__calloc`, except that it does not zero memory. + */ +GIT_INLINE(void *) git__mallocarray(size_t nelem, size_t elsize) +{ + return git__reallocarray(NULL, nelem, elsize); +} + GIT_INLINE(void) git__free(void *ptr) { free(ptr); diff --git a/src/vector.c b/src/vector.c index b636032b1..27eafebc8 100644 --- a/src/vector.c +++ b/src/vector.c @@ -29,14 +29,9 @@ GIT_INLINE(size_t) compute_new_size(git_vector *v) GIT_INLINE(int) resize_vector(git_vector *v, size_t new_size) { - size_t new_bytes; void *new_contents; - /* Check for overflow */ - GITERR_CHECK_ALLOC_MULTIPLY(new_size, sizeof(void *)); - new_bytes = new_size * sizeof(void *); - - new_contents = git__realloc(v->contents, new_bytes); + new_contents = git__reallocarray(v->contents, new_size, sizeof(void *)); GITERR_CHECK_ALLOC(new_contents); v->_alloc_size = new_size; diff --git a/src/win32/utf-conv.c b/src/win32/utf-conv.c index 624611205..0dad4eab0 100644 --- a/src/win32/utf-conv.c +++ b/src/win32/utf-conv.c @@ -99,8 +99,7 @@ int git__utf8_to_16_alloc(wchar_t **dest, const char *src) return -1; } - if (GIT_ALLOC_OVERFLOW_MULTIPLY(utf16_size, sizeof(wchar_t)) || - !(*dest = git__malloc(utf16_size * sizeof(wchar_t)))) { + if (!(*dest = git__mallocarray(utf16_size, sizeof(wchar_t)))) { errno = ENOMEM; return -1; } -- cgit v1.2.1