fix(lib,scheduler): allocation constructors and allocators report OOM instead of building NULL-name records (bug-1882)

make_alloc() and make_host_alloc() assigned strdup() results unchecked,
so under memory exhaustion they returned a record whose resname/reserve/
host was NULL; merge_job_alloc()'s same_alloc_slot() (bug-1863) then
strcmp()'d it (ASAN SEGV at xcealloc.c:337). make_alloc() also passed a
NULL resname straight to strdup().
- xcealloc.c: make_alloc()/make_host_alloc() reject a NULL name (EINVAL)
  and check every strdup(), freeing what they built and returning NULL
  (ENOMEM); on failure make_host_alloc()'s caller keeps `allocs`.
  dup_host_alloc() frees its partial copy on failure instead of leaking.
- scheduler.c sched_make_allocs(): a record that cannot be built fails
  the whole array (NULL, ENOMEM) instead of leaving a NULL entry.
- seqjob/parjob my_alloc(): check every constructor; on failure free
  what was built (parjob also rolls back the remaining claims) and
  return -ENOMEM with ctx->alloc NULL -- the caller moves ctx.alloc onto
  the job whatever the rc.

Tests: new xcealloc_oom_test (8, --wrap=strdup) and
sched_make_allocs_oom_test (real scheduler_srcs + --wrap=strdup); a new
-ENOMEM test in the seqjob and parjob allocator suites. All RED before
(SEGV / "Expected NULL" / "Expected -12 Was 1"), GREEN after.
xce-build.sh all asan: 115/115 suites OK, Go tests OK.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Rongsong Shen 2026-09-28 08:49:51 +00:00
parent c02d8e5cf6
commit f9146864a3
Signed by untrusted user: rshen
GPG key ID: 80533536B0EB651C
10 changed files with 563 additions and 12 deletions

View file

@ -0,0 +1,210 @@
/*
* bug-1882 regression: make_alloc()/make_host_alloc() must never return a
* record whose resname/reserve/host string is NULL. Before the fix they
* assigned strdup() results unchecked, so an allocation failure produced a
* half-built record, and merge_job_alloc()'s same_alloc_slot() later
* strcmp()'d the NULL resname (SEGV at xcealloc.c:337).
*
* `-Wl,--wrap=strdup` (lib/meson.build, same linker-interposition idiom as
* bug-1843's `--wrap=realloc` in sched_record_lifecycle_test) lets a test
* make the Nth strdup() call return NULL. libxce is a static library, so the
* wrap reaches the calls inside xcealloc.c too. __wrap_strdup() passes
* through to __real_strdup() unless a test arms it; this is a separate
* binary so xcelib_test's other tests are untouched.
*/
#include <errno.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include "unity.h"
#include "xce/types.h"
#include "xcealloc.h"
#include "controller.capnp.h"
extern char* __real_strdup(const char* s);
static int g_strdup_fail_at = -1; /* -1 == disabled */
static int g_strdup_calls = 0;
char* __wrap_strdup(const char* s)
{
if (g_strdup_fail_at >= 0) {
int this_call = g_strdup_calls++;
if (this_call == g_strdup_fail_at) {
return NULL;
}
}
return __real_strdup(s);
}
/* Make the strdup() call with 0-based index `n` (counted from now) fail. */
static void fail_strdup_at(int n)
{
g_strdup_calls = 0;
g_strdup_fail_at = n;
}
void setUp(void)
{
g_strdup_fail_at = -1;
g_strdup_calls = 0;
}
void tearDown(void)
{
g_strdup_fail_at = -1;
}
static void test_make_alloc_null_resname_returns_null(void)
{
xce_alloc_t* a;
errno = 0;
a = make_alloc(NULL, (char*)"", 1, 1.0f, 0);
TEST_ASSERT_NULL(a);
TEST_ASSERT_EQUAL_INT(EINVAL, errno);
}
static void test_make_alloc_resname_strdup_failure_returns_null(void)
{
xce_alloc_t* a;
fail_strdup_at(0);
errno = 0;
a = make_alloc((char*)"mem", (char*)"", 1, 1.0f, 0);
TEST_ASSERT_NULL(a);
TEST_ASSERT_EQUAL_INT(ENOMEM, errno);
}
static void test_make_alloc_reserve_strdup_failure_returns_null(void)
{
xce_alloc_t* a;
fail_strdup_at(1);
errno = 0;
a = make_alloc((char*)"mem", (char*)"r1", 1, 1.0f, 0);
TEST_ASSERT_NULL(a);
TEST_ASSERT_EQUAL_INT(ENOMEM, errno);
}
static void test_make_alloc_default_reserve_strdup_failure_returns_null(void)
{
xce_alloc_t* a;
fail_strdup_at(1); /* the strdup("") for a NULL reserve */
a = make_alloc((char*)"mem", NULL, 1, 1.0f, 0);
TEST_ASSERT_NULL(a);
}
static void test_make_alloc_success_sets_every_string(void)
{
xce_alloc_t* a = make_alloc((char*)"mem", NULL, 1, 1.0f, 0);
TEST_ASSERT_NOT_NULL(a);
TEST_ASSERT_EQUAL_STRING("mem", a->resname);
TEST_ASSERT_EQUAL_STRING("", a->reserve);
free_ResAlloc_ptr(&a);
}
static void test_make_host_alloc_null_host_returns_null(void)
{
xce_host_alloc_t* h;
errno = 0;
h = make_host_alloc(NULL, 0, NULL);
TEST_ASSERT_NULL(h);
TEST_ASSERT_EQUAL_INT(EINVAL, errno);
}
/* On failure the caller keeps ownership of `allocs`: freeing it afterwards
* must be neither a double free nor a leak (ASAN checks both). */
static void test_make_host_alloc_strdup_failure_leaves_allocs_to_caller(void)
{
xce_alloc_t** allocs;
xce_host_alloc_t* h;
allocs = make_allocs(1, make_alloc((char*)"mem", NULL, 1, 1.0f, 0));
TEST_ASSERT_NOT_NULL(allocs);
fail_strdup_at(0);
errno = 0;
h = make_host_alloc((char*)"h0", 1, allocs);
TEST_ASSERT_NULL(h);
TEST_ASSERT_EQUAL_INT(ENOMEM, errno);
free_ResAlloc_list(1, allocs);
}
static xce_job_alloc_t* one_host_job(const char* host)
{
xce_alloc_t** allocs;
xce_host_alloc_t** hallocs;
allocs = make_allocs(1, make_alloc((char*)"mem", NULL, 1, 1.0f, 0));
hallocs = make_host_allocs(1, make_host_alloc((char*)host, 1, allocs));
return make_job_alloc(7, 1, hallocs);
}
/* merge_job_alloc() deep-copies a new src host via dup_host_alloc(), which
* builds it with make_alloc()/make_host_alloc(): a failed strdup() there must
* surface as -ENOMEM, never as a host or resource with a NULL name left in
* dst for the next merge to strcmp(). */
static void test_merge_job_alloc_strdup_failure_reports_enomem(void)
{
int n;
/* strdup() calls when copying one new host with one resource:
* 0 = resname, 1 = reserve, 2 = host. Fail each in turn. */
for (n = 0; n < 3; n++) {
xce_job_alloc_t* dst = one_host_job("h0");
xce_job_alloc_t* src = one_host_job("h1");
int rc;
int i;
TEST_ASSERT_NOT_NULL(dst);
TEST_ASSERT_NOT_NULL(src);
fail_strdup_at(n);
rc = merge_job_alloc(dst, src);
g_strdup_fail_at = -1;
TEST_ASSERT_EQUAL_INT_MESSAGE(-ENOMEM, rc, "merge must report OOM");
for (i = 0; i < dst->nhosts; i++) {
TEST_ASSERT_NOT_NULL(dst->allocs[i]);
TEST_ASSERT_NOT_NULL(dst->allocs[i]->host);
}
free_JobAlloc_ptr(&dst);
free_JobAlloc_ptr(&src);
}
}
int main(void)
{
UNITY_BEGIN();
printf("\n>> xcealloc OOM test (bug-1882)\n\n");
RUN_TEST(test_make_alloc_null_resname_returns_null);
RUN_TEST(test_make_alloc_resname_strdup_failure_returns_null);
RUN_TEST(test_make_alloc_reserve_strdup_failure_returns_null);
RUN_TEST(test_make_alloc_default_reserve_strdup_failure_returns_null);
RUN_TEST(test_make_alloc_success_sets_every_string);
RUN_TEST(test_make_host_alloc_null_host_returns_null);
RUN_TEST(test_make_host_alloc_strdup_failure_leaves_allocs_to_caller);
RUN_TEST(test_merge_job_alloc_strdup_failure_reports_enomem);
return UNITY_END();
}