Re: [PATCH 08/18] use st_add and st_mult for allocation size computation
From: Eric Sunshine <hidden>
Date: 2016-06-15 23:08:15
On Mon, Feb 15, 2016 at 4:53 PM, Jeff King [off-list ref] wrote:
quoted hunk ↗ jump to hunk
If our size computation overflows size_t, we may allocate a much smaller buffer than we expected and overflow it. It's probably impossible to trigger an overflow in most of these sites in practice, but it is easy enough convert their additions and multiplications into overflow-checking variants. This may be fixing real bugs, and it makes auditing the code easier. Signed-off-by: Jeff King <redacted> ---diff --git a/builtin/apply.c b/builtin/apply.c@@ -2632,7 +2632,7 @@ static void update_image(struct image *img, - result = xmalloc(img->len + insert_count - remove_count + 1); + result = xmalloc(st_add3(st_sub(img->len, remove_count), insert_count, 1));
Phew, what a mouthful, and not easy to read compared to the original. Fortunately, the remainder of the changes in this patch are straightforward and often simple.
quoted hunk ↗ jump to hunk
diff --git a/sha1_name.c b/sha1_name.c@@ -87,9 +87,8 @@ static void find_short_object_filename(int len, const char *hex_pfx, struct disa const char *objdir = get_object_directory(); - int objdir_len = strlen(objdir); - int entlen = objdir_len + 43; - fakeent = xmalloc(sizeof(*fakeent) + entlen); + size_t objdir_len = strlen(objdir); + fakeent = xmalloc(st_add3(sizeof(*fakeent), objdir_len, 43)); memcpy(fakeent->base, objdir, objdir_len); fakeent->name = fakeent->base + objdir_len + 1;
If we've gotten this far without die()ing due to overflow in st_add3() when invoking xmalloc(), then we know that this fakeent->name computation won't overflow. Okay.
fakeent->name[-1] = '/';