fix(reactor): defer IO request free while its libuv op is still in flight (#173)#175
Merged
Merged
Conversation
…ight (#173) An awaiter that abandons its request — stream_set_timeout()+fgets() where the timer wins, or a cancelled coroutine parked on a write/flush/stat/sendfile — called req->dispose(req) while libuv still referenced the request, leaving several distinct use-after-frees (seen as an intermittent access violation on Windows process teardown in io/042): - Armed stream read (the io/042 crash): dispose freed the request but left io->active_req pointing at it with uv_read_start still armed; the later uv_close callback ran dispose() through freed memory. Dispose now stops the reader and detaches io->active_req; the same detach was added to udp_req_dispose for an abandoned recvfrom. - In-flight uv_write / uv_fs_read / uv_fs_write / uv_fs_fsync / uv_fs_fstat / sendfile: these cannot be cancelled synchronously, so dispose now defers the free with a UV_IN_FLIGHT/DISPOSE_PENDING rendezvous (mirroring the existing UDP-sendto one) — the completion callback finishes the deferred dispose and skips the NOTIFY; a queued-but-not-started threadpool op is uv_cancel'ed. io_file_stat_cb additionally skips writing the stat result into the vanished awaiter buffer. - FILE io freed under a threadpool op: FILE-type handles have no uv_close rendezvous, so an owner dispose could free the async_io_t while a completion callback still reached through it. Every fs submission (and fs_open pending io, and both sendfile ios) now holds an event reference for the op duration. - Latent: disposing a not-yet-closed stream/UDP io freed it after scheduling uv_close, leaving the pending close callback with a dangling handle; final teardown is now transferred to io_close_cb.
EdmondDantes
force-pushed
the
fix-173-io-req-dispose-uaf
branch
from
32994bc to
972b057
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
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
1 participant
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 #173.
Root cause
A whole class of use-after-free where an IO request is disposed while libuv still holds a pointer to it — an awaiter abandons the request (
stream_set_timeout()+fgets()where the timer wins, or a cancelled coroutine parked on write/flush/stat/sendfile) and callsreq->dispose(req), but the libuv operation is still armed/in-flight.The
io/042crash is one instance: an armed stream read whose request was freed but leftio->active_reqdangling withuv_read_startstill active. The stale pointer survived until theuv_closecallback drained on teardown, where theactive_reqbackstop inlibuv_io_event_disposecalleddispose()through freed memory →0xC0000005afterEnd. Intermittent because the freed Zend-MM block is usually still intact (silent double-free) and only crashes when it was reused betweenfclose()and shutdown.Fix
uv_read_stop/uv_udp_recv_stopand detachesio->active_reqbefore freeing.uv_write/uv_fs_*/ sendfile (not synchronously cancellable): defer the free with aUV_IN_FLIGHT/DISPOSE_PENDINGrendezvous, mirroring the one that already existed for UDPsendto. The completion callback finishes the deferred dispose and skips the NOTIFY; a queued-but-not-started threadpool op isuv_cancel'ed.io_file_stat_cbalso skips writing the stat result into the vanished awaiter's buffer.uv_closerendezvous, so every fs submission (plusfs_open's pending io and both sendfile ios) now holds an event reference for the op's duration.uv_close; final teardown is now transferred toio_close_cb.Verification
Built ZTS on Windows (VS17 x64). Full
ext/asyncsuite: 988 passed, 0 failed (1 pre-existing XFAIL-but-passes WARN, unrelated). Stress-ranio/04260/60 clean, no access violation.