Skip to content

Commit

Permalink
fiber: fix heap-buffer-overflow in fiber_stack_watermark_create
Browse files Browse the repository at this point in the history
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 tarantool#9026

NO_DOC=bugfix
  • Loading branch information
nshy committed Aug 25, 2023
1 parent 2408423 commit 23e2c81
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 15 deletions.
3 changes: 3 additions & 0 deletions 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).
20 changes: 11 additions & 9 deletions src/lib/core/fiber.c
Expand Up @@ -1190,12 +1190,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;

/*
Expand Down Expand Up @@ -1231,9 +1232,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 */

Expand Down Expand Up @@ -1280,10 +1283,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) {
Expand Down Expand Up @@ -1330,7 +1333,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;
}

Expand Down Expand Up @@ -1381,8 +1384,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;
}
Expand Down
34 changes: 29 additions & 5 deletions test/unit/fiber_stack.c
Expand Up @@ -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.
Expand Down Expand Up @@ -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()
Expand All @@ -142,5 +166,5 @@ int main()
ev_run(loop(), 0);
fiber_free();
memory_free();
return 0;
return check_plan();
}
2 changes: 1 addition & 1 deletion test/unit/suite.ini
Expand Up @@ -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

0 comments on commit 23e2c81

Please sign in to comment.