[Backport][0.9 to 0.8] | | fix(iouring): plug UAF on _async_io early returns (#1569) (#1619)

io/iouring-wrapper.cpp

@@ -55,6 +55,10 @@ class iouringEngine : public MasterEventEngine, public CascadingEventEngine, pub
     int reset() override {
         fini();
         m_event_contexts.clear();
+        // The old ring died along with all its in-flight requests, so their
+        // CQEs can never arrive. Bump the generation to release the waiters
+        // in _async_io.
+        m_generation++;
         return init();
     }
 
@@ -182,19 +186,29 @@ class iouringEngine : public MasterEventEngine, public CascadingEventEngine, pub
     }
 
     int32_t _async_io(io_uring_sqe* sqe, Timeout timeout, uint32_t ring_flags) {
+        auto* first_sqe = sqe;
+        auto gen = m_generation;
         sqe->flags |= (uint8_t) (ring_flags & 0xff);
         ioCtx io_ctx(false, false);
         io_uring_sqe_set_data(sqe, &io_ctx);
 
         ioCtx timer_ctx(true, false);
         __kernel_timespec ts;
         auto usec = timeout.timeout_us();
-        if (usec < (uint64_t)std::numeric_limits<int64_t>::max()) {
+        bool has_timer = usec < (uint64_t) std::numeric_limits<int64_t>::max();
+        if (has_timer) {
             sqe->flags |= IOSQE_IO_LINK;
             ts = usec_to_timespec(usec);
             sqe = _get_sqe();
-            if (sqe == nullptr)
-                return -1;
+            if (sqe == nullptr) {
+                // The first SQE is already in the SQ ring and will be submitted
+                // sooner or later. Turn it into a harmless NOP without user data
+                // (prep_rw clears sqe->flags, including IOSQE_IO_LINK), so that
+                // its CQE won't reference the stack contexts after we return.
+                io_uring_prep_nop(first_sqe);
+                io_uring_sqe_set_data(first_sqe, nullptr);
+                return -1;      // errno was set to EBUSY by _get_sqe
+            }
             io_uring_prep_link_timeout(sqe, &ts, 0);
             io_uring_sqe_set_data(sqe, &timer_ctx);
         }
@@ -213,13 +227,37 @@ class iouringEngine : public MasterEventEngine, public CascadingEventEngine, pub
         } else {
             // Interrupted by external user thread. Try to cancel the previous I/O
             ERRNO err_backup;
+            if (gen != m_generation) {
+                // reset() has re-created the ring (e.g. after fork). Our I/O
+                // died with the old ring, so nothing refers to the stack
+                // contexts anymore, and there is nothing to cancel.
+                errno = err_backup.no;
+                return -1;
+            }
             sqe = _get_sqe();
-            if (sqe == nullptr)
+            if (sqe == nullptr) {
+                // Unable to cancel. Wait for the in-flight I/O (and its linked
+                // timer) to complete, before the stack-allocated contexts go
+                // out of scope.
+                while (gen == m_generation &&
+                       (!io_ctx.done || (has_timer && !timer_ctx.done)))
+                    photon::thread_sleep(-1);
+                errno = err_backup.no;
                 return -1;
+            }
             ioCtx cancel_ctx(true, false);
             io_uring_prep_cancel(sqe, &io_ctx, 0);
             io_uring_sqe_set_data(sqe, &cancel_ctx);
-            photon::thread_sleep(-1);
+            // No explicit submit here: this engine submits lazily, from
+            // wait_and_fire_events(), which the loop below yields to.
+            // Wait until all in-flight CQEs referring to our stack contexts are
+            // reaped, regardless of premature wake-ups (shutdown truncation,
+            // external interrupts, or io/cancel CQEs arriving in different
+            // reap batches). A generation change means reset() has dropped the
+            // ring holding those CQEs, so they can never arrive.
+            while (gen == m_generation &&
+                   (!io_ctx.done || !cancel_ctx.done || (has_timer && !timer_ctx.done)))
+                photon::thread_sleep(-1);
             errno = err_backup.no;
             return -1;
         }
@@ -366,11 +404,20 @@ class iouringEngine : public MasterEventEngine, public CascadingEventEngine, pub
                 // The cqe for notify, corresponding to IORING_CQE_F_MORE
                 if (unlikely(cqe->res != 0))
                     LOG_WARN("iouring: send_zc fall back to copying");
+                assert(!ctx->is_event);
+                ctx->done = true;
                 photon::thread_interrupt(ctx->th_id, EOK);
                 continue;
             }
 
             ctx->res = cqe->res;
+            // A CQE without F_MORE is the final one of its request. Set `done`
+            // here, ahead of the -ECANCELED branches below: they `continue`,
+            // and both the I/O and its linked timer report -ECANCELED once the
+            // cancellation of _async_io takes effect, so setting it at the end
+            // of the loop body would leave those waiters stuck forever.
+            if (!(cqe->flags & IORING_CQE_F_MORE))
+                ctx->done = true;
             if (!ctx->is_canceller && ctx->res == -ECANCELED) {
                 // An I/O was canceled because of:
                 // 1. IORING_OP_LINK_TIMEOUT. Leave the interrupt job to the linked timer later.
@@ -432,6 +479,11 @@ class iouringEngine : public MasterEventEngine, public CascadingEventEngine, pub
         int32_t res = -1;
         bool is_canceller;
         bool is_event;
+        // Set by reap_events when the final CQE of this request arrives.
+        // Stack-allocated contexts in _async_io must not go out of scope
+        // before this flag turns true. No atomic needed, since a vCPU is
+        // single OS thread and work stealing is paused during the wait.
+        bool done = false;
     };
 
     struct eventCtx {
@@ -547,6 +599,9 @@ class iouringEngine : public MasterEventEngine, public CascadingEventEngine, pub
     bool m_master;
     io_uring* m_ring = nullptr;
     int m_eventfd = -1;
+    // Incremented by reset() each time the ring is re-created (e.g. after
+    // fork), invalidating all in-flight requests of the old ring.
+    uint64_t m_generation = 0;
     std::unordered_map<fdInterest, eventCtx, fdInterestHasher> m_event_contexts;
     static int m_register_files_flag;
     static int m_cooperative_task_flag;

io/test/CMakeLists.txt

@@ -21,4 +21,8 @@ add_test(NAME test-syncio COMMAND $<TARGET_FILE:test-syncio>)
 add_executable(test-iouring test-iouring.cpp)
 target_link_libraries(test-iouring PRIVATE photon_shared)
 add_test(NAME test-iouring COMMAND $<TARGET_FILE:test-iouring>)
+
+add_executable(test-iouring-uaf test-iouring-uaf.cpp)
+target_link_libraries(test-iouring-uaf PRIVATE photon_shared)
+add_test(NAME test-iouring-uaf COMMAND $<TARGET_FILE:test-iouring-uaf>)
 endif ()

io/test/test-iouring-uaf.cpp

@@ -0,0 +1,171 @@
+/*
+Copyright 2022 The Photon Authors
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+// Regression tests for issue #1270: use-after-free on the early-return paths
+// of iouringEngine::_async_io. The stack-allocated ioCtx must not go out of
+// scope while its CQE is still in flight, even when the waiting thread gets
+// interrupted by an external OS thread.
+
+#include <unistd.h>
+#include <atomic>
+#include <chrono>
+#include <thread>
+#include <photon/io/fd-events.h>
+#include <photon/thread/thread11.h>
+#include <photon/common/alog.h>
+#include <photon/photon.h>
+#include "../../test/gtest.h"
+
+// io_uring requires Linux kernel >= 5.1. Prints a skip notice and returns
+// false if unavailable. gtest 1.8 has no GTEST_SKIP, so callers must
+// `return` on false.
+static bool iouring_init() {
+    if (photon::init(photon::INIT_EVENT_IOURING, photon::INIT_IO_NONE) != 0) {
+        fprintf(stderr, "  [ SKIPPED ] io_uring is not supported on this kernel\n");
+        return false;
+    }
+    return true;
+}
+
+// Sanity check: the normal success/timeout paths are untouched by the fix.
+TEST(iouring_uaf, sanity) {
+    if (!iouring_init())
+        return;
+    DEFER(photon::fini());
+
+    int fds[2];
+    ASSERT_EQ(0, ::pipe(fds));
+    DEFER({ ::close(fds[0]); ::close(fds[1]); });
+
+    // Timeout path
+    ASSERT_EQ(-1, photon::wait_for_fd_readable(fds[0], 1000));
+    ASSERT_EQ(ETIMEDOUT, errno);
+
+    // Success path
+    char buf[1] = {};
+    ::write(fds[1], buf, 1);
+    ASSERT_EQ(0, photon::wait_for_fd_readable(fds[0], 1000 * 1000));
+}
+
+// An external std::thread interrupts a photon thread that keeps issuing
+// timed iouring I/O. Each interrupt drives _async_io into the cancel branch,
+// whose wait loop must survive premature wake-ups and only return after all
+// CQEs referring to the stack contexts have been reaped. Before the fix this
+// scenario corrupts the photon thread's stack (typically crashing or hanging).
+TEST(iouring_uaf, interrupt_storm) {
+    if (!iouring_init())
+        return;
+    DEFER(photon::fini());
+
+    int fds[2];
+    ASSERT_EQ(0, ::pipe(fds));
+    DEFER({ ::close(fds[0]); ::close(fds[1]); });
+
+    std::atomic<bool> stop{false};
+    std::atomic<uint64_t> rounds{0};
+    auto th = photon::thread_create11([&] {
+        while (!stop.load(std::memory_order_acquire)) {
+            // 1ms timeout, so the linked timer SQE is always present and the
+            // interrupt races against submit/complete/timeout at all phases
+            photon::wait_for_fd_readable(fds[0], 1000);
+            rounds.fetch_add(1, std::memory_order_relaxed);
+        }
+    });
+    photon::thread_enable_join(th);
+
+    std::thread interrupter([&] {
+        for (int i = 0; i < 5000; ++i) {
+            photon::thread_interrupt(th);
+            // Vary the interrupt timing to hit different interleavings
+            if (i % 4 == 0)
+                std::this_thread::sleep_for(std::chrono::microseconds(200));
+        }
+        // The last interrupt happens-before this store, so `th` is still
+        // alive whenever thread_interrupt touches it
+        stop.store(true, std::memory_order_release);
+    });
+    photon::thread_join((photon::join_handle*) th);
+    interrupter.join();
+    LOG_INFO("survived ` rounds of interrupted I/O", rounds.load());
+}
+
+// Same storm as above, but through multiple concurrent photon threads, so
+// that io/cancel/timer CQEs of different requests interleave in reap batches.
+TEST(iouring_uaf, interrupt_storm_multi_threads) {
+    if (!iouring_init())
+        return;
+    DEFER(photon::fini());
+
+    constexpr int kThreads = 8;
+    int fds[2];
+    ASSERT_EQ(0, ::pipe(fds));
+    DEFER({ ::close(fds[0]); ::close(fds[1]); });
+
+    std::atomic<bool> stop{false};
+    photon::thread* workers[kThreads];
+    for (int i = 0; i < kThreads; ++i) {
+        workers[i] = photon::thread_create11([&] {
+            while (!stop.load(std::memory_order_acquire))
+                photon::wait_for_fd_readable(fds[0], 1000);
+        });
+        photon::thread_enable_join(workers[i]);
+    }
+
+    std::thread interrupter([&] {
+        for (int i = 0; i < 5000; ++i) {
+            photon::thread_interrupt(workers[i % kThreads]);
+            if (i % 4 == 0)
+                std::this_thread::sleep_for(std::chrono::microseconds(100));
+        }
+        stop.store(true, std::memory_order_release);
+    });
+    for (auto* w : workers)
+        photon::thread_join((photon::join_handle*) w);
+    interrupter.join();
+}
+
+// Interrupt an in-flight I/O whose timeout is far in the future, then join
+// the thread and go through photon::fini. The join can only return after the
+// cancel-path wait loop has seen all its CQEs reaped, which guarantees the
+// engine is drained before it gets destroyed by fini. Note that calling fini
+// while a thread is still inside _async_io is not a supported usage, since
+// fini deletes the master engine before waiting for the remaining threads.
+TEST(iouring_uaf, shutdown_with_interrupted_inflight_io) {
+    if (!iouring_init())
+        return;
+
+    int fds[2];
+    ASSERT_EQ(0, ::pipe(fds));
+
+    auto th = photon::thread_create11([&] {
+        // Long timeout: only the interrupt below can terminate this I/O
+        photon::wait_for_fd_readable(fds[0], 100ULL * 1000 * 1000);
+    });
+    photon::thread_enable_join(th);
+    photon::thread_yield();     // let the I/O get prepared and submitted
+    photon::thread_interrupt(th);
+    photon::thread_join((photon::join_handle*) th);
+
+    ::close(fds[0]);
+    ::close(fds[1]);
+    ASSERT_EQ(0, photon::fini());
+}
+
+int main(int argc, char** argv) {
+    set_log_output_level(ALOG_INFO);
+    testing::InitGoogleTest(&argc, argv);
+    return RUN_ALL_TESTS();
+}