diff --git a/Documentation/git.adoc b/Documentation/git.adoc index 8a5cdd3b3d22c5..07da5c4f124cea 100644 --- a/Documentation/git.adoc +++ b/Documentation/git.adoc @@ -688,6 +688,12 @@ For each path `GIT_EXTERNAL_DIFF` is called, two environment variables, other ~~~~~ + +`GIT_ALLOC_LIMIT`:: + A number limiting how much memory can be allocated in a single + hunk. This only limits single allocations and does not limit the + total memory used by the process. + `GIT_MERGE_VERBOSITY`:: A number controlling the amount of output shown by the recursive merge strategy. Overrides merge.verbosity. diff --git a/Makefile b/Makefile index d4b775953d3842..594385321986a6 100644 --- a/Makefile +++ b/Makefile @@ -1327,6 +1327,7 @@ LIB_OBJS += sparse-index.o LIB_OBJS += split-index.o LIB_OBJS += stable-qsort.o LIB_OBJS += statinfo.o +LIB_OBJS += strbuf-safe.o LIB_OBJS += strbuf.o LIB_OBJS += string-list.o LIB_OBJS += strmap.o diff --git a/common-init.c b/common-init.c index d26c9c1f20239e..bf73c754b4ff43 100644 --- a/common-init.c +++ b/common-init.c @@ -39,6 +39,8 @@ static void setup_environment(void) char *git_replace_ref_base; const char *replace_ref_base; + initialize_git_alloc_limit(); + if (getenv(NO_REPLACE_OBJECTS_ENVIRONMENT)) disable_replace_refs(); replace_ref_base = getenv(GIT_REPLACE_REF_BASE_ENVIRONMENT); diff --git a/environment.h b/environment.h index e7ec5b0437342d..86b67da877f2b1 100644 --- a/environment.h +++ b/environment.h @@ -5,6 +5,7 @@ #include "branch.h" /* Double-check local_repo_env below if you add to this list. */ +#define GIT_ALLOC_LIMIT "GIT_ALLOC_LIMIT" #define GIT_DIR_ENVIRONMENT "GIT_DIR" #define GIT_COMMON_DIR_ENVIRONMENT "GIT_COMMON_DIR" #define GIT_NAMESPACE_ENVIRONMENT "GIT_NAMESPACE" diff --git a/json-writer.c b/json-writer.c index 34577dc25f887c..38351f3439bc2a 100644 --- a/json-writer.c +++ b/json-writer.c @@ -2,6 +2,9 @@ #include "git-compat-util.h" #include "json-writer.h" +#include "strbuf.h" +/* banned-die must be last. */ +#include "banned-die.h" void jw_init(struct json_writer *jw) { @@ -9,10 +12,15 @@ void jw_init(struct json_writer *jw) memcpy(jw, &blank, sizeof(*jw));; } -void jw_release(struct json_writer *jw) +int jw_release(struct json_writer *jw) { - strbuf_release(&jw->json); - strbuf_release(&jw->open_stack); + enum safe_result result = SUCCESS; + + /* attempt both removals without short-circuiting. */ + result = sstrbuf_release(&jw->json) || result; + result = sstrbuf_release(&jw->open_stack) || result; + + return result; } /* @@ -98,16 +106,17 @@ static void maybe_add_comma(struct json_writer *jw) jw->need_comma = 1; } -static void fmt_double(struct json_writer *jw, int precision, - double value) +static int fmt_double(struct json_writer *jw, int precision, + double value) { if (precision < 0) { strbuf_addf(&jw->json, "%f", value); + return 0; } else { struct strbuf fmt = STRBUF_INIT; strbuf_addf(&fmt, "%%.%df", precision); strbuf_addf(&jw->json, fmt.buf, value); - strbuf_release(&fmt); + return sstrbuf_release(&fmt); } } @@ -234,8 +243,8 @@ static void kill_indent(struct strbuf *sb, } } -static void append_sub_jw(struct json_writer *jw, - const struct json_writer *value) +static int append_sub_jw(struct json_writer *jw, + const struct json_writer *value) { /* * If both are pretty, increase the indentation of the sub_jw @@ -254,18 +263,17 @@ static void append_sub_jw(struct json_writer *jw, struct strbuf sb = STRBUF_INIT; increase_indent(&sb, value, jw->open_stack.len * 2); strbuf_addbuf(&jw->json, &sb); - strbuf_release(&sb); - return; + return sstrbuf_release(&sb); } if (!jw->pretty && value->pretty) { struct strbuf sb = STRBUF_INIT; kill_indent(&sb, value); strbuf_addbuf(&jw->json, &sb); - strbuf_release(&sb); - return; + return sstrbuf_release(&sb); } strbuf_addbuf(&jw->json, &value->json); + return 0; } void jw_object_sub_jw(struct json_writer *jw, const char *key, diff --git a/json-writer.h b/json-writer.h index 8f845d4d294d8d..72277d983931ad 100644 --- a/json-writer.h +++ b/json-writer.h @@ -70,7 +70,7 @@ * of the given strings. */ -#include "strbuf.h" +#include "strbuf-safe.h" struct json_writer { @@ -103,9 +103,10 @@ struct json_writer void jw_init(struct json_writer *jw); /* - * Release the internal buffers of a json_writer. + * Release the internal buffers of a json_writer. Returns nonzero on + * failure. */ -void jw_release(struct json_writer *jw); +int jw_release(struct json_writer *jw); /* * Begin the json_writer using an object as the top-level data structure. If diff --git a/meson.build b/meson.build index d86f2acd2b2a46..368fdd00d5ee82 100644 --- a/meson.build +++ b/meson.build @@ -532,6 +532,7 @@ libgit_sources = [ 'split-index.c', 'stable-qsort.c', 'statinfo.c', + 'strbuf-safe.c', 'strbuf.c', 'string-list.c', 'strmap.c', diff --git a/strbuf-safe.c b/strbuf-safe.c new file mode 100644 index 00000000000000..7a8701e827b7ae --- /dev/null +++ b/strbuf-safe.c @@ -0,0 +1,52 @@ +#include "git-compat-util.h" +#include "strbuf-safe.h" +#include "banned-die.h" + +/* + * A safe version of ALLOC_GROW from git-compat-util.h and + * xrealloc() from wrapper.c. + */ +#define SAFE_ALLOC_GROW(x, nr, alloc) \ + do { \ + if ((nr) > alloc) { \ + if (alloc_nr(alloc) < (nr)) \ + alloc = (nr); \ + else \ + alloc = alloc_nr(alloc); \ + if (srealloc((void **)&(x), alloc)) \ + return MEMORY_ERROR; \ + } \ + } while (0) + +enum safe_result sstrbuf_grow(struct strbuf *sb, size_t extra) +{ + int new_buf = !sb->alloc; + size_t new_len = st_add3(sb->len, extra, 1); + if (new_buf) + sb->buf = NULL; + + SAFE_ALLOC_GROW(sb->buf, new_len, sb->alloc); + + if (new_buf) + sb->buf[0] = '\0'; + + return SUCCESS; +} + +enum safe_result sstrbuf_init(struct strbuf *sb, size_t hint) +{ + struct strbuf blank = STRBUF_INIT; + memcpy(sb, &blank, sizeof(*sb)); + if (!hint) + return 0; + return sstrbuf_grow(sb, hint); +} + +enum safe_result sstrbuf_release(struct strbuf *sb) +{ + if (sb->alloc) { + free(sb->buf); + return sstrbuf_init(sb, 0); + } + return 0; +} diff --git a/strbuf-safe.h b/strbuf-safe.h new file mode 100644 index 00000000000000..fe04d9cf62ae1c --- /dev/null +++ b/strbuf-safe.h @@ -0,0 +1,97 @@ +#ifndef STRBUF_SAFE_H +#define STRBUF_SAFE_H + +/* + * NOTE FOR STRBUF DEVELOPERS + * + * strbuf is a low-level primitive; as such it should interact only + * with other low-level primitives. Do not introduce new functions + * which interact with higher-level APIs. + * + * This header file specifically conatins the "safe" API surface for + * working with strbufs. The implementations of these methods avoid + * using die() and other exits. Thus, these methods are appropriate + * for use within lower-level APIs such as trace2. + */ + +struct string_list; + +/** + * strbufs are meant to be used with all the usual C string and memory + * APIs. Given that the length of the buffer is known, it's often better to + * use the mem* functions than a str* one (e.g., memchr vs. strchr). + * Though, one has to be careful about the fact that str* functions often + * stop on NULs and that strbufs may have embedded NULs. + * + * A strbuf is NUL terminated for convenience, but no function in the + * strbuf API actually relies on the string being free of NULs. + * + * strbufs have some invariants that are very important to keep in mind: + * + * - The `buf` member is never NULL, so it can be used in any usual C + * string operations safely. strbufs _have_ to be initialized either by + * `strbuf_init()` or by `= STRBUF_INIT` before the invariants, though. + * + * Do *not* assume anything on what `buf` really is (e.g. if it is + * allocated memory or not), use `strbuf_detach()` to unwrap a memory + * buffer from its strbuf shell in a safe way. That is the sole supported + * way. This will give you a malloced buffer that you can later `free()`. + * + * However, it is totally safe to modify anything in the string pointed by + * the `buf` member, between the indices `0` and `len-1` (inclusive). + * + * - The `buf` member is a byte array that has at least `len + 1` bytes + * allocated. The extra byte is used to store a `'\0'`, allowing the + * `buf` member to be a valid C-string. All strbuf functions ensure this + * invariant is preserved. + * + * NOTE: It is OK to "play" with the buffer directly if you work it this + * way: + * + * strbuf_grow(sb, SOME_SIZE); <1> + * strbuf_setlen(sb, sb->len + SOME_OTHER_SIZE); + * + * <1> Here, the memory array starting at `sb->buf`, and of length + * `strbuf_avail(sb)` is all yours, and you can be sure that + * `strbuf_avail(sb)` is at least `SOME_SIZE`. + * + * NOTE: `SOME_OTHER_SIZE` must be smaller or equal to `strbuf_avail(sb)`. + * + * Doing so is safe, though if it has to be done in many places, adding the + * missing API to the strbuf module is the way to go. + * + * WARNING: Do _not_ assume that the area that is yours is of size `alloc + * - 1` even if it's true in the current implementation. Alloc is somehow a + * "private" member that should not be messed with. Use `strbuf_avail()` + * instead. +*/ + +/** + * Data Structures + * --------------- + */ + +/** + * This is the string buffer structure. The `len` member can be used to + * determine the current length of the string, and `buf` member provides + * access to the string itself. + */ +struct strbuf { + size_t alloc; + size_t len; + char *buf; +}; + +extern char strbuf_slopbuf[]; +#define STRBUF_INIT { .buf = strbuf_slopbuf } + +enum safe_result { + SUCCESS = 0, + MEMORY_ERROR, +}; + +enum safe_result sstrbuf_grow(struct strbuf *sb, size_t extra); +enum safe_result sstrbuf_init(struct strbuf *sb, size_t hint); +enum safe_result sstrbuf_release(struct strbuf *sb); + +#endif /* STRBUF_SAFE_H */ diff --git a/strbuf.c b/strbuf.c index 44955669e8c504..835238dc645846 100644 --- a/strbuf.c +++ b/strbuf.c @@ -8,6 +8,8 @@ #include "utf8.h" #include "date.h" +#define STRBUF_DIE(f) die(_("unexpected error during string manipulation: %s"), f) + bool starts_with(const char *str, const char *prefix) { for (; ; str++, prefix++) @@ -68,18 +70,14 @@ char strbuf_slopbuf[1]; void strbuf_init(struct strbuf *sb, size_t hint) { - struct strbuf blank = STRBUF_INIT; - memcpy(sb, &blank, sizeof(*sb)); - if (hint) - strbuf_grow(sb, hint); + if (sstrbuf_init(sb, hint)) + STRBUF_DIE("strbuf_init"); } void strbuf_release(struct strbuf *sb) { - if (sb->alloc) { - free(sb->buf); - strbuf_init(sb, 0); - } + if (sstrbuf_release(sb)) + STRBUF_DIE("strbuf_release"); } char *strbuf_detach(struct strbuf *sb, size_t *sz) @@ -105,13 +103,8 @@ void strbuf_attach(struct strbuf *sb, void *buf, size_t len, size_t alloc) void strbuf_grow(struct strbuf *sb, size_t extra) { - int new_buf = !sb->alloc; - size_t new_len = st_add3(sb->len, extra, 1); - if (new_buf) - sb->buf = NULL; - ALLOC_GROW(sb->buf, new_len, sb->alloc); - if (new_buf) - sb->buf[0] = '\0'; + if (sstrbuf_grow(sb, extra)) + STRBUF_DIE("strbuf_grow"); } void strbuf_trim(struct strbuf *sb) diff --git a/strbuf.h b/strbuf.h index 1089ae687bda95..b41f8ef901ee58 100644 --- a/strbuf.h +++ b/strbuf.h @@ -1,85 +1,19 @@ #ifndef STRBUF_H #define STRBUF_H +#include "strbuf-safe.h" + /* * NOTE FOR STRBUF DEVELOPERS * * strbuf is a low-level primitive; as such it should interact only * with other low-level primitives. Do not introduce new functions * which interact with higher-level APIs. - */ - -struct string_list; - -/** - * strbufs are meant to be used with all the usual C string and memory - * APIs. Given that the length of the buffer is known, it's often better to - * use the mem* functions than a str* one (e.g., memchr vs. strchr). - * Though, one has to be careful about the fact that str* functions often - * stop on NULs and that strbufs may have embedded NULs. - * - * A strbuf is NUL terminated for convenience, but no function in the - * strbuf API actually relies on the string being free of NULs. - * - * strbufs have some invariants that are very important to keep in mind: - * - * - The `buf` member is never NULL, so it can be used in any usual C - * string operations safely. strbufs _have_ to be initialized either by - * `strbuf_init()` or by `= STRBUF_INIT` before the invariants, though. - * - * Do *not* assume anything on what `buf` really is (e.g. if it is - * allocated memory or not), use `strbuf_detach()` to unwrap a memory - * buffer from its strbuf shell in a safe way. That is the sole supported - * way. This will give you a malloced buffer that you can later `free()`. - * - * However, it is totally safe to modify anything in the string pointed by - * the `buf` member, between the indices `0` and `len-1` (inclusive). - * - * - The `buf` member is a byte array that has at least `len + 1` bytes - * allocated. The extra byte is used to store a `'\0'`, allowing the - * `buf` member to be a valid C-string. All strbuf functions ensure this - * invariant is preserved. - * - * NOTE: It is OK to "play" with the buffer directly if you work it this - * way: * - * strbuf_grow(sb, SOME_SIZE); <1> - * strbuf_setlen(sb, sb->len + SOME_OTHER_SIZE); - * - * <1> Here, the memory array starting at `sb->buf`, and of length - * `strbuf_avail(sb)` is all yours, and you can be sure that - * `strbuf_avail(sb)` is at least `SOME_SIZE`. - * - * NOTE: `SOME_OTHER_SIZE` must be smaller or equal to `strbuf_avail(sb)`. - * - * Doing so is safe, though if it has to be done in many places, adding the - * missing API to the strbuf module is the way to go. - * - * WARNING: Do _not_ assume that the area that is yours is of size `alloc - * - 1` even if it's true in the current implementation. Alloc is somehow a - * "private" member that should not be messed with. Use `strbuf_avail()` - * instead. -*/ - -/** - * Data Structures - * --------------- + * Also see strbuf-safe.h for the struct definitions and safe versions + * of some methods declared in this header file. */ -/** - * This is the string buffer structure. The `len` member can be used to - * determine the current length of the string, and `buf` member provides - * access to the string itself. - */ -struct strbuf { - size_t alloc; - size_t len; - char *buf; -}; - -extern char strbuf_slopbuf[]; -#define STRBUF_INIT { .buf = strbuf_slopbuf } - struct object_id; /** diff --git a/trace2/tr2_tgt_event.c b/trace2/tr2_tgt_event.c index 36a746cc108e09..b25fa0fb307dbd 100644 --- a/trace2/tr2_tgt_event.c +++ b/trace2/tr2_tgt_event.c @@ -5,6 +5,7 @@ #include "json-writer.h" #include "repository.h" #include "run-command.h" +#include "strbuf.h" #include "version.h" #include "trace2/tr2_dst.h" #include "trace2/tr2_tbuf.h" diff --git a/trace2/tr2_tgt_perf.c b/trace2/tr2_tgt_perf.c index 96a5bc7f10a097..5554081c3ccb58 100644 --- a/trace2/tr2_tgt_perf.c +++ b/trace2/tr2_tgt_perf.c @@ -7,6 +7,7 @@ #include "quote.h" #include "version.h" #include "json-writer.h" +#include "strbuf.h" #include "trace2/tr2_dst.h" #include "trace2/tr2_sid.h" #include "trace2/tr2_sysenv.h" diff --git a/wrapper.c b/wrapper.c index 561f9ee9c99fc1..69ff9a8ff6568c 100644 --- a/wrapper.c +++ b/wrapper.c @@ -6,6 +6,7 @@ #include "git-compat-util.h" #include "abspath.h" +#include "environment.h" #include "parse.h" #include "gettext.h" #include "strbuf.h" @@ -18,26 +19,42 @@ #undef SystemFunction036 #endif -static int memory_limit_check(size_t size, int gentle) +static size_t git_alloc_limit = 0; + +void initialize_git_alloc_limit(void) { - static size_t limit = 0; - if (!limit) { - limit = git_env_ulong("GIT_ALLOC_LIMIT", 0); - if (!limit) - limit = SIZE_MAX; + if (!git_alloc_limit) { + git_alloc_limit = git_env_ulong(GIT_ALLOC_LIMIT, 0); + if (!git_alloc_limit) + git_alloc_limit = SIZE_MAX; } +} + +static int safe_memory_limit_check(size_t size, int verbose) +{ + size_t limit = git_alloc_limit ? git_alloc_limit : SIZE_MAX; if (size > limit) { - if (gentle) { + if (verbose) error("attempting to allocate %"PRIuMAX" over limit %"PRIuMAX, - (uintmax_t)size, (uintmax_t)limit); - return -1; - } else - die("attempting to allocate %"PRIuMAX" over limit %"PRIuMAX, - (uintmax_t)size, (uintmax_t)limit); + (uintmax_t)size, (uintmax_t)git_alloc_limit); + return -1; } return 0; } +static int memory_limit_check(size_t size, int gentle) +{ + int res; + initialize_git_alloc_limit(); + + res = safe_memory_limit_check(size, gentle); + if (res && !gentle) { + die("attempting to allocate %"PRIuMAX" over limit %"PRIuMAX, + (uintmax_t)size, (uintmax_t)git_alloc_limit); + } + return res; +} + char *xstrdup(const char *str) { char *ret = strdup(str); @@ -127,20 +144,28 @@ int xstrncmpz(const char *s, const char *t, size_t len) return s[len] == '\0' ? 0 : 1; } -void *xrealloc(void *ptr, size_t size) +int srealloc(void **ptr, size_t size) { - void *ret; - if (!size) { - free(ptr); - return xmalloc(0); + free(*ptr); + if ((*ptr = malloc(1))) + return 0; + return -1; } - memory_limit_check(size, 0); - ret = realloc(ptr, size); - if (!ret) + if (safe_memory_limit_check(size, 0)) + return -1; + if ((*ptr = realloc(*ptr, size))) + return 0; + + return -1; +} + +void *xrealloc(void *ptr, size_t size) +{ + if (srealloc(&ptr, size)) die("Out of memory, realloc failed"); - return ret; + return ptr; } void *xcalloc(size_t nmemb, size_t size) diff --git a/wrapper.h b/wrapper.h index a6287d7f4d11be..956de2c534c732 100644 --- a/wrapper.h +++ b/wrapper.h @@ -27,6 +27,9 @@ char *xgetcwd(void); FILE *fopen_for_writing(const char *path); FILE *fopen_or_warn(const char *path, const char *mode); +/* safe versions of helpers above. */ +int srealloc(void **ptr, size_t size); + /* * Like strncmp, but only return zero if s is NUL-terminated and exactly len * characters long. If it is not, consider it greater than t. @@ -180,4 +183,10 @@ static inline unsigned log2u(uintmax_t sz) return l - 1; } +/* + * Initialize the global state for GIT_ALLOC_LIMIT at an appropriate + * time so it can be effective for safe allocation methods. + */ +void initialize_git_alloc_limit(void); + #endif /* WRAPPER_H */