50 lines
6.1 KiB
Markdown
50 lines
6.1 KiB
Markdown
---
|
|
id: "bug-1824"
|
|
status: "fixed"
|
|
status_updated: "2026-09-30"
|
|
status_note: ""
|
|
component: "scheduler"
|
|
severity: "wrong-result"
|
|
timestamp: "2026-09-17T02:45:00Z"
|
|
last_seen: "2026-09-19T09:00:00Z"
|
|
occurrences: 1
|
|
files: ["src/scheduler/src/repl_info.c (repl_info_resource_one)", "src/lib/zarray.c (zarray_get_ex)"]
|
|
tags: ["scheduler", "repl", "zarray", "bounds-check", "out-of-bounds-read", "latent", "discovered-during-scheduler-sim-diagnostics"]
|
|
related: []
|
|
fix_commits: ["16b7aa4b"]
|
|
legacy: true
|
|
---
|
|
|
|
# bug-1824: out-of-scope latent bug discovered while implementing scheduler-sim-diagnostics task 3.1 (new…
|
|
|
|
## Description
|
|
NOT A CRASH -- pre-existing, out-of-scope latent bug discovered while implementing scheduler-sim-diagnostics task 3.1 (new repl_info_limit() live-usage display). repl_info_resource_one() (src/scheduler/src/repl_info.c ~line 996) bounds-checks a resource's zarray index via `(r->idx < 0) || (r->idx >= sim->core.sharedInst.count)` before calling zarray_get() -- but zarray_t.count tracks the number of POPULATED slots (incremented only when a previously-NULL slot becomes non-NULL via zarray_put_ex/zarray_store), not the array's allocated capacity (.caps, grown by zarray_ensure_items). A sparsely-populated zarray (e.g. only a high index ever zarray_put() -- realistic if resources are registered out of index order, or an index is skipped) can have count=1 while a LOWER, unpopulated index is itself < count and would incorrectly pass this check and be read as though populated, or a HIGHER populated index can exceed .count and be incorrectly rejected as absent even though zarray_get()'s own .caps-based check would have safely returned it (or correctly returned NULL for a genuinely-unpopulated-but-in-capacity slot, since zarray_ensure_items() NULL-initializes newly-grown slots). Net effect: repl_info_resource_one() can report a resource as having no value ('value=(none)') when a real, populated value exists at that index but the array is sparse, OR (less likely) skip a real bounds-safety net zarray_get_ex() would otherwise provide.
|
|
|
|
## Environment
|
|
- OS: no /etc/os-release; Docker container; NixOS kernel; kernel Linux 6.18.33 x86_64
|
|
- libc: glibc 2.42-47 (interpreter /nix/store/wb6rhpznjfczwlwx23zmdrrw74bayxw4-glibc-2.42-47/lib/ld-linux-x86-64.so.2)
|
|
- Toolchain: gcc (GCC) 15.2.0, meson 1.9.1
|
|
- Build: asan; meson runs the suites with ASAN_OPTIONS=detect_leaks=0
|
|
- Code: found by code reading during scheduler-sim-diagnostics task 3.1 (Sep 2026); reproduced and fixed at fd61721b, 2026-09-29
|
|
- Deployment: n/a -- xce_scheduler simulator REPL (`info resource`) and libxce
|
|
|
|
## Reproduction
|
|
1. `bash src/tools/xce-build.sh scheduler` -- test_info_resource_shows_value_when_slot_is_above_count (src/scheduler/testsuite/test_repl_resource_table.c) registers a SHARED resource, sets its value, then lowers sim.core.sharedInst.count below the resource's populated slot and runs `info resource cluster_load1m`: it prints `value=(none)`.
|
|
2. `bash src/tools/xce-build.sh lib` -- test_zarray_get_negative_index_returns_null (src/lib/testsuite/test_zarray.c) calls zarray_get(z, -1): ASAN `heap-buffer-overflow ... READ of size 8 ... in zarray_get_ex ../lib/zarray.c:356`.
|
|
- Frequency: deterministic (both tests), 2026-09-29
|
|
- Evidence: the two tests above; build logs not kept
|
|
|
|
## Root cause
|
|
repl_info_resource_one()'s pre-check compares r->idx against sim->core.sharedInst.count (populated-slot count) instead of relying on zarray_get_ex()'s own bounds check, which is against .caps (allocated capacity) -- these two counters diverge for any sparsely-populated zarray. Confirmed by direct reading of src/lib/zarray.c: zarray_get_ex() (~line 341-362) checks only `idx >= array->caps` before dereferencing array->data[idx]; zarray_put_ex()/zarray_store() are the only writers of .count, incrementing it exclusively when filling a previously-NULL slot -- so .count is a 'high-water mark of contiguous-from-zero fill,' not a true capacity bound. Separately, and more seriously: neither the .count-based check NOR zarray_get_ex()'s own .caps check safely rejects a NEGATIVE idx -- `idx >= caps` is false for any negative idx when caps >= 0, so a negative idx falls through to `array->data[idx]`, a genuine out-of-bounds read. A negative idx is reachable in this codebase: add_vres() (src/lib/xcelimit.c ~407-435) unconditionally sets a newly-allocated xce_resource_t's ->idx to -1 before key_add() runs, only overwriting it with the real slot index on success -- a caller that reaches a resource-value read before/without that overwrite succeeding would pass a negative idx through. repl_info_resource_one()'s existing `r->idx < 0` half of its check does happen to guard this specific negative case correctly (by luck of ordering, not by understanding zarray's real contract) -- the count-vs-caps confusion is the part that is a genuine, distinct latent bug.
|
|
|
|
## Fix
|
|
src/lib/zarray.c: zarray_get_ex() returns NULL for idx < 0 as well as idx >= caps, so every zarray_get() caller is safe from a negative index. src/scheduler/src/repl_info.c: repl_info_resource_one() no longer pre-bounds the slot by sharedInst.count or sharedInUse.count (populated-slot counts); it relies on zarray_get()'s own capacity check, which returns NULL for an unpopulated or out-of-range slot. A search for other `idx < / >= <zarray>.count` bounds in scheduler, controller, lib, worker and harness found no other site.
|
|
|
|
## Verification
|
|
- Test: test_info_resource_shows_value_when_slot_is_above_count (src/scheduler/testsuite/test_repl_resource_table.c): RED before the fix (`bug-1824: a populated shared slot at an index >= sharedInst.count must still display its value`), GREEN after. test_zarray_get_negative_index_returns_null (src/lib/testsuite/test_zarray.c): RED before (ASAN heap-buffer-overflow in zarray_get_ex), GREEN after
|
|
- Suites: xcelib_test 700/700, scheduler 90/90, controller 1258/1258, worker 13/13, asan, 0 sanitizer reports
|
|
- Live: n/a -- a bounds check in a library getter and a REPL display path, both covered by the unit tests above
|
|
|
|
## References
|
|
- Fix commit: 16b7aa4b
|
|
- Related: bug-1688 (same count-vs-capacity mistake in info worker), scheduler-sim-diagnostics task 3.1
|