chore(buglog): bug-1824 fixed (16b7aa4b)

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Rongsong Shen 2026-09-30 03:35:25 +00:00
parent 16b7aa4b60
commit e9698c3f54
Signed by untrusted user: rshen
GPG key ID: 80533536B0EB651C
2 changed files with 31 additions and 17 deletions

View file

@ -14378,18 +14378,18 @@
},
{
"id": "bug-1824",
"status": "open",
"status_updated": "2026-09-24",
"status_note": "repl_info.c bounds check uses zarray count instead of caps",
"status": "fixed",
"status_updated": "2026-09-30",
"status_note": "",
"component": "scheduler",
"severity": "crash",
"severity": "wrong-result",
"timestamp": "2026-09-17T02:45:00Z",
"last_seen": "2026-09-19T09:00:00Z",
"occurrences": 1,
"error_message": "out-of-scope latent bug discovered while implementing scheduler-sim-diagnostics task 3.1 (new…\n\nNOT 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.",
"file": "src/scheduler/src/repl_info.c",
"file": "src/scheduler/src/repl_info.c (repl_info_resource_one), src/lib/zarray.c (zarray_get_ex)",
"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": "NOT FIXED -- discovered and characterized during scheduler-sim-diagnostics task 3.1 (limit-usage display), out of that task's scope (task 3.1 only touches its OWN new code path, repl_print_limit_usage(), which was written from scratch to avoid this exact mistake: it omits the imprecise .count pre-check entirely, relies on zarray_get_ex()'s own .caps bound for the upper end, and adds an explicit idx<0 guard of its own -- verified correct and reviewed independently, see this session's git history for scheduler-sim-diagnostics task 3.1). repl_info_resource_one() itself was deliberately left untouched to keep task 3.1 in its declared boundary. Recommended fix when picked up: replace the `r->idx >= sim->core.sharedInst.count` half of the pre-check with reliance on zarray_get()'s own .caps-based bound (mirroring task 3.1's new repl_print_limit_usage() pattern), keeping the `r->idx < 0` guard. Likely benign in practice today only because resource registration order in this codebase's current production paths tends to be dense/contiguous from index 0, so .count and .caps rarely diverge for sim->core.sharedInst specifically -- but this is an incidental property of current callers, not a guarantee, and worth a regression test with a deliberately sparse fixture (zarray_put() at a high index with no lower indices ever populated) before relying on it.",
"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.\n\nVerification:\n- 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\n- Suites: xcelib_test 700/700, scheduler 90/90, controller 1258/1258, worker 13/13, asan, 0 sanitizer reports\n- Live: n/a -- a bounds check in a library getter and a REPL display path, both covered by the unit tests above",
"tags": [
"scheduler",
"repl",
@ -14400,7 +14400,9 @@
"discovered-during-scheduler-sim-diagnostics"
],
"related_bugs": [],
"fix_commits": [],
"fix_commits": [
"16b7aa4b"
],
"record": ".wolf/bugs/bug-1824.md"
},
{

View file

@ -1,16 +1,17 @@
---
id: "bug-1824"
status: "open"
status_updated: "2026-09-24"
status_note: "repl_info.c bounds check uses zarray count instead of caps"
status: "fixed"
status_updated: "2026-09-30"
status_note: ""
component: "scheduler"
severity: "crash"
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"]
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
---
@ -20,19 +21,30 @@ legacy: true
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
_Not recorded (migrated from buglog.json)._
- 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
_Not recorded (migrated from buglog.json)._
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
NOT FIXED -- discovered and characterized during scheduler-sim-diagnostics task 3.1 (limit-usage display), out of that task's scope (task 3.1 only touches its OWN new code path, repl_print_limit_usage(), which was written from scratch to avoid this exact mistake: it omits the imprecise .count pre-check entirely, relies on zarray_get_ex()'s own .caps bound for the upper end, and adds an explicit idx<0 guard of its own -- verified correct and reviewed independently, see this session's git history for scheduler-sim-diagnostics task 3.1). repl_info_resource_one() itself was deliberately left untouched to keep task 3.1 in its declared boundary. Recommended fix when picked up: replace the `r->idx >= sim->core.sharedInst.count` half of the pre-check with reliance on zarray_get()'s own .caps-based bound (mirroring task 3.1's new repl_print_limit_usage() pattern), keeping the `r->idx < 0` guard. Likely benign in practice today only because resource registration order in this codebase's current production paths tends to be dense/contiguous from index 0, so .count and .caps rarely diverge for sim->core.sharedInst specifically -- but this is an incidental property of current callers, not a guarantee, and worth a regression test with a deliberately sparse fixture (zarray_put() at a high index with no lower indices ever populated) before relying on it.
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
_Not recorded (migrated from buglog.json)._
- 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
_Not recorded (migrated from buglog.json)._
- Fix commit: 16b7aa4b
- Related: bug-1688 (same count-vs-capacity mistake in info worker), scheduler-sim-diagnostics task 3.1