Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit e4217241 authored by TreeHugger Robot's avatar TreeHugger Robot Committed by Android (Google) Code Review
Browse files

Merge changes Id23fafaf,I50450881

* changes:
  SF: Fix UAF due to outliving idle timer
  SF: Fix leak in unit tests
parents 1b73395f 83bd771c
Loading
Loading
Loading
Loading
+10 −11
Original line number Diff line number Diff line
@@ -714,20 +714,19 @@ RefreshRateConfigs::RefreshRateConfigs(const DisplayModes& modes, DisplayModeId

void RefreshRateConfigs::initializeIdleTimer() {
    if (mConfig.idleTimerTimeoutMs > 0) {
        const auto getCallback = [this]() -> std::optional<IdleTimerCallbacks::Callbacks> {
            std::scoped_lock lock(mIdleTimerCallbacksMutex);
            if (!mIdleTimerCallbacks.has_value()) return {};
            return mConfig.supportKernelIdleTimer ? mIdleTimerCallbacks->kernel
                                                  : mIdleTimerCallbacks->platform;
        };

        mIdleTimer.emplace(
                "IdleTimer", std::chrono::milliseconds(mConfig.idleTimerTimeoutMs),
                [getCallback] {
                    if (const auto callback = getCallback()) callback->onReset();
                [this] {
                    std::scoped_lock lock(mIdleTimerCallbacksMutex);
                    if (const auto callbacks = getIdleTimerCallbacks()) {
                        callbacks->onReset();
                    }
                },
                [getCallback] {
                    if (const auto callback = getCallback()) callback->onExpired();
                [this] {
                    std::scoped_lock lock(mIdleTimerCallbacksMutex);
                    if (const auto callbacks = getIdleTimerCallbacks()) {
                        callbacks->onExpired();
                    }
                });
    }
}
+28 −23
Original line number Diff line number Diff line
@@ -348,16 +348,24 @@ public:

    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 @@ public:
            return;
        }
        mIdleTimer->reset();
    };
    }

    void dump(std::string& result) const EXCLUDES(mLock);

@@ -448,6 +456,13 @@ private:

    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 @@ private:
    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
+35 −5
Original line number Diff line number Diff line
@@ -62,6 +62,15 @@ namespace android::scheduler {
Scheduler::Scheduler(ICompositor& compositor, ISchedulerCallback& callback, FeatureFlags features)
      : impl::MessageQueue(compositor), mFeatures(features), mSchedulerCallback(callback) {}

Scheduler::~Scheduler() {
    // Stop timers and wait for their threads to exit.
    mDisplayPowerTimer.reset();
    mTouchTimer.reset();

    // Stop idle timer and clear callbacks, as the RefreshRateConfigs may outlive the Scheduler.
    setRefreshRateConfigs(nullptr);
}

void Scheduler::startTimers() {
    using namespace sysprop;
    using namespace std::string_literals;
@@ -84,11 +93,32 @@ void Scheduler::startTimers() {
    }
}

Scheduler::~Scheduler() {
    // Ensure the OneShotTimer threads are joined before we start destroying state.
    mDisplayPowerTimer.reset();
    mTouchTimer.reset();
    mRefreshRateConfigs.reset();
void Scheduler::setRefreshRateConfigs(std::shared_ptr<RefreshRateConfigs> configs) {
    {
        // The current RefreshRateConfigs instance may outlive this call, so unbind its idle timer.
        std::scoped_lock lock(mRefreshRateConfigsLock);
        if (mRefreshRateConfigs) {
            mRefreshRateConfigs->stopIdleTimer();
            mRefreshRateConfigs->clearIdleTimerCallbacks();
        }
    }
    {
        // Clear state that depends on the current instance.
        std::scoped_lock lock(mPolicyLock);
        mPolicy = {};
    }

    std::scoped_lock lock(mRefreshRateConfigsLock);
    mRefreshRateConfigs = std::move(configs);
    if (!mRefreshRateConfigs) return;

    mRefreshRateConfigs->setIdleTimerCallbacks(
            {.platform = {.onReset = [this] { idleTimerCallback(TimerState::Reset); },
                          .onExpired = [this] { idleTimerCallback(TimerState::Expired); }},
             .kernel = {.onReset = [this] { kernelIdleTimerCallback(TimerState::Reset); },
                        .onExpired = [this] { kernelIdleTimerCallback(TimerState::Expired); }}});

    mRefreshRateConfigs->startIdleTimer();
}

void Scheduler::run() {
+6 −32
Original line number Diff line number Diff line
@@ -74,12 +74,16 @@ class Scheduler : impl::MessageQueue {

public:
    Scheduler(ICompositor&, ISchedulerCallback&, FeatureFlags);
    ~Scheduler();
    virtual ~Scheduler();

    void createVsyncSchedule(FeatureFlags);
    void startTimers();
    void setRefreshRateConfigs(std::shared_ptr<RefreshRateConfigs>)
            EXCLUDES(mRefreshRateConfigsLock);

    void run();

    void createVsyncSchedule(FeatureFlags);

    using Impl::initVsync;
    using Impl::setInjector;

@@ -198,36 +202,6 @@ public:
    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();
+1 −1
Original line number Diff line number Diff line
@@ -384,7 +384,7 @@ SurfaceFlinger::SurfaceFlinger(Factory& factory, SkipInitializationTag)
        mInternalDisplayDensity(getDensityFromProperty("ro.sf.lcd_density", true)),
        mEmulatedDisplayDensity(getDensityFromProperty("qemu.sf.lcd_density", false)),
        mPowerAdvisor(*this),
        mWindowInfosListenerInvoker(new WindowInfosListenerInvoker(this)) {
        mWindowInfosListenerInvoker(sp<WindowInfosListenerInvoker>::make(*this)) {
    ALOGI("Using HWComposer service: %s", mHwcServiceName.c_str());
}

Loading