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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AgYRU3zDTqNGV5Jk8aiKLg
This commit is contained in:
parent
5e68fcc0d8
commit
002075030b
4 changed files with 103 additions and 3 deletions
|
|
@ -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',
|
||||
]
|
||||
|
||||
|
|
|
|||
|
|
@ -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();
|
||||
}
|
||||
|
|
|
|||
97
src/lib/testsuite/test_xcealloc.c
Normal file
97
src/lib/testsuite/test_xcealloc.c
Normal file
|
|
@ -0,0 +1,97 @@
|
|||
#include <stdio.h>
|
||||
#include <stdlib.h>
|
||||
#include <string.h>
|
||||
|
||||
#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);
|
||||
}
|
||||
|
|
@ -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;
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue