From 002075030bd18d8b9c7b2dc8b67f1e0c7c3e7820 Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Thu, 27 Aug 2026 08:43:33 +0000 Subject: [PATCH] fix(lib): heap buffer overflow in make_job_account() for multi-host jobs (bug-1722) make_job_account() allocates account->accounts via calloc(1, sizeof(xce_host_account_t*)) -- room for exactly 1 host pointer -- but its own loop writes account->accounts[i] for i up to jalloc->nhosts-1, overflowing the heap allocation for any job spanning more than one host. Already reachable from live production dispatch (core_events.c, schedobj.c, scheduler.c) for any multi-host job -- found while verifying viability of a proposed scheduler snapshot-allocation-capture fix, not from a user report. Sibling function make_host_allocs() has the identical bug shape (calloc(1, ...) vs a loop writing up to nhosts-1); every current call site happens to pass nhosts=1 so it has never actually overflowed in production, but it's the same live footgun in a public exported helper. Fixed alongside since it's the identical bug class in the same file. New test_xcealloc.c drives a 2-host allocation through both functions, reproducing the exact ASAN heap-buffer-overflow at xcealloc.c:188 pre-fix (RED), passing post-fix (GREEN). Both calloc(1, ...) sites changed to size the allocation by the actual host count. Full regression: libxce 303/303 (was 301, +2 new), scheduler 53/54 (1 pre-existing unrelated bug-1560 flake), controller 1211/1213 (2 pre-existing unrelated bug-1561 overlayfs flakes) -- zero new failures. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01AgYRU3zDTqNGV5Jk8aiKLg --- src/lib/meson.build | 1 + src/lib/testsuite/test.c | 2 + src/lib/testsuite/test_xcealloc.c | 97 +++++++++++++++++++++++++++++++ src/lib/xcealloc.c | 6 +- 4 files changed, 103 insertions(+), 3 deletions(-) create mode 100644 src/lib/testsuite/test_xcealloc.c diff --git a/src/lib/meson.build b/src/lib/meson.build index 3dbc3af4..ea2ba3cc 100644 --- a/src/lib/meson.build +++ b/src/lib/meson.build @@ -188,6 +188,7 @@ libxce_test_src = ['testsuite/test.c', 'testsuite/test_wal_wire.c', 'testsuite/test_ctrlstatus.c', 'testsuite/test_vtask_admission.c', + 'testsuite/test_xcealloc.c', 'raft/raft_sim.c', ] diff --git a/src/lib/testsuite/test.c b/src/lib/testsuite/test.c index f9490398..1be1282d 100644 --- a/src/lib/testsuite/test.c +++ b/src/lib/testsuite/test.c @@ -51,6 +51,7 @@ extern void test_raft(void); extern void test_wal_wire(void); extern void test_ctrlstatus(void); extern void test_vtask_admission(void); +extern void test_xcealloc(void); void setUp(void) { return; } @@ -171,6 +172,7 @@ int main(int argc, char* argv[]) TEST(test_wal_wire); TEST(test_ctrlstatus); TEST(test_vtask_admission); + TEST(test_xcealloc); return UNITY_END(); } diff --git a/src/lib/testsuite/test_xcealloc.c b/src/lib/testsuite/test_xcealloc.c new file mode 100644 index 00000000..253f73b4 --- /dev/null +++ b/src/lib/testsuite/test_xcealloc.c @@ -0,0 +1,97 @@ +#include +#include +#include + +#include "misc.h" +#include "unity.h" +#include "xce/types.h" +#include "xcealloc.h" + +/* Heap buffer overflow: make_job_account() allocates account->accounts with + * room for exactly 1 xce_host_account_t* (xcealloc.c, calloc(1, ...)) but + * its own loop writes account->accounts[i] for i up to jalloc->nhosts-1 -- + * an out-of-bounds write for any job spanning more than one host. Already + * reachable from live production dispatch (core_events.c, schedobj.c, + * scheduler.c) for any multi-host job. This test drives a 2-host allocation + * through make_job_account() so ASAN's heap-buffer-overflow detector catches + * the out-of-bounds write directly. */ +static void test_make_job_account_multi_host_no_overflow(void) +{ + xce_alloc_t* a0; + xce_alloc_t* a1; + xce_host_alloc_t* h0; + xce_host_alloc_t* h1; + xce_host_alloc_t** hallocs; + xce_job_alloc_t* jalloc; + xce_job_account_t* account; + + a0 = make_alloc((char*)"slots", NULL, 0, 1.0f, 0); + a1 = make_alloc((char*)"slots", NULL, 0, 1.0f, 0); + TEST_ASSERT_NOT_NULL(a0); + TEST_ASSERT_NOT_NULL(a1); + + h0 = make_host_alloc((char*)"host0", 1, make_allocs(1, a0)); + h1 = make_host_alloc((char*)"host1", 1, make_allocs(1, a1)); + TEST_ASSERT_NOT_NULL(h0); + TEST_ASSERT_NOT_NULL(h1); + + hallocs = (xce_host_alloc_t**)calloc(2, sizeof(*hallocs)); + hallocs[0] = h0; + hallocs[1] = h1; + + jalloc = make_job_alloc(4242, 2, hallocs); + TEST_ASSERT_NOT_NULL(jalloc); + TEST_ASSERT_EQUAL_INT(2, jalloc->nhosts); + + account = make_job_account(jalloc); + TEST_ASSERT_NOT_NULL(account); + TEST_ASSERT_EQUAL_INT(2, account->nhosts); + TEST_ASSERT_NOT_NULL(account->accounts); + TEST_ASSERT_NOT_NULL(account->accounts[0]); + TEST_ASSERT_NOT_NULL(account->accounts[1]); + TEST_ASSERT_EQUAL_STRING("host0", account->accounts[0]->host); + TEST_ASSERT_EQUAL_STRING("host1", account->accounts[1]->host); + + free_job_account(account); +} + +/* Same bug shape, sibling function: make_host_allocs() allocates its result + * array with calloc(1, ...) regardless of nhosts, then writes hallocs[i] for + * i up to nhosts-1. Every current call site passes nhosts=1 (so this has + * never actually overflowed in production), but it is a public, exported + * helper -- a future 2+-host caller would silently corrupt the heap. Fixed + * alongside make_job_account() since it is the identical bug class in the + * same file. */ +static void test_make_host_allocs_multi_host_no_overflow(void) +{ + xce_alloc_t* a0; + xce_alloc_t* a1; + xce_host_alloc_t* h0; + xce_host_alloc_t* h1; + xce_host_alloc_t** hallocs; + + a0 = make_alloc((char*)"slots", NULL, 0, 1.0f, 0); + a1 = make_alloc((char*)"slots", NULL, 0, 1.0f, 0); + TEST_ASSERT_NOT_NULL(a0); + TEST_ASSERT_NOT_NULL(a1); + + h0 = make_host_alloc((char*)"host0", 1, make_allocs(1, a0)); + h1 = make_host_alloc((char*)"host1", 1, make_allocs(1, a1)); + TEST_ASSERT_NOT_NULL(h0); + TEST_ASSERT_NOT_NULL(h1); + + hallocs = make_host_allocs(2, h0, h1); + TEST_ASSERT_NOT_NULL(hallocs); + TEST_ASSERT_NOT_NULL(hallocs[0]); + TEST_ASSERT_NOT_NULL(hallocs[1]); + TEST_ASSERT_EQUAL_STRING("host0", hallocs[0]->host); + TEST_ASSERT_EQUAL_STRING("host1", hallocs[1]->host); +} + +void test_xcealloc(void) +{ + printf("\n>> xcealloc test\n\n"); + + RUN_TEST(test_make_job_account_multi_host_no_overflow); + RUN_TEST(test_make_host_allocs_multi_host_no_overflow); +} diff --git a/src/lib/xcealloc.c b/src/lib/xcealloc.c index e0137045..b6785240 100644 --- a/src/lib/xcealloc.c +++ b/src/lib/xcealloc.c @@ -131,7 +131,7 @@ xce_host_alloc_t** make_host_allocs(int nhosts, ...) return NULL; } - hallocs = (xce_host_alloc_t**)calloc(1, sizeof(*hallocs)); + hallocs = (xce_host_alloc_t**)calloc((size_t)nhosts, sizeof(*hallocs)); if (hallocs == NULL) { return NULL; } @@ -177,8 +177,8 @@ xce_job_account_t* make_job_account(xce_job_alloc_t* jalloc) return NULL; } - account->accounts - = (xce_host_account_t**)calloc(1, sizeof(xce_host_account_t*)); + account->accounts = (xce_host_account_t**)calloc( + (size_t)jalloc->nhosts, sizeof(xce_host_account_t*)); if (account->accounts == NULL) { FREEUP(account); return NULL;