[Backport][0.7 to 0.6] | | | | fix(iouring): plug UAF on _async_io early returns (#1569) (#1619) (#1623) (#1634) - #1639
Open
photonlibos wants to merge 3 commits into
Conversation
…y returns (#1569) (#1619) (#1623) (#1634) * [Backport][0.9 to 0.8] | | fix(iouring): plug UAF on _async_io early returns (#1569) (#1619) (#1623) * [Backport][main to 0.9] | fix(iouring): plug UAF on _async_io early returns (#1569) (#1619) * fix(iouring): plug UAF on _async_io early returns (#1569) The stack-allocated ioCtx/timer_ctx could go out of scope while their CQEs were still in flight, on three early-return paths: timer/cancel SQE allocation failure and the cancel branch itself. Introduce an ioCtx::done flag set by reap_events on the final CQE; neutralize the queued first SQE with a NOP, and wait on the done flags before returning. A generation counter unblocks the waiters once reset() (fork) drops the old ring, whose CQEs never arrive. Add regression test test-iouring-uaf. Fixes #1270. * chore: drop files not present in 0.9 * fix(iouring): resolve backport conflicts for the _async_io UAF fix This engine still has the pre-refactoring shape of main, so the fix had to be re-expressed rather than applied. - io/iouring-wrapper.cpp: set ctx->done in the CQE loop of wait_and_fire_events(), which here is separate from wait_for_events() and has no is_event handling, so main's unified reap_events() block is not imported. The flag is set right after ctx->res, guarded by !IORING_CQE_F_MORE, instead of at the end of the loop body: the -ECANCELED branches of this branch come first and `continue`, and both io_ctx and timer_ctx report -ECANCELED once a cancellation takes effect, so the end of the body is unreachable for exactly the contexts the new wait loops in _async_io block on. Verified by experiment: with the flag at the end, iouring_uaf.interrupt_storm hangs. - io/iouring-wrapper.cpp: drop the try_submit() call the cherry-pick brought into the cancel path. That helper does not exist here and cannot be ported, as it is gated on m_args.eager_submit, which this branch has no equivalent of; submission stays lazy, driven by wait_and_fire_events(), as it was before. Note that git reported no conflict for this hunk, so it would have been a build break. - io/test/CMakeLists.txt: keep the signalfdboom target of this branch and register test-iouring-uaf with add_executable/target_link_libraries/ add_test under the existing `if (NOT APPLE)`, since photon_add_test() and `if (LINUX)` come from main's test-helper refactoring. - io/test/test-iouring-uaf.cpp: restore the regression test, which the backport tooling dropped as a new file while still referencing it from CMakeLists.txt. It needs no adaptation. Validated: test-iouring-uaf passes all 4 cases, including the interrupt storms that drive the cancel path over ~1200 rounds; test-iouring (10) and test-fork (5) pass, the latter covering ForkTest.Iouring, i.e. the reset()/generation path that releases the waiters. --------- Co-authored-by: Jiangtian Feng <fengjiangtian.fjt@alibaba-inc.com> Co-authored-by: lihuiba <282919+lihuiba@users.noreply.github.com> Co-authored-by: Coldwings <coldwings@me.com> * chore: drop files not present in 0.8 * fix(iouring): resolve backport conflicts for the _async_io UAF fix Unlike the 0.9 backport, the engine here is structurally identical to the source branch, so the fix applies as-is and little adaptation was needed. - io/iouring-wrapper.cpp: the only conflict was whitespace around the numeric_limits cast, plus the has_timer variable the incoming side introduces. Take the incoming side, since the wait loops of the cancel path need has_timer. - io/test/test-iouring-uaf.cpp: restore the regression test, which the backport tooling again dropped as a new file. Identical to the 0.9 version, no adaptation needed. io/test/CMakeLists.txt needed no resolution: the 0.9 side already registers the test with add_executable/target_link_libraries/add_test, which is the style of this branch too. Checked the parts that applied without conflict, since a clean hunk is not necessarily a compiling one: the CQE loop of wait_and_fire_events() has the same shape as 0.9, with the -ECANCELED branches ahead of the F_MORE check, so the placement of ctx->done before them is still the correct one here. try_submit() is absent, as the 0.9 backport had already dropped it. Validated: test-iouring-uaf (4), test-iouring (10) and test-fork (5) all pass, the last one covering ForkTest.Iouring, i.e. the reset()/generation path that releases the waiters. --------- Co-authored-by: Jiangtian Feng <fengjiangtian.fjt@alibaba-inc.com> Co-authored-by: lihuiba <282919+lihuiba@users.noreply.github.com> Co-authored-by: Coldwings <coldwings@me.com> * chore: drop files not present in 0.7 * Update iouring-wrapper.cpp * test: restore test-iouring-uaf.cpp dropped by the backport The auto-PR drops every file the cherry-pick would add, but the CMakeLists.txt hunk registering the new test applies cleanly, leaving add_executable pointing at a missing source -- CMake configure then fails before anything is built. Restore the file verbatim from release/0.8, where it is green across the whole gcc 8.5-12 matrix. A file the cherry-pick adds should be kept by a backport; only a file the source branch modifies but the target branch lacks has to go. * Update iouring-wrapper.cpp * fix(iouring): restore has_timer in the _async_io wait loops The 0.8 -> 0.7 cherry-pick lost the has_timer variable, so both wait loops of the cancel path require timer_ctx.done unconditionally. A linked-timeout SQE is only queued when the timeout is finite, so an I/O with an infinite timeout gets no timer CQE and timer_ctx.done never turns true -- the waiter sleeps forever. Photon's own signal event loop is exactly such a caller: wait_for_signal() uses wait_for_fd_readable(sgfd) with the default timeout of -1. So any photon::fini() on an iouring vCPU with INIT_EVENT_SIGNAL deadlocks: sync_signal_fini() calls EventLoop::stop(), which interrupts the loop thread; that thread enters the cancel branch of _async_io and blocks on timer_ctx.done, while stop() itself blocks on m_cond until the loop thread reaches STOP. Neither side can proceed. This is the x86_64 CI failure: both processes of ForkTest.Iouring hang right after their signal handler runs, until the test SIGKILLs them, and the child/parent exit status assertions fail. ARM CI passed only because its workflow never sets PHOTON_ENABLE_URING=ON, so ForkTest.Iouring is compiled out and the whole engine is absent from libphoton. The four test-iouring-uaf cases cannot catch it either, as they all pass finite timeouts (1ms / 100s), keeping has_timer true. Both loops now match release/0.8 and main verbatim; the remaining textual difference in _async_io is only this branch's pre-existing signature (uint64_t timeout vs Timeout, and the usec_to_timespec out-parameter). Also drops the trailing whitespace the cherry-pick left on the first loop. Validated with PHOTON_ENABLE_URING=ON against a real ring: test-fork now passes all 5 cases including ForkTest.Iouring, test-iouring (10) and test-iouring-uaf (4, ~2000 interrupt rounds) stay green. Reverting only this hunk reproduces the CI failure exactly, force-kill warnings included. --------- Co-authored-by: Jiangtian Feng <fengjiangtian.fjt@alibaba-inc.com> Co-authored-by: lihuiba <282919+lihuiba@users.noreply.github.com> Co-authored-by: Coldwings <coldwings@me.com> Co-authored-by: Huiba Li <huiba.lhb@alibaba-inc.com>
photonlibos
added
bugfix
release/0.6 is the most divergent target of this cascade: besides the
pre-refactoring engine shape, it has no ResetHandle / reset() and no
fork support for iouring at all (no reset_handle.* , no test-fork). So
the fork-safety half of the upstream fix -- the m_generation counter
that releases the _async_io waiters after reset() drops the old ring --
has nothing to drive it here and was left out entirely. What remains is
the pure use-after-free fix on the early-return paths, re-expressed
against this branch:
- io/iouring-wrapper.cpp: add a `done` flag to ioCtx (kept an aggregate,
default-initialized, so the existing brace initializers are untouched),
set by the reap loop of wait_and_fire_events(). It is set right after
ctx->res, guarded by !IORING_CQE_F_MORE, ahead of the -ECANCELED
branches: those `continue`, and both io_ctx and timer_ctx report
-ECANCELED once a cancellation takes effect, so the end of the loop
body is unreachable for exactly the contexts the wait loops block on.
The F_NOTIF branch (send_zc) also sets it, as that CQE is the final one
of a zero-copy send.
- _async_io: on timer-SQE allocation failure, neutralize the already-queued
first SQE with a NOP and null user data instead of returning while its
CQE still points at the stack context. On the cancel path, keep the
branch's signature (uint64_t timeout, no ring_flags) and wait on the
done flags before returning: `has_timer` guards timer_ctx.done so an
infinite-timeout I/O (e.g. the signal loop, or this branch's cancel
poller, both of which poll with timeout -1 and thus queue no linked
timer) does not wait forever for a timer CQE that never comes.
- io/test/test-iouring-uaf.cpp: restored (the auto-PR dropped it as a new
file while keeping the CMake hunk that registers it). Adapted to 0.6:
include <gtest/gtest.h> directly, since this branch has no test/gtest.h
wrapper, and link ${testing_libs}, since photon_shared here does not
bring gtest transitively as it does on 0.7+.
Validated with PHOTON_ENABLE_URING=ON against a real ring: test-iouring-uaf
passes all 4 cases, including interrupt_storm (~2000 rounds driving the
cancel path) and shutdown_with_interrupted_inflight_io (the has_timer /
infinite-timeout path); test-iouring passes all 8, covering the
master_interrupted cases and the cancel-poller teardown on shutdown.
Coldwings
marked this pull request as ready for review
September 14, 2026 07:14
Coldwings
removed
the
needs-manual-merge
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
3 participants
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.
[Backport][0.9 to 0.8] | | fix(iouring): plug UAF on _async_io early returns (fix(iouring): plug UAF on _async_io early returns #1569) ([Backport][main to 0.9] | fix(iouring): plug UAF on _async_io early returns (#1569) #1619) ([Backport][0.9 to 0.8] | | fix(iouring): plug UAF on _async_io early returns (#1569) (#1619) #1623)
[Backport][main to 0.9] | fix(iouring): plug UAF on _async_io early returns (fix(iouring): plug UAF on _async_io early returns #1569) ([Backport][main to 0.9] | fix(iouring): plug UAF on _async_io early returns (#1569) #1619)
fix(iouring): plug UAF on _async_io early returns (fix(iouring): plug UAF on _async_io early returns #1569)
The stack-allocated ioCtx/timer_ctx could go out of scope while
their CQEs were still in flight, on three early-return paths:
timer/cancel SQE allocation failure and the cancel branch itself.
Introduce an ioCtx::done flag set by reap_events on the final CQE;
neutralize the queued first SQE with a NOP, and wait on the done
flags before returning. A generation counter unblocks the waiters
once reset() (fork) drops the old ring, whose CQEs never arrive.
Add regression test test-iouring-uaf. Fixes #1270.
chore: drop files not present in 0.9
fix(iouring): resolve backport conflicts for the _async_io UAF fix
This engine still has the pre-refactoring shape of main, so the fix had
to be re-expressed rather than applied.
wait_and_fire_events(), which here is separate from wait_for_events()
and has no is_event handling, so main's unified reap_events() block is
not imported. The flag is set right after ctx->res, guarded by
!IORING_CQE_F_MORE, instead of at the end of the loop body: the
-ECANCELED branches of this branch come first and
continue, and bothio_ctx and timer_ctx report -ECANCELED once a cancellation takes
effect, so the end of the body is unreachable for exactly the contexts
the new wait loops in _async_io block on. Verified by experiment: with
the flag at the end, iouring_uaf.interrupt_storm hangs.
brought into the cancel path. That helper does not exist here and
cannot be ported, as it is gated on m_args.eager_submit, which this
branch has no equivalent of; submission stays lazy, driven by
wait_and_fire_events(), as it was before. Note that git reported no
conflict for this hunk, so it would have been a build break.
register test-iouring-uaf with add_executable/target_link_libraries/
add_test under the existing
if (NOT APPLE), since photon_add_test()and
if (LINUX)come from main's test-helper refactoring.backport tooling dropped as a new file while still referencing it from
CMakeLists.txt. It needs no adaptation.
Validated: test-iouring-uaf passes all 4 cases, including the interrupt
storms that drive the cancel path over ~1200 rounds; test-iouring (10)
and test-fork (5) pass, the latter covering ForkTest.Iouring, i.e. the
reset()/generation path that releases the waiters.
Co-authored-by: Jiangtian Feng fengjiangtian.fjt@alibaba-inc.com
Co-authored-by: lihuiba 282919+lihuiba@users.noreply.github.com
Co-authored-by: Coldwings coldwings@me.com
chore: drop files not present in 0.8
fix(iouring): resolve backport conflicts for the _async_io UAF fix
Unlike the 0.9 backport, the engine here is structurally identical to the
source branch, so the fix applies as-is and little adaptation was needed.
numeric_limits cast, plus the has_timer variable the incoming side
introduces. Take the incoming side, since the wait loops of the cancel
path need has_timer.
backport tooling again dropped as a new file. Identical to the 0.9
version, no adaptation needed.
io/test/CMakeLists.txt needed no resolution: the 0.9 side already
registers the test with add_executable/target_link_libraries/add_test,
which is the style of this branch too.
Checked the parts that applied without conflict, since a clean hunk is
not necessarily a compiling one: the CQE loop of wait_and_fire_events()
has the same shape as 0.9, with the -ECANCELED branches ahead of the
F_MORE check, so the placement of ctx->done before them is still the
correct one here. try_submit() is absent, as the 0.9 backport had already
dropped it.
Validated: test-iouring-uaf (4), test-iouring (10) and test-fork (5) all
pass, the last one covering ForkTest.Iouring, i.e. the reset()/generation
path that releases the waiters.
Co-authored-by: Jiangtian Feng fengjiangtian.fjt@alibaba-inc.com
Co-authored-by: lihuiba 282919+lihuiba@users.noreply.github.com
Co-authored-by: Coldwings coldwings@me.com
chore: drop files not present in 0.7
Update iouring-wrapper.cpp
test: restore test-iouring-uaf.cpp dropped by the backport
The auto-PR drops every file the cherry-pick would add, but the CMakeLists.txt
hunk registering the new test applies cleanly, leaving add_executable pointing
at a missing source -- CMake configure then fails before anything is built.
Restore the file verbatim from release/0.8, where it is green across the whole
gcc 8.5-12 matrix. A file the cherry-pick adds should be kept by a backport;
only a file the source branch modifies but the target branch lacks has to go.
Update iouring-wrapper.cpp
fix(iouring): restore has_timer in the _async_io wait loops
The 0.8 -> 0.7 cherry-pick lost the has_timer variable, so both wait loops
of the cancel path require timer_ctx.done unconditionally. A linked-timeout
SQE is only queued when the timeout is finite, so an I/O with an infinite
timeout gets no timer CQE and timer_ctx.done never turns true -- the waiter
sleeps forever.
Photon's own signal event loop is exactly such a caller: wait_for_signal()
uses wait_for_fd_readable(sgfd) with the default timeout of -1. So any
photon::fini() on an iouring vCPU with INIT_EVENT_SIGNAL deadlocks:
sync_signal_fini() calls EventLoop::stop(), which interrupts the loop
thread; that thread enters the cancel branch of _async_io and blocks on
timer_ctx.done, while stop() itself blocks on m_cond until the loop thread
reaches STOP. Neither side can proceed.
This is the x86_64 CI failure: both processes of ForkTest.Iouring hang
right after their signal handler runs, until the test SIGKILLs them, and
the child/parent exit status assertions fail. ARM CI passed only because
its workflow never sets PHOTON_ENABLE_URING=ON, so ForkTest.Iouring is
compiled out and the whole engine is absent from libphoton. The four
test-iouring-uaf cases cannot catch it either, as they all pass finite
timeouts (1ms / 100s), keeping has_timer true.
Both loops now match release/0.8 and main verbatim; the remaining textual
difference in _async_io is only this branch's pre-existing signature
(uint64_t timeout vs Timeout, and the usec_to_timespec out-parameter).
Also drops the trailing whitespace the cherry-pick left on the first loop.
Validated with PHOTON_ENABLE_URING=ON against a real ring: test-fork now
passes all 5 cases including ForkTest.Iouring, test-iouring (10) and
test-iouring-uaf (4, ~2000 interrupt rounds) stay green. Reverting only
this hunk reproduces the CI failure exactly, force-kill warnings included.
Co-authored-by: Jiangtian Feng fengjiangtian.fjt@alibaba-inc.com
Co-authored-by: lihuiba 282919+lihuiba@users.noreply.github.com
Co-authored-by: Coldwings coldwings@me.com
Co-authored-by: Huiba Li huiba.lhb@alibaba-inc.com
Generated by Backport Auto PR, by cherry-pick related commits.
Please review and decide whether to merge or close this backport PR.
Conflicts
Cherry-pick produced conflicts. Conflict markers are committed as-is; please resolve them manually before merging.
Dropped new files
The following new files introduced by cherry-pick were dropped because they don't exist in
release/0.6: