Repository navigation
Conversation
push_raw_with_key queued the SQE first, then recorded the key in in_flight and leaked it. set_in_flight panics when the op carries a Poll Extra (reachable through Proactor::push_with_extra on a fusion build); the unwind then freed the key while the kernel held its pointer as user_data, a use-after-free once the CQE arrived. - push_raw_with_key records and leaks the key first, so queueing the SQE is its last step; a failed push takes the key back - push_with_extra completes the op with InvalidInput when the Extra was created for another driver type, instead of panicking
Member
|
Please split this PR into two ones. One for possible double-free, and one for driver type validation. |
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.
iour::Driver::push_raw_with_keyqueued the SQE first, with the key pointer asuser_data, and only then recorded the key inin_flightand leaked it withinto_raw. Anything that panicked in between unwound with the key still owned by the stack, so the key and its buffers were freed while the SQE sat in the ring (or was already running under SQPOLL). The CQE then madepoll_entriescallErasedKey::from_rawon freed memory, which is a use-after-free followed by a double free.One panic is reachable through the public API. On a fusion build,
Proactor::push_with_extraaccepts anExtrafrom a proactor of the other driver type, andset_in_flightgoes throughas_iour_mut, which panics with "Current driver is notio_uring" on a PollExtra.Fix
push_raw_with_keyfirst gets the io_uringExtra, which is the only step that can panic, before anything is recorded. It then records the key inin_flightand leaks it, and queues the SQE as its last step. Ifpush_rawfails, the key is taken back withfrom_rawand removed fromin_flight.push_with_extrachecks theExtraagainst the driver type with a new crate-privateExtra::is_for. Only the fusion driver can mismatch. On a mismatch, the op completes withInvalidInputand is handed back in theBufResult, so the signature doesn't change.Test
compio-driver/tests/foreign_extra.rspushes aPollOncewith a PollExtrainto an io_uring proactor, and an io_uringExtrainto a poll proactor. It's skipped when io_uring isn't available.io_uring".InvalidInput.--features alland--features all,polling, apart fromcompio-process'sarg0, which fails locally because busyboxshdispatches onargv[0]. The poll-only driver suite (--no-default-features --features polling,sync) passes too.polling) and macOS.Caveats
push_rawfails has no test, becausesubmit_autocan't be made to fail on demand.push_rawreaps CQEs and wakes their futures while our key is already inin_flightbut its SQE isn't queued. If a waker panics there, the key stays inin_flightwithout an SQE. That's safe, since the kernel never saw the pointer. TodayDriver::dropfrees it. With the teardown drain from fix(driver): drain in-flight io_uring ops before teardown #1135, teardown would wait the full timeout for a CQE that never comes, then leak the key.