From 23c3fd6b6a7e7efa2605ca28a65f379cb04259c8 Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Tue, 18 Aug 2026 07:54:29 +0000 Subject: [PATCH] chore(wolf): checkpoint buglog.json (959 bugs) --- .wolf/buglog.json | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/.wolf/buglog.json b/.wolf/buglog.json index cb236d15..44807636 100644 --- a/.wolf/buglog.json +++ b/.wolf/buglog.json @@ -17181,7 +17181,7 @@ "error_message": "Controller (xce_controller) reproducibly exits via SIGSEGV (si_addr=NULL, si_code=1/SEGV_MAPERR) within ~1s of the last resupdate follow-up verification reply completing, when the synthetic worker then disconnects ('quit'). ROOT-CAUSED via live repro (2/2) with an LD_PRELOAD signal-handler shim (gdb reliably masked the crash for both ASAN and debug/-O0 builds -- ptrace overhead perturbs timing enough to avoid the window; ~20s under gdb = no crash, every time). Resolved backtrace (addr2line against the debug-symbol'd ASAN binary): encode_NameValue <- encode_NameValue_list <- encode_Worker <- capnp_encode_worker (persist.c) <- persist_worker_hook_fun <- xce_run_hooks <- mark_worker_unavail <- handle_worker_leave <- handle_endpoint_error (cluster.c) <- io_worker_dispatch_batch <- io_worker (netio.c).", "file": "src/controller/sched.c (sched_worker_change(), ~line 548-553) -- crash SITE is persist.c/controller.capnp.c (capnp_encode_worker/encode_Worker/encode_NameValue_list/encode_NameValue) but the DEFECT is in sched.c", "root_cause": "ROOT-CAUSED, PRE-EXISTING (predates resource-value-verification by ~5 months, NOT introduced by tasks 2.1/2.2/6.1). sched_worker_change() (src/controller/sched.c) aliases w->host.resvals_to_sched/nresvals_to_sched directly to a local xlist_t resvalist's backing array (resvalist.data), pushes it to the scheduler connection, then calls xlist_fini(&resvalist) -- which frees that backing array -- WITHOUT first resetting w->host.resvals_to_sched to NULL / nresvals_to_sched to 0. The correct reset-before-free code exists in the function but is wrapped in a dead `#if 0 /*FIXME*/ ... #endif` block, introduced already-disabled by commit acfbec875 (2026-03-03, 'save works'). This leaves w->host.resvals_to_sched a DANGLING pointer and nresvals_to_sched a stale nonzero count on the worker_t struct for the rest of that worker's lifetime. commit e5fde7b5 (2026-05-20, 'fix(persist): zero nresvals_to_sched after free_name_val_list') attempted to fix exactly this class of bug but edited code INSIDE the already-dead #if 0 block, so the fix silently never took effect. When the worker later disconnects, handle_worker_leave() -> mark_worker_unavail() fires the WORKER_DEAD hook -> persist_worker_hook_fun() -> capnp_encode_worker() -> encode_Worker() -> encode_NameValue_list(), which iterates the stale count over the freed array and dereferences a garbage/NULL entry in encode_NameValue() -> SIGSEGV. Only manifests once a scheduler has connected at least once: on_scheduler_join() (sched.c) registers sched_worker_hook_fun into c->hooksys (gated by a `static int hook_installed` one-shot flag) ONLY on the first scheduler join -- a controller with no scheduler ever connecting never reaches the buggy sched_worker_change(), so w->host.resvals_to_sched stays NULL/0 forever and persist_worker_hook_fun's later read is always safe. This exactly matches run-resource-verify.sh's controller+scheduler+apiserver deployment shape and explains the original isolation matrix precisely. The sibling function apiserver_worker_change() (src/controller/apiserver.c ~line 479-481) does the reset correctly (NOT wrapped in #if 0), which is why apiserver-only interaction never crashed.", - "fix": "FIX IDENTIFIED, NOT YET APPLIED (out of task 6.1's boundary -- lives entirely in sched.c, a file no task in this feature touches). In src/controller/sched.c's sched_worker_change(), replace the dead block:\n #if 0\n /*FIXME*/\n free_name_val_list(&(w->host.resvals_to_sched), &(w->host.nresvals_to_sched));\n #endif\n xlist_fini(&resvalist);\nwith the exact working pattern already used by apiserver_worker_change():\n w->host.nresvals_to_sched = 0;\n w->host.resvals_to_sched = NULL;\n xlist_fini(&resvalist);\nIMPORTANT: do NOT simply uncomment the existing free_name_val_list() call as-is -- that would introduce a DOUBLE FREE instead. free_name_val_list() (src/lib/utils.c:412) already deep-frees every entry AND the outer array (FREEUP(resvals) where resvals == w->host.resvals_to_sched == the identical pointer value as resvalist.data), then sets w->host.resvals_to_sched itself to NULL -- but resvalist.data (the xlist_t's own separate field, still holding that same now-freed address) is untouched by that call, so the very next line's xlist_fini(&resvalist) -> FREEUP(resvalist.data) would free the identical address a second time. This is almost certainly why the original author disabled it with #if 0 rather than deleting it. Secondary/lower-severity, NOT part of this fix: both sched_worker_change() and apiserver_worker_change() leak the individual xce_name_val_t entries collected into resvalist -- xlist_fini() only frees the outer array (FREEUP(l->data)), never each entry's .name/.val/nv allocations -- worth a separate follow-up. Also worth a wider audit: other readers/freers of w->resvals_to_sched (opget.c:128 do_reply_workerlist(), apiserver.c, dataex.c) may have the same latent dangling-pointer exposure, only the WORKER_DEAD->persist path was confirmed live in this repro. FIX APPLIED 2026-08-18: src/controller/sched.c sched_worker_change() -- replaced the dead `#if 0 /*FIXME*/ free_name_val_list(...) #endif` block with `w->host.nresvals_to_sched = 0; w->host.resvals_to_sched = NULL;` immediately before `xlist_fini(&resvalist);` -- matching apiserver_worker_change()'s proven pattern exactly (src/controller/apiserver.c:480-482). Verified: (1) xce-build.sh controller -- clean build, 1102/1104 controller tests pass (the 2 failures are pre-existing bug-1561 overlayfs/fdatasync sandbox artifacts, unrelated); (2) live repro via run-resource-verify.sh --keep-running -- deployed controller+scheduler+apiserver, joined synthetic worker 'resverify-worker', ran resupdate for 9 resource values (all accepted), worker disconnected ('quit') triggering handle_worker_leave -> sched_worker_change (pushing 17 resvals to scheduler, twice, situation=HOOK_AFTER_EVENT) -- the exact previously-fatal sequence. Controller stayed alive and logging normally for ~2 minutes past the disconnect (previously died within 1-10s); no SEGV, no ASAN report, no core file. No new unit test added -- sched_worker_change() is a static function with no existing test coverage and no testable-static-function harness pattern in this codebase (confirmed via codegraph), and the crash path requires a live scheduler connection + hooksys + persist/capnp encoding chain across sched.c/cluster.c/persist.c -- disproportionate scaffolding for a 2-line fix; live-repro evidence used instead, per this codebase's established xce-test-ha-class convention.", + "fix": "FIX IDENTIFIED, NOT YET APPLIED (out of task 6.1's boundary -- lives entirely in sched.c, a file no task in this feature touches). In src/controller/sched.c's sched_worker_change(), replace the dead block:\n #if 0\n /*FIXME*/\n free_name_val_list(&(w->host.resvals_to_sched), &(w->host.nresvals_to_sched));\n #endif\n xlist_fini(&resvalist);\nwith the exact working pattern already used by apiserver_worker_change():\n w->host.nresvals_to_sched = 0;\n w->host.resvals_to_sched = NULL;\n xlist_fini(&resvalist);\nIMPORTANT: do NOT simply uncomment the existing free_name_val_list() call as-is -- that would introduce a DOUBLE FREE instead. free_name_val_list() (src/lib/utils.c:412) already deep-frees every entry AND the outer array (FREEUP(resvals) where resvals == w->host.resvals_to_sched == the identical pointer value as resvalist.data), then sets w->host.resvals_to_sched itself to NULL -- but resvalist.data (the xlist_t's own separate field, still holding that same now-freed address) is untouched by that call, so the very next line's xlist_fini(&resvalist) -> FREEUP(resvalist.data) would free the identical address a second time. This is almost certainly why the original author disabled it with #if 0 rather than deleting it. Secondary/lower-severity, NOT part of this fix: both sched_worker_change() and apiserver_worker_change() leak the individual xce_name_val_t entries collected into resvalist -- xlist_fini() only frees the outer array (FREEUP(l->data)), never each entry's .name/.val/nv allocations -- worth a separate follow-up. Also worth a wider audit: other readers/freers of w->resvals_to_sched (opget.c:128 do_reply_workerlist(), apiserver.c, dataex.c) may have the same latent dangling-pointer exposure, only the WORKER_DEAD->persist path was confirmed live in this repro. FIX APPLIED 2026-08-18: src/controller/sched.c sched_worker_change() -- replaced the dead `#if 0 /*FIXME*/ free_name_val_list(...) #endif` block with `w->host.nresvals_to_sched = 0; w->host.resvals_to_sched = NULL;` immediately before `xlist_fini(&resvalist);` -- matching apiserver_worker_change()'s proven pattern exactly (src/controller/apiserver.c:480-482). Verified: (1) xce-build.sh controller -- clean build, 1102/1104 controller tests pass (the 2 failures are pre-existing bug-1561 overlayfs/fdatasync sandbox artifacts, unrelated); (2) live repro via run-resource-verify.sh --keep-running -- deployed controller+scheduler+apiserver, joined synthetic worker 'resverify-worker', ran resupdate for 9 resource values (all accepted), worker disconnected ('quit') triggering handle_worker_leave -> sched_worker_change (pushing 17 resvals to scheduler, twice, situation=HOOK_AFTER_EVENT) -- the exact previously-fatal sequence. Controller stayed alive and logging normally for ~2 minutes past the disconnect (previously died within 1-10s); no SEGV, no ASAN report, no core file. No new unit test added -- sched_worker_change() is a static function with no existing test coverage and no testable-static-function harness pattern in this codebase (confirmed via codegraph), and the crash path requires a live scheduler connection + hooksys + persist/capnp encoding chain across sched.c/cluster.c/persist.c -- disproportionate scaffolding for a 2-line fix; live-repro evidence used instead, per this codebase's established xce-test-ha-class convention. UNIT TEST ADDED 2026-08-18 (follow-up task, closes the gap this entry originally flagged as missing): src/controller/testsuite/test_sched_worker_change.c (new standalone binary, sched_worker_change_unit_test, wired into src/controller/meson.build mirroring sched_push_unit_test's real-sched.c+dataex.c+egress.c linking strategy). Test 1 fires XCE_CONTROLLER_HOOK_WORKER/HOOK_DETAIL_WORKER_DEAD via xce_run_hooks() (the same production entry point handle_worker_leave()->mark_worker_unavail() uses) on a worker_t with a populated w->host.resvals, then asserts w->host.resvals_to_sched==NULL && nresvals_to_sched==0. Test 2 goes further: after the same hook fire, it calls the REAL production encoder encode_Worker() (controller.capnp.c) directly on the worker -- the exact function bug-1578's crash backtrace bottoms out in via capnp_encode_worker/persist_worker_hook_fun -- so a regression is caught as an ASAN heap-use-after-free, not just a failed assertion. RED/GREEN verified live: temporarily re-wrapping the fix in the original dead `#if 0 ... #endif` block reproduced a genuine ASAN heap-use-after-free (encode_NameValue_list, controller.capnp.c:3408, freed by xlist_fini<-sched_worker_change<-sched_worker_dead<-sched_worker_hook_fun) -- confirming the earlier claim in this same entry ('no testable-static-function harness pattern exists in this codebase for it') was wrong; xce_run_hooks() through the real installed hooksys is exactly that harness, already proven by sched_push_unit_test for the JOB hook and now for the WORKER hook too. Restored the approved fix afterward (verified via git diff byte-identical to the pre-revert state) and confirmed GREEN again.", "tags": [ "CRITICAL", "capnp-encode", @@ -17206,15 +17206,16 @@ "task-6.1", "task-6.2-blocker", "use-after-free", - "fixed" + "fixed", + "unit-test-added" ], "related_bugs": [ "bug-1572", "bug-1574", "bug-1577" ], - "occurrences": 3, - "last_seen": "2026-08-18T06:55:00Z" + "occurrences": 4, + "last_seen": "2026-08-18T08:00:00Z" } ] }