fix(FIX-LTX2-DEVICE-UAF-904): the staged view outlived the temporary that owned it (#904) - #936
Conversation
…that owned it Closes #904. Makes `sanitize-cpu` green on `main` on both arms. One LTX-2 device case restored its bf16 keyframe view from an unnamed `Ltx2StageDitWeightsToDevice(...)`. A `vt::Tensor` is a BORROWED view, and the storage it points into is owned by the `Ltx2DitDeviceWeights` it came from. The temporary died at the end of that full-expression and freed every staged buffer; the forward two lines later read through the dangling pointer. Binding the restaging to a named local is the whole fix. ## #904'S STATED CAUSE IS WRONG, AND THE CORRECTION MATTERS The issue and the issue-index row both say `~Ltx2DitDeviceWeights` frees the buffers while a threadpool worker is still reading them, so "the staged weights' lifetime is not joined to the in-flight parallel op". That reading sends the next person to redesign where a join belongs — the op dispatch, the queue, or the destructor. None of them is implicated. ASan attributes the allocation AND the free to test line 724, and the read to the call at 726. A teardown-races-an-in-flight-op defect would attribute the free to the closing brace at 729, where the NAMED `staged` dies. The named object is never the freed one. Which thread performs the read is not the signal, and reading it as one is how the issue reached the threadpool. `ParallelForRows` dispatches rows and blocks, so a worker frame under a synchronous `vt::Add` is ordinary. #904 captured the read on worker T2; CI run 31885935312 captured the same defect with the whole chain inline on T0. Same address, same `LoadF32At` frame, same free site — the thread was never the variable. `Ltx2StageDitWeightsToDevice` has 14 other call sites, all in this file, and every one binds its result to a named object. That is why exactly one case aborts. ## EVIDENCE Same configuration as the `sanitize-cpu (address,undefined)` job, plus `setarch -R`, which this host needs or the binary SIGSEGVs with no output. RED at 5a0ffe9 unmodified: rc=1, heap-use-after-free at cpu_layernorm.cpp:33 in LoadF32At, address 0x50a000038c40 — the address CI reports. GREEN with this change: rc=0, 18 passed / 0 failed, 546 assertions passed / 0 failed. A test that stops aborting can stop asserting, so the guarantee itself was mutated: `ltx2_device.cpp:832`, `dtype ==` inverted to `!=` so the refusal fires on the matching bf16 view the positive control restores. compile_rc=0, diff touched exactly 1 line, run_rc=1, the named case failed (2 failed of 18). Tree restored byte-for-byte. No production file is touched, so no shipped behaviour can move. Spec: `.agents/specs/ltx2-device-staged-view-uaf.md`. It records one owed item — nothing in the tree refuses a borrowed `vt::Tensor` that outlives its owner, and it took ASan on a lane cancelled on every `main` run for weeks to find this instance. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5 [claude-code]
…istration (#878) Closes #873. Spec: [`ci-concurrency.md`](.agents/specs/ci-concurrency.md). Row: `GATE-CI-CONCURRENCY`. `main` @ `0141cb4b4` was RED on **seven** preflight gates, every one of them owned by a checker whose only job is to stop a checker silently leaving CI: ``` check-release-binary-contract check-test-registration check-release-workflow test_check_release_binary_contract test_release_manifest test_release_pipeline test_check_test_registration ``` Reproduced independently at a clean detached `origin/main` with zero modifications. #873 listed six; `test_check_test_registration` is the seventh, and `test_release_pipeline` carried six failing cases rather than one. ## What broke, and why the name of the key matters `check-release-binary-contract.py` and `check-test-registration.py` credit a command to CI only through `_unconditional_ci_run_blocks`, which drops every job carrying an `if:` — **any** `if:`: ```python if "if" in job_fields: continue ``` #865 gave `agent-record` one, and the eight invocations that job owns stopped counting as wired. The workflow still ran them; the checkers could no longer prove it, which is the whole thing they exist to prove. That is not a quirk of those two checkers. A job-level `if:` is the same key whether it is guarding a lane or gating a registration, so a checker reading for registration cannot tell the two apart. The mechanism has to separate them. ## The repair **`agent-record` carries no `if:` at all**, restoring the pre-#865 shape. #822's closed-pull-request skip is re-expressed through `needs:`: `last-gated-commit` widens to every lane but the closed action, and a skipped dependency skips its dependents. Same skip, expressed where a checker can still see the gate. `always()` must not come back — it would defeat exactly that — and its absence is now asserted. Dropping `always()` buys an asymmetry that `ci.yml` now records: the implicit `success()` couples `agent-record` to `last-gated-commit` **succeeding**, not merely running, while `documentation-checkpoint` and `commit-protocol-tag` on the same `needs:` keep `always()`. A failure in the sha resolver therefore silences this job's checkers rather than reding them, which is why that job does nothing but resolve a string and falls back instead of failing. `last-gated-commit` resolves a sha only on the push lane; every other lane leaves it empty and its consumers take the fallback they already had. **The two `windows-msvc-*` jobs go back to the byte-exact schema.** `validate_pr_ci` compares the whole job mapping for equality; it is the artifact proving the PR lane holds no release, upload, write-token or OIDC authority (#117). ## Two ratified requirements that cannot both hold The Windows schema admits no extra key, so `needs:` is rejected, and it fixes the `if:` string, so a closed-action clause is rejected. No third mechanism reaches those two jobs *while the schema stands as written* — which is a cost, not an impossibility, and the spec now says so rather than claiming there is no third route and then naming two. *"Every job excludes the closed action"* and *"the Windows PR proof is byte-exact"* are unsatisfiable together **as long as the pinned literal is not advanced**. Precedence went to the authority proof, so those two jobs **do** still start on a closed pull request. Relative to `origin/main` today that is a guard this PR **removes**: two `windows-2022` runners at `timeout-minutes: 180` start on every close, including every merge. That is a decision rather than an implementation detail, so it is **#874**, and the cost has been [recorded there](#874 (comment)) where it will be decided rather than only in this PR. ## The pin that was relaxed instead of satisfied #865 hit `test_the_agent_record_job_is_unconditional`, whose whole subject was that a CI registration behind an `if:` is not a registration, and rewrote it to permit one condition. The three checkers asserting the same property against the workflow stayed red, and `main` shipped red for every branch cut from it. The assertion is restored, with what it protects written into its docstring. `test_a_closed_pull_request_executes_no_gate` now walks the `needs:` graph instead of grepping each job's `if:` — a change of mechanism, not of guarantee. ## Review repairs (F1, F2) Both findings were the same shape as #755 and #866: a claim that nothing asserts. **F1 — the exemption was an unratcheted allowlist.** This body, the commit message and the spec all said `UNGUARDABLE_JOBS` was *"pinned to exactly two names so a third cannot join them quietly"*. Nothing enforced it: adding `"pr-size"` to the tuple and stripping `pr-size`'s own closed clause from `ci.yml` left every gate green. `test_every_unguardable_job_is_one_the_pinned_schema_owns` now asserts the *mechanical fact that justifies the exemption* — every entry must appear in `validate_pr_ci`'s `contracts` tuple, i.e. must be a job whose whole mapping is compared against a literal and which therefore can carry neither key. `contracts` is a local, so the test parses the checker with `ast` rather than importing it; exposing it would mean editing `scripts/`, and the subject of this row is that a test may not reshape the thing it pins in order to pin it. A failed parse yields an empty set, so the non-empty assertion comes first — otherwise the subset check passes vacuously, which is mutation **E3** below. **F2 — the closed-PR test rejected only `always()`.** `always()` is not the one thing that breaks the transitive form. GitHub runs a dependent whose `if` calls *any* status check function when a needed job was skipped, so `!cancelled()`, `failure()` and `success() || failure()` do it too — `documentation-checkpoint` rewritten as `${{ !cancelled() && (…) }}` with no closed clause left every gate green, while the #865 version this replaced (`assertIn("closed", if)`) reds on that same config. A genuine widening, now closed. The repair permits a **shape** rather than forbidding four names: a job leaning on `needs:` may carry no expression call at all. Everything that defeats skip propagation is a call, so this is a superset of `success` / `failure` / `cancelled` / `always` and stays correct if GitHub adds a fifth — a denylist of four names is the same "evaded by a shape nobody listed" problem as F1. It fails **closed**: a harmless `contains(...)` is refused too, and the answer to that is to carry the closed clause directly, which every job but the two pinned Windows proofs can. ## Nothing was weakened `git diff origin/main -- scripts/` is **0 lines** — verified again at this head, after the review repair and after the merge forward. So is the diff of all five suites owned by the four checkers (`test_check_release_binary_contract.py`, `test_release_manifest.py`, `test_release_pipeline.py`, `test_check_test_registration.py`, `test_release_windows_metadata.py`). The only suite touched is `test_main_baseline.py`, which #865 authored. ## Mutation evidence Twelve mutations. Each rebuilds a pristine scratch tree from this head with `git archive`, applies one change, runs the nine gates, then restores and proves the tree byte-identical with `sha256sum -c`. Failures are classified from the unittest **summary line** (`FAILED (failures=N)` vs `errors=N`), not from the presence of "Traceback" — a unittest *failure* prints one too, and a Python checker that fails to parse also exits non-zero and would read as a pass. Every substitution asserts its anchor occurs **exactly once**. That is not decoration: the first attempt at M7 used `needs: [last-gated-commit]` + `runs-on: ubuntu-latest`, which matches **twice**, and produced `duplicate YAML key 'if'` — a parse failure that exits non-zero and reads as a passing test. The assertion caught it and the sweep printed `MUTATION FAILED TO APPLY — not a test result` instead of a gate verdict. Denominator for every row: 3 checkers (`check-release-binary-contract`, `check-release-workflow`, `check-test-registration`) and 6 suites (`test_check_release_binary_contract`, `test_release_manifest`, `test_release_pipeline`, `test_check_test_registration`, `test_main_baseline`, `test_release_windows_metadata`). All 9 are green unmutated. | # | Mutation | Red gates (of 9) | Kind | Message | |---|---|---|---|---| | E1 | add `"pr-size"` to `UNGUARDABLE_JOBS`, strip its closed clause | 1 — `test_main_baseline` | assertion, `failures=1` | `{'windows-msvc-cpu', 'windows-msvc-vulkan', 'pr-size'} not less than or equal to …` | | E2 | `documentation-checkpoint` → `${{ !cancelled() && (…) }}`, no closed clause | 1 — `test_main_baseline` | assertion, `failures=1` | `documentation-checkpoint runs on a closed pull request: neither its own if: nor any job it needs…` | | E3 | rename `contracts` so the `ast` parse finds nothing | 1 — `test_main_baseline` | assertion, `failures=1` | `set() is not true : no contracts tuple found in check-release-workflow.py::validate_pr_ci` | | M1 | drop `scripts/check-release-binary-contract.py` | 3 — contract checker, `test_check_release_binary_contract`, `test_release_manifest` | assertion | release checker is missing from the explicit CI checker step | | M2 | drop `tests/scripts/test_check_release_binary_contract.py` | 3 — same three | assertion | release mutation suite is missing from the explicit CI step | | M3 | drop `tests/scripts/test_release_manifest.py` | 3 — same three | assertion | W5 manifest suite is missing from an unconditional CI step | | M4 | drop `tests/scripts/test_release_windows_metadata.py` | 3 — same three | assertion | W15 Windows metadata suite is missing from an unconditional CI step | | M5 | drop `scripts/check-test-registration.py` | 2 — registration checker, `test_check_test_registration` | assertion | check-test-registration is missing from the explicit CI checker step | | M6 | drop `tests/scripts/test_check_test_registration.py` | 2 — same two | assertion | test_check_test_registration is missing from the CI mutation suite | | M7 | re-add the #865 `if:` to `agent-record` **alone** | 6 — contract checker, registration checker, `test_check_release_binary_contract`, `test_release_manifest`, `test_check_test_registration`, `test_main_baseline` | assertion | CI release step must contain checker and suite as direct active commands | | M8 | re-add the #865 closed clause to the two Windows proofs | 3 — `check-release-workflow`, `test_release_pipeline` (`failures=6`), `test_main_baseline` | assertion | must exactly match the read-only native Windows PR proof schema | | M9 | drop the closed guard from `last-gated-commit` | 1 — `test_main_baseline` | assertion | agent-record runs on a closed pull request | **M7's counts were wrong in the previous revision of this body** ("3 checkers + 3 suites"). Applied precisely to `agent-record` and nothing else, it reds **two** checkers and **four** suites — `check-release-workflow` and `test_release_pipeline` stay green, because the Windows schema is untouched by it. The earlier figure came from an anchor that also hit a second job. M9 is the anti-overcorrection half: it proves the closed-PR skip is really carried by the `needs:` edge and not merely asserted. E3 is the instrument's own failure mode: a broken parse must red, not pass vacuously. ## Gates Every baseline number below was taken in a **clean detached worktree at `origin/main`**, never in the shared checkout — which currently carries 15 staged modifications from another session and reports `main` regardless, so a gate run there measures a tree nobody intended. **Baseline, clean detached `e8048ef63`** — the seven are still red on `main`, which is what this PR closes: | Gate | `origin/main` `e8048ef63` | this head `c03b734f0` | |---|---|---| | `check-release-binary-contract` | RED — release mutation suite is missing from the explicit CI step (+3 more) | ok | | `check-release-workflow` | RED — `windows-msvc-cpu`/`vulkan` must exactly match the read-only native Windows PR proof schema | ok | | `check-test-registration` | RED — check-test-registration is missing from the explicit CI checker step (+2 more) | ok | | `test_check_release_binary_contract` | RED — `failures=2` | ok, 30 cases | | `test_release_manifest` | RED — `failures=1` | ok, 22 cases | | `test_release_pipeline` | RED — `failures=6` | ok, 42 cases | | `test_check_test_registration` | RED — `failures=1` | ok, 52 cases | | `test_main_baseline` | ok, **55** cases (the #865-relaxed pin) | ok, **56** cases (+1 for F1) | | `test_release_windows_metadata` | ok, 7 cases | ok, 7 cases | **Full gate at `c03b734f0`, after merging `origin/main` at the pinned `e8048ef63`:** `scripts/agent-preflight.sh --quiet` → **`All gates green.`**, 76 of 76, exit 0, zero `FAIL` lines. `test_cpu_x86_llamacpp_floor` is green in this run too (box load 5.99); the earlier `NO_QUIET_WINDOW … load=75.05` red on the previous head was #618's load-dependent case, and it was already green 3/3 on that same tree at load 30.5 / 22.7 / 19.5. `main` then advanced once more, so the branch was merged forward again at the pinned `04be1390b` — head **`adef0b26a`**. The three gated files (`ci-concurrency.md`, `ci.yml`, `test_main_baseline.py`) are **byte-identical** across `53c6a9acc` → `c03b734f0` → `adef0b26a`; the only file that moved is `.agents/issue-index.md`, gaining `main`'s own rows. Preflight at `adef0b26a` is 75 of 76 with `test_cpu_x86_llamacpp_floor` the sole red at `NO_QUIET_WINDOW … load=57.54` — #618 again — and green on that same tree once the box quieted to load 18.14. Both merges hand-resolved `.agents/issue-index.md`: `git merge` auto-merged it cleanly both times and both results were **discarded**, because the first one inserted this row's entries in the middle of the table rather than at the tail. Each time the target's version was taken wholesale and the two rows reappended, then verified — rows byte-identical to the ones `578ab36eb` authored, every unrelated key byte-for-byte identical to the pin, the pin's file a strict prefix of the result, no duplicate key. E1 and E2 were re-run after the merge and both still RED at `failures=1` with the suite's own assertion, restored byte-identical. The three gated files are byte-for-byte identical to the coordinator-verified head `53c6a9acc`, so nothing in the merge disturbed them. ## The previous head's CI red was not #873, and its absence is the proof `agent-record` failed on the previous head with ``` ERROR: docs/WEIGHT-OFFLOAD.md: published but absent from website/data/nav.yaml, so it has no sidebar entry and is unreachable on the site ``` The **five #873 errors are absent from that job's output** — which is the CI-side confirmation that this repair works: the job ran, its checkers were credited, and it got far enough to fail on something else entirely. That something else was a defect that landed on `main` at `62406c30e` via #885 and that `main` fixed in #895, which is inside the range merged here. A stale-merge-ref artifact, cleared by the re-merge. Not filed. Baseline reds to subtract, not attribute to this branch: `windows-msvc-cpu` and `windows-msvc-vulkan` fail on every PR (#584) — read the failure text before assuming it is the schema, since this PR restores those two jobs' schema. Disk 87–91% throughout (42–60 GiB free), so no ENOSPC window and no false policy refusals. ## Landed after a fourth re-merge, and why the forge called it a conflict GitHub reported this branch `CONFLICTING` and refused to merge it. Nothing about the branch changed — `main` moved. `.gitattributes:7` gives `.agents/issue-index.md` `merge=union`, and GitHub's server-side merge does not apply a `.gitattributes` merge driver, so the one file that merges cleanly in every local checkout is exactly the file the forge cannot merge. `git merge-tree origin/main <branch>` returns rc=0 and writes a tree; with the union driver disabled it returns the same file in all three stages. `7061e8129` merges `origin/main` at `f270b4b0a` and resolves the index by hand, as the three merges before it did. The union driver is clean and wrong the same way each time: it interleaves this row's #873/#874 rows among the rows `main` appended, so `origin/main`'s file stops being a PREFIX of the result. Measured on the automatic result before discarding it: 226 unique rows, no duplicates, and PREFIX: NO. Resolved by taking `origin/main` wholesale and re-appending — PREFIX: YES, both rows byte-identical to the branch head, 226 of 226 keys unique. The gated files (`ci.yml`, `test_main_baseline.py`, and the three checkers) are byte-identical to the reviewed head `887660b17`, so the fresh review's PASS still covers this content; the three commits `main` gained in the range (`f270b4b0a`, `abe01ddd5`, `c1d02bfbe`) touch no path under `.github/`, `scripts/`, or `tests/scripts/`. All ten record gates re-run GREEN locally on that exact merged tree: the three checkers, `check-agent-record`, `test_check_release_binary_contract`, `test_release_manifest`, `test_release_pipeline`, `test_check_test_registration`, `test_check_issue_index_append_only`, and `test_main_baseline`. Reds subtracted, each verified rather than assumed. `windows-msvc-cpu` and `windows-msvc-vulkan` fail identically on the unrelated PR #929 — `server_main.cpp(1315,55): error C2220`, a `C4456` shadowing warning promoted to an error — so it is a pre-existing MSVC break, not this PR's restored schema, and both jobs are `skipped` on the `main` push lane. `sanitize-cpu` is red for #904, which PR #936 fixes, and is `continue-on-error` on this lane. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5 [claude-code]
Picks up the #873 checker repair that landed in #878, so this branch's `agent-record` job reports on its own content rather than on main's. Clean merge with no hand resolution. This row touches one test file and adds one spec, and it never edits `.agents/issue-index.md`, so the append-only record has no conflict to resolve here: origin/main's index is a strict PREFIX of the merged result, unchanged. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5 [claude-code]
…n of it that had to be thrown away Records the evidence §5 was relying on and had not yet produced. The full 484-test `build-sanitize` suite is green apart from the two standing skips. `test_ltx2_device` reported `***Failed` in that run, and the failure was this session's own instrument rather than a defect: the mutant from the §4 mutation was still linked in. Restoring the mutated source with `cp`/`mv` gave it an mtime OLDER than the object built from the mutant, so ninja judged it up to date, exited 0 having compiled nothing, and left the mutation in libvllm.a. `git diff` on the source was empty throughout; only `stat` disagreed. A build that compiles nothing and exits 0 defeats every ordinary discriminator for a stale binary -- no non-zero exit, no diagnostic, no dirty tree -- which is why it is written down rather than quietly fixed. The tell was the numbers: the failure reproduced the mutation's `16 passed | 2 failed` and 528 assertions to the digit, and two unrelated defects do not agree that closely. Corrected by touching the source, rebuilding 399 translation units including `ltx2_device.cpp.o`, and rerunning the affected family: 10 of 10 passed. `ltx2_device.cpp` was the only mutated unit and `ltx2_video.cpp` the only other one that reaches it; both are inside those 10, so the other 479 results stand. No code changes. The spec is the deliverable. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5 [claude-code]
|
Operator attribution, since it was not in the issue and it changes who owns this. The defect was introduced by #880, which I gated and merged this morning.
How it got past me, stated plainly because the pattern is more useful than the instance:
Two corrections to how I have described this elsewhere, before they propagate:
A companion search across FOLLOWING_AGENTS_PROTOCOL |
…r-arm run that already proved "both arms" Three record repairs from the fresh review. No code changes; the fix itself is unchanged and stays proven. The call-site count was wrong, and under `squash_merge_commit_message = PR_BODY` a wrong number in the body becomes the landed commit message, where it cannot be corrected. `Ltx2StageDitWeightsToDevice` has 12 other call sites, not 14. The 14 is a grep hit count: `tests/vllm/models/test_ltx2_device.cpp` holds 14 occurrences of the identifier, one of which is the `using` declaration at `:64` and so is not a call, and one of the 13 remaining call expressions is the offending site itself. Recorded with the method rather than only the number, so that separating occurrences from call syntax is repeatable, and with a control that moves: appending one call and one comment mention to a scratch copy takes the pair 14/13 to 16/14, and a misspelled needle returns 0 instead of a plausible number. Counted at `5a0ffe9e3` and again at this head. The "green on both arms" claim outran its evidence, and the evidence already existed inside the run the spec cites. At the main-lane SHA `04be1390b`, run 31885935312 failed on BOTH arms with exactly one test of 478 and the same one: `71 - test_ltx2_device`, ASan reporting heap-use-after-free at `cpu_layernorm.cpp:33 in LoadF32At` and TSan reporting the same defect at the same frame. §8 now carries that per-arm result, so "both arms" is measured on each arm rather than generalised from one. §5's full-suite promise was already discharged by `800dd082f` and needed no further wording. The owed guard now has an issue. `## Owed` references #949 and the issue index carries a row for it, so it is no longer discoverable only by reading this spec. The index row SHA-anchors its two `path:NN` citations as `@ 800dd08`, which is the remedy #911 names, because an anchor in an append-only row cannot be corrected after it lands. While in there: #904's stated cause is wrong and the spec now says why at anchors rather than by assertion. There was no race. `ParallelForRows` (`cpu_threadpool.cpp:413`) dispatches through `Threadpool::Run` (`:354`), which runs the body on the caller as worker 0 (`:383`); every `ComputeThread` (`:220`) ends in `Barrier()` (`:234`) whose exit is a full seq-cst fence (`:208-212`), so `Run` returns only once every worker has passed it. The thread id in the report was always incidental. The decisive evidence is that the thread sanitizer was asked and answered no: the `sanitize-cpu (thread)` arm of the same run reports heap-use-after-free and 2 warnings, zero of them a data race, against a control of 5 `ThreadSanitizer` matches in that log so the absence is not a wrong search term. Every `file:line` in the edited text was re-derived at this tree from the claim rather than by reading the span out of the cited file, including the bare `:NN` continuations: 14 claims examined, 14 unique and at the claimed line, 0 failed, plus a negative control that returns 0 hits. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5 [claude-code]
…EN-APPEND `origin/main` moved again while this row's gate was running, from `c2019b0e3` to `4880c5715`. Clean auto-merge, no conflicts, three files: the append-only issue index, the new `ltx2-device-staged-view-uaf.md` spec, and `test_ltx2_device.cpp`. Taken rather than deferred for two reasons, neither of which is that the row would otherwise revert #936. It would not: a squash merge resolves against the merge base, so content that exists only on the base survives, and the `.agents/specs/ltx2-device-staged-view-uaf.md | 290 ----` that `git diff origin/main HEAD` printed before this merge is the ordinary shape of an unmerged branch rather than a pending deletion. That distinction is worth stating because the first draft of this message got it wrong. The reasons that do hold. A gate is evidence about a tree, and the tree a reviewer cares about is the one that will exist after landing, not the one this branch had before #936 touched a file it shares. `test_ltx2_device.cpp` is exactly such a file. And a clean `git merge-tree` says the merge has no textual conflict; it does not say the merged tree compiles. Merging here and running the full gate on the result is what turns the first claim into the second. Checked after the merge rather than assumed: the issue index carries this row exactly once and #936's row exactly once at 235 rows total, with this row's `schedulers.py:32 @ fd4ded7fa` anchor intact — union-append is only correct because that file is genuinely append-only. The READER ANCHORS line still derives to `756 811 907 923 925 1003 1028 1133 1174`, since nothing in this merge touches `ltx2_video.cpp`, and `check-public-doc-tables.py` is green. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5 [claude-code]
fix(FIX-LTX2-DEVICE-UAF-904): the staged view outlived the temporary that owned it
Closes #904. Makes
sanitize-cpugreen onmainon both arms — see EVIDENCEfor the per-arm result that claim rests on.
One LTX-2 device case restored its bf16 keyframe view from an unnamed
Ltx2StageDitWeightsToDevice(...). Avt::Tensoris a BORROWED view, andthe storage it points into is owned by the
Ltx2DitDeviceWeightsit camefrom. The temporary died at the end of that full-expression and freed every
staged buffer; the forward two lines later read through the dangling
pointer. Binding the restaging to a named local is the whole fix.
#904'S STATED CAUSE IS WRONG, AND THE CORRECTION MATTERS
The issue and the issue-index row both say
~Ltx2DitDeviceWeightsfrees thebuffers while a threadpool worker is still reading them, so "the staged
weights' lifetime is not joined to the in-flight parallel op". That reading
sends the next person to redesign where a join belongs — the op dispatch,
the queue, or the destructor. None of them is implicated.
ASan attributes the allocation AND the free to test line 724 at
5a0ffe9e3,and the read to the call at 726. A teardown-races-an-in-flight-op defect
would attribute the free to the closing brace at 729, where the NAMED
stageddies. The named object is never the freed one.THERE WAS NEVER A RACE, and
ParallelForRowscannot produce one. Insrc/vt/cpu/cpu_threadpool.cpp:ParallelForRows(:413) hands the body totp.Run(...)(:427);Threadpool::Run(:354) runs it on the CALLER asworker 0 via
ComputeThread(workers_[0])(:383); every participatingthread's
ComputeThread(:220) ends inBarrier()(:234), whose exit isa full seq-cst fence (
:208-212).Runreturns only after every worker haspassed that barrier — its own comment at
:352-353calls the return "thecompletion point for every output element". Nothing dispatched can still be
reading once
Runhas returned. The join #904 asks for is already there.The thread id is incidental. #904 recorded the read on worker T1/T2; CI run
31885935312 recorded the same defect with the whole chain inline on the main
thread. Same address, same
LoadF32Atframe, same free site.The decisive check is that the THREAD sanitizer was asked and answered no.
The
sanitize-cpu (thread)arm of that run reportsThreadSanitizer: heap-use-after-free, the readby main thread, andreported 2 warnings— ZERO of them a data race. Control, so the absence isnot just a wrong search term: the same log yields 5 matches for
ThreadSanitizer.Ltx2StageDitWeightsToDevicehas 12 other call sites, all in this file, andevery one binds its result to a named object. That is why exactly one case
aborts. Not 14, which is a grep hit count: the file holds 14 occurrences of
the identifier, one of which is the
usingdeclaration attests/vllm/models/test_ltx2_device.cpp:64and so is not a call, and one ofthe 13 remaining call expressions is the offending site itself. Counted by
separating occurrences from call syntax at
5a0ffe9e3and again at thishead, with a control that moves: appending one call and one comment mention
to a scratch copy takes the pair 14/13 to 16/14, and a misspelled needle
returns 0 rather than a plausible number.
EVIDENCE
mainis red on BOTH arms for this one case, measured per arm rather thangeneralised from one. Scheduled run 31885935312 at the main-lane SHA
04be1390b:sanitize-cpu (address,undefined): 1 test failed out of 478,
71 - test_ltx2_device, AddressSanitizer heap-use-after-free atcpu_layernorm.cpp:33 in LoadF32At, address 0x50a000038c40.
sanitize-cpu (thread): 1 test failed out of 478,
71 - test_ltx2_device, ThreadSanitizer heap-use-after-free at thesame cpu_layernorm.cpp:33 in LoadF32At.
Exactly one test fails on each arm and it is the same test on both, so
removing it is what "green on both arms" rests on.
Local reproduction used the same configuration as the
sanitize-cpu (address,undefined)job, plussetarch -R, which this hostneeds or the binary SIGSEGVs with no output.
RED at 5a0ffe9 unmodified: rc=1, heap-use-after-free at
cpu_layernorm.cpp:33 in LoadF32At, address 0x50a000038c40 — the
address CI reports.
GREEN with this change: rc=0, 18 passed / 0 failed, 546 assertions
passed / 0 failed.
A test that stops aborting can stop asserting, so the guarantee itself was
mutated:
ltx2_device.cpp:832,dtype ==inverted to!=so the refusalfires on the matching bf16 view the positive control restores. compile_rc=0,
diff touched exactly 1 line, run_rc=1, the named case failed (2 failed of
18). Tree restored byte-for-byte.
No production file is touched, so no shipped behaviour can move.
Spec:
.agents/specs/ltx2-device-staged-view-uaf.md. Its one owed item isnow filed as #949 and carries a row in
.agents/issue-index.md: nothing inthe tree refuses a borrowed
vt::Tensorthat outlives its owner, and theonly instrument that catches one is a
continue-on-errorlane. The reviewmeasured that gap rather than asserting it — with this fix reverted, a plain
Release build with no sanitizer runs the case 18/18, 546 assertions, rc=0.
THE FULL SWEEP, AND THE RUN OF IT I THREW AWAY
The full 484-test
build-sanitizesuite is green apart from the two standingskips (
test_modelopt_mixed_precision_checkpoint,test_voxtral_e2e).test_ltx2_devicereported***Failedin that run, and it was MY INSTRUMENT,not a defect. The mutant from the mutation above was still linked in. I restored
the mutated source with
cp/mv, which gave it an mtime OLDER than the objectbuilt from the mutant; ninja judged it up to date, exited 0 having compiled
nothing, and left the mutation in
libvllm.a.git diffon the source wasempty the whole time. Only
statdisagreed — the object was 24s newer than thesource it came from.
A build that compiles nothing and exits 0 defeats every ordinary discriminator
for a stale binary: no non-zero exit, no diagnostic, no dirty tree. The tell was
the numbers — the failure reproduced the mutation's
16 passed | 2 failedand528 assertions to the digit, and two unrelated defects do not agree that
closely.
Corrected by touching the source, rebuilding 399 translation units including
ltx2_device.cpp.o, and rerunning the affected family:ltx2_device.cppwas the only mutated unit, andsrc/vllm/multimodal/ ltx2_video.cppis the only other translation unit that reaches it. Both areinside those 10, so the other 479 results stand.
Baseline reds to subtract:
windows-msvc-cpuandwindows-msvc-vulkanfailidentically on the unrelated PR #929 —
server_main.cpp(1315,55): error C2220,a
C4456shadowing warning promoted to an error. Pre-existing MSVC break, andboth jobs are
skippedon themainpush lane.FOLLOWING_AGENTS_PROTOCOL
Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: AGENT:claude-opus-5 [claude-code]