fix(vm): reacquire mutable syscall output pointers after account growth - #1762
Open
0xzrf wants to merge 1 commit into
Open
fix(vm): reacquire mutable syscall output pointers after account growth#17620xzrf wants to merge 1 commit into
0xzrf wants to merge 1 commit into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1741
Problem
sol_get_return_datahas two mutable outputs (r1return data,r3program id). Sig translated the return-data address, kept the resulting host pointer, and then translated the program-id address. Underdirect_mapping+ VASA (SIMD-0460), that second translation can miss and invoke the access-violation handler, which grows the account:AccountSharedData.resizereallocates and frees the old backing, and the handler re-anchorsregion.host_memoryto the new allocation. The subsequent@memcpythen wrote the return data through the stale (freed) pointer, so the live account never received it.The syscall still reported success, so the divergence is silent:
7b 00 00 0042 22 33 44Agave avoids this by touching every mutable output range before retaining any pointer, letting all growth settle, and only then acquiring the pointers (
translate_mut!). The macro's own contract states the invariant Sig violated: "No other translated references can be live when calling this."Scope
this PR covers both:
getReturnData— 2 mutable outputs.getProcessedSiblingInstruction— 4 mutable outputs (header,program_id,data,accounts), all captured back-to-back. Here the stale pointers also corrupted the sixisOverlappingchecks, which compare host addresses and would have diffed against a freed range.findProgramAddressalready implemented this pattern (with a comment naming the hazard), so this brings the remaining sites in line with the existing in-repo solution.Changes
getReturnData— touch both output ranges viavmapbefore translating, then copy through the post-growth pointers.getProcessedSiblingInstruction— three ordered changes:data_len/accounts_lenout ofheaderup front, so the later translations aren't sized by reads through a pointer they can strand (agave's "collect the parameters first" phase).meta_addrincluded — before retaining anything. Zero-length ranges are skipped to matchtranslateSlice, which never translates them, mirroringtouch_slice_mut's early return; without this guard a zero-length output would newly error on an address the old code never resolved.headeramong them, so both the overlap checks and the trailingheader.data_lenwrite target live memory. Agave re-mapsmeta_addrin its secondtranslate_mut!for exactly this reason.vmapis used for the touch phase rather thantranslate*because it performs access checking and growth but skips the alignment check, matching agave'stouch_*helpers. This keeps error ordering as access-violation-before-unaligned; usingtranslateTypeto touch would surfaceUnalignedPointerwhere agave reportsAccessViolation.Tests
Three regression tests, each failing before the fix and passing after:
getReturnData reacquires output after growth(vm/syscalls/lib.zig) — isolation test with a synthetic growth handler.getReturnData reacquires outputs after real account growth(runtime/program/bpf_loader/execute.zig) -- integration test forgetReturnDatausing the existingAccessViolationHandlerCtx.handleto check the errorgetProcessedSiblingInstruction reacquires outputs after growth(vm/syscalls/lib.zig)