diff --git a/remoting/host/win/desktop_event_handler_unittest.cc b/remoting/host/win/desktop_event_handler_unittest.cc index 0000000..0000001 100644 --- a/remoting/host/win/desktop_event_handler_unittest.cc +++ b/remoting/host/win/desktop_event_handler_unittest.cc @@ -14,6 +14,8 @@ #include "base/test/task_environment.h" #include "base/threading/thread_local.h" #include "testing/gtest/include/gtest/gtest.h" +#include "base/threading/platform_thread.h" +#include "base/timer/timer.h" namespace remoting { @@ -74,6 +76,62 @@ class TestDelegate : public DesktopEventHandler::Delegate { base::OnceClosure on_thread_stopped_; }; +// A delegate that mirrors MouseCursorMonitorWin::Delegate: it owns a heap +// object and a sequence-affine RepeatingTimer started on the worker thread. +// When DesktopEventHandler::Core::Stop() runs delegate_.reset() on the caller +// thread, the timer (and the heap object) are destroyed off-sequence while +// the worker thread may still be inside Poll(), producing a UAF on |target_|. +class RacingTimerDelegate : public DesktopEventHandler::Delegate { + public: + struct Target { + virtual ~Target() = default; + virtual void Touch() { + // Burn some cycles inside the virtual call so the destructor on the + // caller thread has a window to free us mid-call. + for (volatile int i = 0; i < 100000; ++i) { + } + } + }; + + explicit RacingTimerDelegate(base::OnceClosure on_started) + : on_started_(std::move(on_started)) {} + ~RacingTimerDelegate() override = default; + + void OnWorkerThreadStarted() override { + target_ = std::make_unique(); + // Mirrors capture_cursor_image_timer_ in MouseCursorMonitorWin::Delegate. + // Started on the worker sequence; will be destroyed on the caller sequence + // by Core::Stop() -> delegate_.reset(). + timer_.Start(FROM_HERE, base::Microseconds(50), this, + &RacingTimerDelegate::Poll); + if (on_started_) { + std::move(on_started_).Run(); + } + } + + void OnWorkerThreadStopping() override { + // Never reached during teardown via Stop(): Stop() resets delegate_ on the + // caller thread before posting DestroyWorkerThread(), so the null-check in + // DestroyWorkerThread() skips this. The timer is therefore never stopped + // on its own sequence. + timer_.Stop(); + } + + void OnEvent(DWORD event, LONG object_id) override {} + + private: + void Poll() { + // Virtual call on a heap object owned by this delegate; analogous to + // webrtc_monitor_->Capture() in MouseCursorMonitorWin::Delegate. + target_->Touch(); + } + + std::unique_ptr target_; + base::RepeatingTimer timer_; + base::OnceClosure on_started_; +}; + } // namespace TEST_F(DesktopEventHandlerTest, DeleteThreadAfterDestruction) { @@ -122,4 +180,28 @@ TEST_F(DesktopEventHandlerTest, TwoHandlers) { thread_stopped_run_loop_2.Run(); } +// Reproduces the cross-thread destruction race in Core::Stop(): +// delegate_.reset() runs on the caller sequence and frees objects that the +// worker thread's RepeatingTimer is concurrently dereferencing. +// Under is_asan=true on Windows this should report heap-use-after-free in +// RacingTimerDelegate::Poll / Target::Touch (and a sequence-checker DCHECK in +// ~TimerBase under is_debug=true). +TEST_F(DesktopEventHandlerTest, CrossThreadDelegateDestructionRace) { + for (int i = 0; i < 200; ++i) { + base::RunLoop started; + auto handler = std::make_unique(); + handler->Start(EVENT_MIN, EVENT_MAX, + std::make_unique(started.QuitClosure())); + // Wait for the worker thread to start the timer. + started.Run(); + // Let the high-frequency timer spin for a moment so a callback is likely + // in-flight when we tear down. + base::PlatformThread::Sleep(base::Milliseconds(1)); + // Triggers Core::Stop() -> delegate_.reset() on this (caller) thread. + handler.reset(); + base::RunLoop().RunUntilIdle(); + } +} + } // namespace remoting