Merge pull request #5232 from pks-t/pks/buffer-ensure-size-oom buffer: fix writes into out-of-memory buffers
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112
diff --git a/src/buffer.c b/src/buffer.c
index 51fb48a..61cf967 100644
--- a/src/buffer.c
+++ b/src/buffer.c
@@ -18,7 +18,8 @@ char git_buf__initbuf[1];
char git_buf__oom[1];
#define ENSURE_SIZE(b, d) \
- if ((d) > (b)->asize && git_buf_grow((b), (d)) < 0)\
+ if ((b)->ptr == git_buf__oom || \
+ ((d) > (b)->asize && git_buf_grow((b), (d)) < 0))\
return -1;
@@ -58,20 +59,26 @@ int git_buf_try_grow(
new_ptr = NULL;
} else {
new_size = buf->asize;
+ /*
+ * Grow the allocated buffer by 1.5 to allow
+ * re-use of memory holes resulting from the
+ * realloc. If this is still too small, then just
+ * use the target size.
+ */
+ if ((new_size = (new_size << 1) - (new_size >> 1)) < target_size)
+ new_size = target_size;
new_ptr = buf->ptr;
}
- /* grow the buffer size by 1.5, until it's big enough
- * to fit our target size */
- while (new_size < target_size)
- new_size = (new_size << 1) - (new_size >> 1);
-
/* round allocation up to multiple of 8 */
new_size = (new_size + 7) & ~7;
if (new_size < buf->size) {
- if (mark_oom)
+ if (mark_oom) {
+ if (buf->ptr && buf->ptr != git_buf__initbuf)
+ git__free(buf->ptr);
buf->ptr = git_buf__oom;
+ }
git_error_set_oom();
return -1;
diff --git a/tests/core/buffer.c b/tests/core/buffer.c
index 58c98cb..4e895af 100644
--- a/tests/core/buffer.c
+++ b/tests/core/buffer.c
@@ -231,9 +231,9 @@ check_buf_append(
git_buf_puts(&tgt, data_b);
cl_assert(git_buf_oom(&tgt) == 0);
cl_assert_equal_s(expected_data, git_buf_cstr(&tgt));
- cl_assert(tgt.size == expected_size);
+ cl_assert_equal_i(tgt.size, expected_size);
if (expected_asize > 0)
- cl_assert(tgt.asize == expected_asize);
+ cl_assert_equal_i(tgt.asize, expected_asize);
git_buf_dispose(&tgt);
}
@@ -308,7 +308,7 @@ void test_core_buffer__5(void)
REP16("x") REP16("o"), 32, 40);
check_buf_append(test_4096, "", test_4096, 4096, 4104);
- check_buf_append(test_4096, test_4096, test_8192, 8192, 9240);
+ check_buf_append(test_4096, test_4096, test_8192, 8192, 8200);
/* check sequences of appends */
check_buf_append_abc("a", "b", "c",
@@ -1204,3 +1204,39 @@ void test_core_buffer__dont_grow_borrowed(void)
cl_git_fail_with(GIT_EINVALID, git_buf_grow(&buf, 1024));
}
+
+void test_core_buffer__dont_hit_infinite_loop_when_resizing(void)
+{
+ git_buf buf = GIT_BUF_INIT;
+
+ cl_git_pass(git_buf_puts(&buf, "foobar"));
+ /*
+ * We do not care whether this succeeds or fails, which
+ * would depend on platform-specific allocation
+ * semantics. We only want to know that the function
+ * actually returns.
+ */
+ (void)git_buf_try_grow(&buf, SIZE_MAX, true);
+
+ git_buf_dispose(&buf);
+}
+
+void test_core_buffer__avoid_printing_into_oom_buffer(void)
+{
+ git_buf buf = GIT_BUF_INIT;
+
+ /* Emulate OOM situation with a previous allocation */
+ buf.asize = 8;
+ buf.ptr = git_buf__oom;
+
+ /*
+ * Print the same string again. As the buffer still has
+ * an `asize` of 8 due to the previous print,
+ * `ENSURE_SIZE` would not try to reallocate the array at
+ * all. As it didn't explicitly check for `git_buf__oom`
+ * in earlier versions, this would've resulted in it
+ * returning successfully and thus `git_buf_puts` would
+ * just print into the `git_buf__oom` array.
+ */
+ cl_git_fail(git_buf_puts(&buf, "foobar"));
+}