SF: Fix UAF due to outliving idle timer
On Scheduler destruction, stop the timer and clear the callbacks, since
the RefreshRateConfigs may outlive the Scheduler.
On RefreshRateConfigs destruction, ensure the timer is destroyed before
the mutex/callbacks. Invoke the callback under lock.
Remove TestableSurfaceFlinger's subtle precondition that setupScheduler
must be called prior to FakeDisplayDeviceInjector for them to share
RefreshRateConfigs, which was not the case in DisplayModeSwitchingTest.
TestableScheduler is deleted through a Scheduler pointer, so ~Scheduler
should be virtual. Although ~TestableScheduler is trivial, this invoked
undefined behavior.
Bug: 213688734
Test: libsurfaceflinger_unittest --gtest_repeat=1000
Change-Id: Id23fafaf3d7071a5e28e275de386dd731a726006
diff --git a/services/surfaceflinger/Scheduler/RefreshRateConfigs.h b/services/surfaceflinger/Scheduler/RefreshRateConfigs.h
index 4bbdab6..fc45d2b 100644
--- a/services/surfaceflinger/Scheduler/RefreshRateConfigs.h
+++ b/services/surfaceflinger/Scheduler/RefreshRateConfigs.h
@@ -348,16 +348,24 @@
bool supportsKernelIdleTimer() const { return mConfig.supportKernelIdleTimer; }
- void setIdleTimerCallbacks(std::function<void()> platformTimerReset,
- std::function<void()> platformTimerExpired,
- std::function<void()> kernelTimerReset,
- std::function<void()> kernelTimerExpired) {
+ struct IdleTimerCallbacks {
+ struct Callbacks {
+ std::function<void()> onReset;
+ std::function<void()> onExpired;
+ };
+
+ Callbacks platform;
+ Callbacks kernel;
+ };
+
+ void setIdleTimerCallbacks(IdleTimerCallbacks callbacks) EXCLUDES(mIdleTimerCallbacksMutex) {
std::scoped_lock lock(mIdleTimerCallbacksMutex);
- mIdleTimerCallbacks.emplace();
- mIdleTimerCallbacks->platform.onReset = std::move(platformTimerReset);
- mIdleTimerCallbacks->platform.onExpired = std::move(platformTimerExpired);
- mIdleTimerCallbacks->kernel.onReset = std::move(kernelTimerReset);
- mIdleTimerCallbacks->kernel.onExpired = std::move(kernelTimerExpired);
+ mIdleTimerCallbacks = std::move(callbacks);
+ }
+
+ void clearIdleTimerCallbacks() EXCLUDES(mIdleTimerCallbacksMutex) {
+ std::scoped_lock lock(mIdleTimerCallbacksMutex);
+ mIdleTimerCallbacks.reset();
}
void startIdleTimer() {
@@ -380,7 +388,7 @@
return;
}
mIdleTimer->reset();
- };
+ }
void dump(std::string& result) const EXCLUDES(mLock);
@@ -448,6 +456,13 @@
void initializeIdleTimer();
+ std::optional<IdleTimerCallbacks::Callbacks> getIdleTimerCallbacks() const
+ REQUIRES(mIdleTimerCallbacksMutex) {
+ if (!mIdleTimerCallbacks) return {};
+ return mConfig.supportKernelIdleTimer ? mIdleTimerCallbacks->kernel
+ : mIdleTimerCallbacks->platform;
+ }
+
// The list of refresh rates, indexed by display modes ID. This may change after this
// object is initialized.
AllRefreshRatesMapType mRefreshRates GUARDED_BY(mLock);
@@ -492,21 +507,11 @@
mutable std::optional<GetBestRefreshRateInvocation> lastBestRefreshRateInvocation
GUARDED_BY(mLock);
- // Timer that records time between requests for next vsync.
- std::optional<scheduler::OneShotTimer> mIdleTimer;
-
- struct IdleTimerCallbacks {
- struct Callbacks {
- std::function<void()> onReset;
- std::function<void()> onExpired;
- };
-
- Callbacks platform;
- Callbacks kernel;
- };
-
+ // Declare mIdleTimer last to ensure its thread joins before the mutex/callbacks are destroyed.
std::mutex mIdleTimerCallbacksMutex;
std::optional<IdleTimerCallbacks> mIdleTimerCallbacks GUARDED_BY(mIdleTimerCallbacksMutex);
+ // Used to detect (lack of) frame activity.
+ std::optional<scheduler::OneShotTimer> mIdleTimer;
};
} // namespace android::scheduler