From 16b7aa4b602e4d5fa383d0d71cbd3f6c07ffe80d Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Wed, 30 Sep 2026 03:34:57 +0000 Subject: [PATCH] fix(lib,scheduler): zarray_get rejects negative indexes; info resource bounds by capacity (bug-1824) zarray_get_ex() checked only idx >= caps, so a negative index read array->data[idx] out of bounds -- reachable through a resource whose registration failed and kept idx == -1 (xcelimit.c add_vres()). It now returns NULL for idx < 0. repl_info_resource_one() bounded a shared resource's slot by sharedInst.count / sharedInUse.count, the number of populated slots, not the capacity: a populated slot above that count showed as value=(none). It now relies on zarray_get()'s own bounds check. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/lib/testsuite/test_zarray.c | 20 ++++++ src/lib/zarray.c | 3 +- src/scheduler/src/repl_info.c | 15 ++--- .../testsuite/test_repl_resource_table.c | 63 +++++++++++++++++++ 4 files changed, 90 insertions(+), 11 deletions(-) diff --git a/src/lib/testsuite/test_zarray.c b/src/lib/testsuite/test_zarray.c index 6a27de66..3fd790a0 100644 --- a/src/lib/testsuite/test_zarray.c +++ b/src/lib/testsuite/test_zarray.c @@ -210,6 +210,25 @@ static void test_zarray_fini_resets_bounds_for_reuse() zarray_fini(&array); } + +/* bug-1824: zarray_get_ex() only checked idx >= caps, so a negative index + * read array->data[idx] out of bounds. A resource whose registration fails + * keeps idx == -1 (xcelimit.c add_vres()), so a caller can pass one. */ +static void test_zarray_get_negative_index_returns_null(void) +{ + zarray_t z; + int v = 7; + + zarray_init(&z, 4, NULL); + TEST_ASSERT_TRUE(zarray_put(&z, 0, &v) >= 0); + + TEST_ASSERT_NULL(zarray_get(&z, -1)); + TEST_ASSERT_NULL(zarray_get(&z, -1000)); + TEST_ASSERT_EQUAL_PTR(&v, zarray_get(&z, 0)); + + zarray_fini(&z); +} + void test_zarray(void) { printf("\n>> zarray test\n\n"); @@ -221,4 +240,5 @@ void test_zarray(void) RUN_TEST(test_zarray_put_get_vec); RUN_TEST(test_zarray_iter); RUN_TEST(test_zarray_fini_resets_bounds_for_reuse); + RUN_TEST(test_zarray_get_negative_index_returns_null); } diff --git a/src/lib/zarray.c b/src/lib/zarray.c index fbee96fd..13e230af 100644 --- a/src/lib/zarray.c +++ b/src/lib/zarray.c @@ -350,7 +350,8 @@ int zarray_get_ex(zarray_t* array, int count, int* idxs, void** data) for (i = 0; i < count; i++) { int idx = idxs[i]; - if (idx >= array->caps) { + /* bug-1824: a negative idx read array->data[idx] out of bounds */ + if ((idx < 0) || (idx >= array->caps)) { data[i] = NULL; } else { data[i] = array->data[idx]; diff --git a/src/scheduler/src/repl_info.c b/src/scheduler/src/repl_info.c index 305e480d..40cfe110 100644 --- a/src/scheduler/src/repl_info.c +++ b/src/scheduler/src/repl_info.c @@ -1013,12 +1013,10 @@ static void repl_info_resource_one(xce_sched_sim_driver_t* sim, return; } - if ((r->idx < 0) || (r->idx >= sim->core.sharedInst.count)) { - fprintf(stdout, "[sim] value=(none)\n"); - return; - } - { + /* bug-1824: zarray_get() bounds-checks idx (negative included) + * against the capacity; .count counts populated slots, so bounding + * by it hid a populated slot above it. */ xce_val_t* v = (xce_val_t*)zarray_get(&(sim->core.sharedInst), r->idx); if (v == NULL) { @@ -1044,11 +1042,8 @@ static void repl_info_resource_one(xce_sched_sim_driver_t* sim, xce_val_t* used_v = NULL; double used = 0.0; - if ((r->idx >= 0) && - (r->idx < sim->core.sharedInUse.count)) { - used_v = (xce_val_t*)zarray_get(&(sim->core.sharedInUse), - r->idx); - } + used_v = (xce_val_t*)zarray_get(&(sim->core.sharedInUse), + r->idx); if ((used_v != NULL) && (used_v->which == Value_u_fvalue)) { used = used_v->u.fvalue; } diff --git a/src/scheduler/testsuite/test_repl_resource_table.c b/src/scheduler/testsuite/test_repl_resource_table.c index bf7cb6d9..882cfd08 100644 --- a/src/scheduler/testsuite/test_repl_resource_table.c +++ b/src/scheduler/testsuite/test_repl_resource_table.c @@ -1858,6 +1858,68 @@ test_info_resource_load_metric_whole_number_omits_trailing_zeros(void) "zeros: PASS\n"); } +/* + * bug-1824: info resource bounded a shared resource's slot by + * sharedInst.count -- the number of POPULATED slots, not the array's + * capacity -- so a populated slot at an index >= .count (a sparsely filled + * array) was reported as "value=(none)". Same shape as bug-1688's + * stale-count test for info worker. + */ +static void test_info_resource_shows_value_when_slot_is_above_count(void) +{ + xce_sched_sim_driver_t sim; + xce_core_event_t events[1]; + xce_resource_t wire_load; + xce_resource_t* r_load; + xce_val_t val_load; + xce_resval_t rv_load; + xce_resval_t* rv_ptrs[1]; + char* captured; + int rc; + + memset(&sim, 0, sizeof(sim)); + rc = xce_sched_core_init(&(sim.core)); + TEST_ASSERT_EQUAL_INT(0, rc); + + memset(&wire_load, 0, sizeof(wire_load)); + wire_load.name = "cluster_load1m"; + wire_load.valtype = XCE_VAL_NUMERIC; + wire_load.flags = XCE_RESF_SHARED; + r_load = makeResource(sim.core.resmgr, &wire_load); + TEST_ASSERT_NOT_NULL(r_load); + + memset(&val_load, 0, sizeof(val_load)); + val_load.which = Value_u_fvalue; + val_load.u.fvalue = 5.0; + memset(&rv_load, 0, sizeof(rv_load)); + rv_load.name = "cluster_load1m"; + rv_load.val = &val_load; + rv_ptrs[0] = &rv_load; + + memset(events, 0, sizeof(events)); + events[0].type = XCE_CORE_EVENT_RESVALS_FULL; + events[0].u.resvals_full.n = 1; + events[0].u.resvals_full.resvals = rv_ptrs; + + rc = xce_core_push_events(&(sim.core), events, 1); + TEST_ASSERT_EQUAL_INT(0, rc); + TEST_ASSERT_NOT_NULL(zarray_get(&(sim.core.sharedInst), r_load->idx)); + + /* The slot stays populated; only .count under-reports it. */ + sim.core.sharedInst.count = r_load->idx; + + captured = capture_dispatch(&sim, "info resource cluster_load1m"); + + TEST_ASSERT_NOT_NULL_MESSAGE(strstr(captured, "value=5\n"), + "bug-1824: a populated shared slot at an index >= sharedInst.count " + "must still display its value"); + + free(captured); + + printf("test_info_resource_shows_value_when_slot_is_above_count: " + "PASS\n"); +} + /* * AC 6.2 'both' / Req 6.3 default, `info resource `: same * units_mode-aware bytes rendering as info worker's own version above, @@ -2360,6 +2422,7 @@ int main(void) RUN_TEST(test_info_worker_assigned_job_listing_falls_back_per_job_when_slots_differ); RUN_TEST(test_info_resource_groups_resources_into_labeled_table_sections_in_order); RUN_TEST(test_info_resource_load_metric_whole_number_omits_trailing_zeros); + RUN_TEST(test_info_resource_shows_value_when_slot_is_above_count); RUN_TEST(test_info_resource_units_mode_both_renders_raw_and_human_for_bytes_resource); RUN_TEST(test_info_resource_units_mode_both_renders_raw_and_human_for_consumable_bytes_resource);