From 320d0b493a259db3b481f985545b244438e6c086 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 19 Feb 2016 06:21:19 -0500 Subject: add helpers for detecting size_t overflow Performing computations on size_t variables that we feed to xmalloc and friends can be dangerous, as an integer overflow can cause us to allocate a much smaller chunk than we realized. We already have unsigned_add_overflows(), but let's add unsigned_mult_overflows() to that. Furthermore, rather than have each site manually check and die on overflow, we can provide some helpers that will: - promote the arguments to size_t, so that we know we are doing our computation in the same size of integer that will ultimately be fed to xmalloc - check and die on overflow - return the result so that computations can be done in the parameter list of xmalloc. These functions are a lot uglier to use than normal arithmetic operators (you have to do "st_add(foo, bar)" instead of "foo + bar"). To at least limit the damage, we also provide multi-valued versions. So rather than: st_add(st_add(a, b), st_add(c, d)); you can write: st_add4(a, b, c, d); This isn't nearly as elegant as a varargs function, but it's a lot harder to get it wrong. You don't have to remember to add a sentinel value at the end, and the compiler will complain if you get the number of arguments wrong. This patch adds only the numbered variants required to convert the current code base; we can easily add more later if needed. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- git-compat-util.h | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) (limited to 'git-compat-util.h') diff --git a/git-compat-util.h b/git-compat-util.h index 693a336ff5..0c65033481 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -96,6 +96,14 @@ #define unsigned_add_overflows(a, b) \ ((b) > maximum_unsigned_value_of_type(a) - (a)) +/* + * Returns true if the multiplication of "a" and "b" will + * overflow. The types of "a" and "b" must match and must be unsigned. + * Note that this macro evaluates "a" twice! + */ +#define unsigned_mult_overflows(a, b) \ + ((a) && (b) > maximum_unsigned_value_of_type(a) / (a)) + #ifdef __GNUC__ #define TYPEOF(x) (__typeof__(x)) #else @@ -713,6 +721,32 @@ extern void release_pack_memory(size_t); typedef void (*try_to_free_t)(size_t); extern try_to_free_t set_try_to_free_routine(try_to_free_t); +static inline size_t st_add(size_t a, size_t b) +{ + if (unsigned_add_overflows(a, b)) + die("size_t overflow: %"PRIuMAX" + %"PRIuMAX, + (uintmax_t)a, (uintmax_t)b); + return a + b; +} +#define st_add3(a,b,c) st_add((a),st_add((b),(c))) +#define st_add4(a,b,c,d) st_add((a),st_add3((b),(c),(d))) + +static inline size_t st_mult(size_t a, size_t b) +{ + if (unsigned_mult_overflows(a, b)) + die("size_t overflow: %"PRIuMAX" * %"PRIuMAX, + (uintmax_t)a, (uintmax_t)b); + return a * b; +} + +static inline size_t st_sub(size_t a, size_t b) +{ + if (a < b) + die("size_t underflow: %"PRIuMAX" - %"PRIuMAX, + (uintmax_t)a, (uintmax_t)b); + return a - b; +} + #ifdef HAVE_ALLOCA_H # include # define xalloca(size) (alloca(size)) -- cgit v1.3 From e7792a74bcf7fcd554b4546fc91122b8c2af7d75 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 22 Feb 2016 17:43:18 -0500 Subject: harden REALLOC_ARRAY and xcalloc against size_t overflow REALLOC_ARRAY inherently involves a multiplication which can overflow size_t, resulting in a much smaller buffer than we think we've allocated. We can easily harden it by using st_mult() to check for overflow. Likewise, we can add ALLOC_ARRAY to do the same thing for xmalloc calls. xcalloc() should already be fine, because it takes the two factors separately, assuming the system calloc actually checks for overflow. However, before we even hit the system calloc(), we do our memory_limit_check, which involves a multiplication. Let's check for overflow ourselves so that this limit cannot be bypassed. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- git-compat-util.h | 3 ++- wrapper.c | 3 +++ 2 files changed, 5 insertions(+), 1 deletion(-) (limited to 'git-compat-util.h') diff --git a/git-compat-util.h b/git-compat-util.h index 0c65033481..81f2347b27 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -779,7 +779,8 @@ extern int odb_pack_keep(char *name, size_t namesz, const unsigned char *sha1); extern char *xgetcwd(void); extern FILE *fopen_for_writing(const char *path); -#define REALLOC_ARRAY(x, alloc) (x) = xrealloc((x), (alloc) * sizeof(*(x))) +#define ALLOC_ARRAY(x, alloc) (x) = xmalloc(st_mult(sizeof(*(x)), (alloc))) +#define REALLOC_ARRAY(x, alloc) (x) = xrealloc((x), st_mult(sizeof(*(x)), (alloc))) static inline char *xstrdup_or_null(const char *str) { diff --git a/wrapper.c b/wrapper.c index 52001789de..32c6c609da 100644 --- a/wrapper.c +++ b/wrapper.c @@ -152,6 +152,9 @@ void *xcalloc(size_t nmemb, size_t size) { void *ret; + if (unsigned_mult_overflows(nmemb, size)) + die("data too large to fit into virtual memory space"); + memory_limit_check(size * nmemb, 0); ret = calloc(nmemb, size); if (!ret && (!nmemb || !size)) -- cgit v1.3 From 3689539127f786f32ff04da374d37cc1f72fb918 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 22 Feb 2016 17:43:25 -0500 Subject: add helpers for allocating flex-array structs Allocating a struct with a flex array is pretty simple in practice: you over-allocate the struct, then copy some data into the over-allocation. But it can be a slight pain to make sure you're allocating and copying the right amounts. This patch adds a few helpers to turn simple cases of flex-array struct allocation into a one-liner that properly checks for overflow. See the embedded documentation for details. Ideally we could provide a more flexible version that could handle multiple strings, like: FLEX_ALLOC_FMT(ref, name, "%s%s", prefix, name); But we have to implement this as a macro (because of the offset calculation of the flex member), which means we would need all compilers to support variadic macros. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- git-compat-util.h | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) (limited to 'git-compat-util.h') diff --git a/git-compat-util.h b/git-compat-util.h index 81f2347b27..57ff9fb19a 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -782,6 +782,68 @@ extern FILE *fopen_for_writing(const char *path); #define ALLOC_ARRAY(x, alloc) (x) = xmalloc(st_mult(sizeof(*(x)), (alloc))) #define REALLOC_ARRAY(x, alloc) (x) = xrealloc((x), st_mult(sizeof(*(x)), (alloc))) +/* + * These functions help you allocate structs with flex arrays, and copy + * the data directly into the array. For example, if you had: + * + * struct foo { + * int bar; + * char name[FLEX_ARRAY]; + * }; + * + * you can do: + * + * struct foo *f; + * FLEX_ALLOC_MEM(f, name, src, len); + * + * to allocate a "foo" with the contents of "src" in the "name" field. + * The resulting struct is automatically zero'd, and the flex-array field + * is NUL-terminated (whether the incoming src buffer was or not). + * + * The FLEXPTR_* variants operate on structs that don't use flex-arrays, + * but do want to store a pointer to some extra data in the same allocated + * block. For example, if you have: + * + * struct foo { + * char *name; + * int bar; + * }; + * + * you can do: + * + * struct foo *f; + * FLEX_ALLOC_STR(f, name, src); + * + * and "name" will point to a block of memory after the struct, which will be + * freed along with the struct (but the pointer can be repointed anywhere). + * + * The *_STR variants accept a string parameter rather than a ptr/len + * combination. + * + * Note that these macros will evaluate the first parameter multiple + * times, and it must be assignable as an lvalue. + */ +#define FLEX_ALLOC_MEM(x, flexname, buf, len) do { \ + (x) = NULL; /* silence -Wuninitialized for offset calculation */ \ + (x) = xalloc_flex(sizeof(*(x)), (char *)(&((x)->flexname)) - (char *)(x), (buf), (len)); \ +} while (0) +#define FLEXPTR_ALLOC_MEM(x, ptrname, buf, len) do { \ + (x) = xalloc_flex(sizeof(*(x)), sizeof(*(x)), (buf), (len)); \ + (x)->ptrname = (void *)((x)+1); \ +} while(0) +#define FLEX_ALLOC_STR(x, flexname, str) \ + FLEX_ALLOC_MEM((x), flexname, (str), strlen(str)) +#define FLEXPTR_ALLOC_STR(x, ptrname, str) \ + FLEXPTR_ALLOC_MEM((x), ptrname, (str), strlen(str)) + +static inline void *xalloc_flex(size_t base_len, size_t offset, + const void *src, size_t src_len) +{ + unsigned char *ret = xcalloc(1, st_add3(base_len, src_len, 1)); + memcpy(ret + offset, src, src_len); + return ret; +} + static inline char *xstrdup_or_null(const char *str) { return str ? xstrdup(str) : NULL; -- cgit v1.3 From 7eb45b5f78e257a682ec825839b4b33e025a9615 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 22 Feb 2016 17:45:01 -0500 Subject: git-compat-util: drop mempcpy compat code There are no callers of this left, as the last one was dropped in the previous patch. And there are not likely to be new ones, as the function has been around since 2010 without gaining any new callers. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- git-compat-util.h | 9 --------- 1 file changed, 9 deletions(-) (limited to 'git-compat-util.h') diff --git a/git-compat-util.h b/git-compat-util.h index 57ff9fb19a..1459f9b87e 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -681,7 +681,6 @@ extern int git_vsnprintf(char *str, size_t maxsize, #ifdef __GLIBC_PREREQ #if __GLIBC_PREREQ(2, 1) #define HAVE_STRCHRNUL -#define HAVE_MEMPCPY #endif #endif @@ -695,14 +694,6 @@ static inline char *gitstrchrnul(const char *s, int c) } #endif -#ifndef HAVE_MEMPCPY -#define mempcpy gitmempcpy -static inline void *gitmempcpy(void *dest, const void *src, size_t n) -{ - return (char *)memcpy(dest, src, n) + n; -} -#endif - #ifdef NO_INET_PTON int inet_pton(int af, const char *src, void *dst); #endif -- cgit v1.3