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/Scheduler.h b/services/surfaceflinger/Scheduler/Scheduler.h
index 818f1ed..548c34b 100644
--- a/services/surfaceflinger/Scheduler/Scheduler.h
+++ b/services/surfaceflinger/Scheduler/Scheduler.h
@@ -74,11 +74,15 @@
public:
Scheduler(ICompositor&, ISchedulerCallback&, FeatureFlags);
- ~Scheduler();
+ virtual ~Scheduler();
+
+ void startTimers();
+ void setRefreshRateConfigs(std::shared_ptr<RefreshRateConfigs>)
+ EXCLUDES(mRefreshRateConfigsLock);
+
+ void run();
void createVsyncSchedule(FeatureFlags);
- void startTimers();
- void run();
using Impl::initVsync;
using Impl::setInjector;
@@ -198,36 +202,6 @@
std::optional<Fps> getFrameRateOverride(uid_t uid) const
EXCLUDES(mRefreshRateConfigsLock, mFrameRateOverridesLock);
- void setRefreshRateConfigs(std::shared_ptr<RefreshRateConfigs> refreshRateConfigs)
- EXCLUDES(mRefreshRateConfigsLock) {
- // We need to stop the idle timer on the previous RefreshRateConfigs instance
- // and cleanup the scheduler's state before we switch to the other RefreshRateConfigs.
- {
- std::scoped_lock lock(mRefreshRateConfigsLock);
- if (mRefreshRateConfigs) mRefreshRateConfigs->stopIdleTimer();
- }
- {
- std::scoped_lock lock(mPolicyLock);
- mPolicy = {};
- }
- {
- std::scoped_lock lock(mRefreshRateConfigsLock);
- mRefreshRateConfigs = std::move(refreshRateConfigs);
- mRefreshRateConfigs->setIdleTimerCallbacks(
- [this] { std::invoke(&Scheduler::idleTimerCallback, this, TimerState::Reset); },
- [this] {
- std::invoke(&Scheduler::idleTimerCallback, this, TimerState::Expired);
- },
- [this] {
- std::invoke(&Scheduler::kernelIdleTimerCallback, this, TimerState::Reset);
- },
- [this] {
- std::invoke(&Scheduler::kernelIdleTimerCallback, this, TimerState::Expired);
- });
- mRefreshRateConfigs->startIdleTimer();
- }
- }
-
nsecs_t getVsyncPeriodFromRefreshRateConfigs() const EXCLUDES(mRefreshRateConfigsLock) {
std::scoped_lock lock(mRefreshRateConfigsLock);
return mRefreshRateConfigs->getCurrentRefreshRate().getVsyncPeriod();