commit 1e19ac97d44212d312cd304fb987c133d4c881d1
author Ady Abraham <adyabr@google.com> Tue Nov 17 16:56:00 2020 -0800
committer Ady Abraham <adyabr@google.com> Wed Nov 18 19:55:30 2020 +0000
tree c8ce5fc80f768aa87fd2fb5a82877480245e8724
parent b8871072670285c34d18c762d483b553c5473557

SurfaceFlinger: fix OneShotTimer timespec overflow

struct timespec::tv_nsec must be < 1s to be considered as valid
input to sem_clockwait.

Test: interact with device and observe systraces
Bug: 170665374
Change-Id: I4aa5b86cd55fd89adb12cd5a28ce599c8a1c0c93

services/surfaceflinger/Scheduler/OneShotTimer.cpp

diff --git a/services/surfaceflinger/Scheduler/OneShotTimer.cpp b/services/surfaceflinger/Scheduler/OneShotTimer.cpp
index 2783800..8e807d6 100644
--- a/services/surfaceflinger/Scheduler/OneShotTimer.cpp
+++ b/services/surfaceflinger/Scheduler/OneShotTimer.cpp
@@ -16,6 +16,8 @@
 
 #include "OneShotTimer.h"
 
+#include <utils/Log.h>
+#include <utils/Timers.h>
 #include <chrono>
 #include <sstream>
 #include <thread>
@@ -29,9 +31,9 @@
 // (tv_sec) is the whole count of seconds. The second (tv_nsec) is the
 // nanosecond part of the count. This function takes care of translation.
 void calculateTimeoutTime(std::chrono::nanoseconds timestamp, timespec* spec) {
-    clock_gettime(CLOCK_MONOTONIC, spec);
-    spec->tv_sec += static_cast<__kernel_time_t>(timestamp.count() / kNsToSeconds);
-    spec->tv_nsec += timestamp.count() % kNsToSeconds;
+    const nsecs_t timeout = systemTime(CLOCK_MONOTONIC) + timestamp.count();
+    spec->tv_sec = static_cast<__kernel_time_t>(timeout / kNsToSeconds);
+    spec->tv_nsec = timeout % kNsToSeconds;
 }
 } // namespace
 
@@ -47,7 +49,8 @@
 }
 
 void OneShotTimer::start() {
-    sem_init(&mSemaphore, 0, 0);
+    int result = sem_init(&mSemaphore, 0, 0);
+    LOG_ALWAYS_FATAL_IF(result, "sem_init failed");
 
     if (!mThread.joinable()) {
         // Only create thread if it has not been created.
@@ -57,11 +60,13 @@
 
 void OneShotTimer::stop() {
     mStopTriggered = true;
-    sem_post(&mSemaphore);
+    int result = sem_post(&mSemaphore);
+    LOG_ALWAYS_FATAL_IF(result, "sem_post failed");
 
     if (mThread.joinable()) {
         mThread.join();
-        sem_destroy(&mSemaphore);
+        result = sem_destroy(&mSemaphore);
+        LOG_ALWAYS_FATAL_IF(result, "sem_destroy failed");
     }
 }
 
@@ -77,7 +82,12 @@
         }
 
         if (state == TimerState::IDLE) {
-            sem_wait(&mSemaphore);
+            int result = sem_wait(&mSemaphore);
+            if (result && errno != EINTR) {
+                std::stringstream ss;
+                ss << "sem_wait failed (" << errno << ")";
+                LOG_ALWAYS_FATAL("%s", ss.str().c_str());
+            }
             continue;
         }
 
@@ -101,7 +111,12 @@
             // Wait for mInterval time for semaphore signal.
             struct timespec ts;
             calculateTimeoutTime(std::chrono::nanoseconds(mInterval), &ts);
-            sem_clockwait(&mSemaphore, CLOCK_MONOTONIC, &ts);
+            int result = sem_clockwait(&mSemaphore, CLOCK_MONOTONIC, &ts);
+            if (result && errno != ETIMEDOUT && errno != EINTR) {
+                std::stringstream ss;
+                ss << "sem_clockwait failed (" << errno << ")";
+                LOG_ALWAYS_FATAL("%s", ss.str().c_str());
+            }
 
             state = checkForResetAndStop(state);
             if (state == TimerState::RESET) {
@@ -135,7 +150,8 @@
 
 void OneShotTimer::reset() {
     mResetTriggered = true;
-    sem_post(&mSemaphore);
+    int result = sem_post(&mSemaphore);
+    LOG_ALWAYS_FATAL_IF(result, "sem_post failed");
 }
 
 std::string OneShotTimer::dump() const {