From 750e4890028bddf8069fe90ee5686428d8a6ecdf Mon Sep 17 00:00:00 2001
From: Nikolay Shirokovskiy <nshirokovskiy@tarantool.org>
Date: Mon, 19 Jun 2023 11:00:28 +0300
Subject: [PATCH] fiber: fix heap-buffer-overflow in
fiber_stack_watermark_create
Fiber flags are initialized after fiber stack creation. As result
currently check for custom stack in fiber_stack_watermark_create does
not work. This leads to heap-buffer-overflow on putting watermark
if custom stack size is less than FIBER_STACK_SIZE_WATERMARK.
Close #9026
NO_DOC=bugfix
---
.../gh-9026-fix-heap-buffer-overflow.md | 3 ++
src/lib/core/fiber.c | 20 ++++++-----
test/unit/fiber_stack.c | 34 ++++++++++++++++---
test/unit/suite.ini | 2 +-
4 files changed, 44 insertions(+), 15 deletions(-)
create mode 100644 changelogs/unreleased/gh-9026-fix-heap-buffer-overflow.md
diff --git a/changelogs/unreleased/gh-9026-fix-heap-buffer-overflow.md b/changelogs/unreleased/gh-9026-fix-heap-buffer-overflow.md
new file mode 100644
index 0000000000..02e70fa305
--- /dev/null
+++ b/changelogs/unreleased/gh-9026-fix-heap-buffer-overflow.md
@@ -0,0 +1,3 @@
+## bugfix/core
+
+* Fixed a heap-buffer-overflow bug in fiber creation code (gh-9026).
diff --git a/src/lib/core/fiber.c b/src/lib/core/fiber.c
index f56e764de6..377bf626de 100644
--- a/src/lib/core/fiber.c
+++ b/src/lib/core/fiber.c
@@ -1189,12 +1189,13 @@ fiber_stack_recycle(struct fiber *fiber)
* Initialize fiber stack watermark.
*/
static void
-fiber_stack_watermark_create(struct fiber *fiber)
+fiber_stack_watermark_create(struct fiber *fiber,
+ const struct fiber_attr *fiber_attr)
{
assert(fiber->stack_watermark == NULL);
/* No tracking on custom stacks for simplicity. */
- if (fiber->flags & FIBER_CUSTOM_STACK)
+ if (fiber_attr->flags & FIBER_CUSTOM_STACK)
return;
/*
@@ -1230,9 +1231,11 @@ fiber_stack_recycle(struct fiber *fiber)
}
static void
-fiber_stack_watermark_create(struct fiber *fiber)
+fiber_stack_watermark_create(struct fiber *fiber,
+ const struct fiber_attr *fiber_attr)
{
(void)fiber;
+ (void)fiber_attr;
}
#endif /* HAVE_MADV_DONTNEED */
@@ -1279,10 +1282,10 @@ fiber_stack_destroy(struct fiber *fiber, struct slab_cache *slabc)
}
static int
-fiber_stack_create(struct fiber *fiber, struct slab_cache *slabc,
- size_t stack_size)
+fiber_stack_create(struct fiber *fiber, const struct fiber_attr *fiber_attr,
+ struct slab_cache *slabc)
{
- stack_size -= slab_sizeof();
+ size_t stack_size = fiber_attr->stack_size - slab_sizeof();
fiber->stack_slab = slab_get(slabc, stack_size);
if (fiber->stack_slab == NULL) {
@@ -1329,7 +1332,7 @@ fiber_stack_create(struct fiber *fiber, struct slab_cache *slabc,
return -1;
}
- fiber_stack_watermark_create(fiber);
+ fiber_stack_watermark_create(fiber, fiber_attr);
return 0;
}
@@ -1380,8 +1383,7 @@ fiber_new_ex(const char *name, const struct fiber_attr *fiber_attr,
fiber->storage.lua.storage_ref = FIBER_LUA_NOREF;
fiber->storage.lua.fid_ref = FIBER_LUA_NOREF;
- if (fiber_stack_create(fiber, &cord()->slabc,
- fiber_attr->stack_size)) {
+ if (fiber_stack_create(fiber, fiber_attr, &cord()->slabc)) {
mempool_free(&cord->fiber_mempool, fiber);
return NULL;
}
diff --git a/test/unit/fiber_stack.c b/test/unit/fiber_stack.c
index 3c7cf881ce..462540762c 100644
--- a/test/unit/fiber_stack.c
+++ b/test/unit/fiber_stack.c
@@ -31,18 +31,41 @@ main_f(va_list ap)
struct errinj *inj;
struct fiber *fiber;
int fiber_count = fiber_count_total();
- struct fiber_attr *fiber_attr = fiber_attr_new();
+ struct fiber_attr *fiber_attr;
header();
+#ifdef NDEBUG
+ plan(1);
+#else
plan(11);
+#endif
+
+ /*
+ * gh-9026. Stack size crafted to be close to 64k so we should
+ * hit red zone around stack when writing watermark if bug is not
+ * fixed.
+ *
+ * The test is placed at the beginning because stderr is redirected
+ * to /dev/null at the end of the test and ASAN diagnostic will
+ * not be visible if the test will be placed at the end.
+ */
+ fiber_attr = fiber_attr_new();
+ fiber_attr_setstacksize(fiber_attr, (64 << 10) - 128);
+ fiber = fiber_new_ex("gh-9026", fiber_attr, noop_f);
+ fiber_set_joinable(fiber, true);
+ fiber_start(fiber);
+ fiber_join(fiber);
+ fiber_attr_delete(fiber_attr);
/*
* Check the default fiber stack size value.
*/
+ fiber_attr = fiber_attr_new();
ok(default_attr.stack_size == FIBER_STACK_SIZE_DEFAULT,
"fiber_attr: the default stack size is %ld, but %d is set via CMake",
default_attr.stack_size, FIBER_STACK_SIZE_DEFAULT);
+#ifndef NDEBUG
/*
* Set non-default stack size to prevent reusing of an
* existing fiber.
@@ -124,12 +147,13 @@ main_f(va_list ap)
cord_collect_garbage(cord());
ok(fiber_count_total() == fiber_count, "fiber is deleted");
+#endif /* ifndef NDEBUG */
fiber_attr_delete(fiber_attr);
- footer();
-
ev_break(loop(), EVBREAK_ALL);
- return check_plan();
+
+ footer();
+ return 0;
}
int main()
@@ -142,5 +166,5 @@ int main()
ev_run(loop(), 0);
fiber_free();
memory_free();
- return 0;
+ return check_plan();
}
diff --git a/test/unit/suite.ini b/test/unit/suite.ini
index d16ba413bc..cc79666c39 100644
--- a/test/unit/suite.ini
+++ b/test/unit/suite.ini
@@ -2,5 +2,5 @@
core = unittest
description = unit tests
disabled = snap_quorum_delay.test
-release_disabled = fiber_stack.test swim_errinj.test
+release_disabled = swim_errinj.test
is_parallel = True
--
GitLab