libtimeinstate: fix bug when cpu count exceeds CPUS_PER_ENTRY

In getUidCpuFreqTimes() and getUidConcurrentTimes(), in the case where
findMapEntry fails but getFirstMapKey succeeds we overwrite key and
then continue to the next loop iteration. This can cause problems when
gNCpus > CPUS_PER_ENTRY causing us to use multiple buckets: in both
functions, subsequent loop iterations could use the wrong UID, and in
getUidConcurrentTimes() we can enter an infinite loop if
getFirstMapKey() overwrites key.bucket with a lower value.

Instead, use a separate time_key_t variable for the call to
getFirstMapKey(). To prevent similar bugs, make key a const variable
scoped to a single loop iteration.

Bug: 229788490
Test: libtimeinstate_test
Signed-off-by: Connor O'Brien <connoro@google.com>
Change-Id: I491aa3bf4c5bce9218bdc44a5eee6ea426b151ae

1 files changed, 7 insertions, 6 deletions

diff --git a/libs/cputimeinstate/cputimeinstate.cpp b/libs/cputimeinstate/cputimeinstate.cpp
index 7e9bb7d468..d1690437ef 100644
--- a/libs/cputimeinstate/cputimeinstate.cpp
+++ b/libs/cputimeinstate/cputimeinstate.cpp
@@ -300,11 +300,11 @@ std::optional<std::vector<std::vector<uint64_t>>> getUidCpuFreqTimes(uint32_t ui
     }
 
     std::vector<tis_val_t> vals(gNCpus);
-    time_key_t key = {.uid = uid};
     for (uint32_t i = 0; i <= (maxFreqCount - 1) / FREQS_PER_ENTRY; ++i) {
-        key.bucket = i;
+        const time_key_t key = {.uid = uid, .bucket = i};
         if (findMapEntry(gTisMapFd, &key, vals.data())) {
-            if (errno != ENOENT || getFirstMapKey(gTisMapFd, &key)) return {};
+            time_key_t tmpKey;
+            if (errno != ENOENT || getFirstMapKey(gTisMapFd, &tmpKey)) return {};
             continue;
         }
 
@@ -412,10 +412,11 @@ std::optional<concurrent_time_t> getUidConcurrentTimes(uint32_t uid, bool retry)
     concurrent_time_t ret = {.active = std::vector<uint64_t>(gNCpus, 0)};
     for (const auto &cpuList : gPolicyCpus) ret.policy.emplace_back(cpuList.size(), 0);
     std::vector<concurrent_val_t> vals(gNCpus);
-    time_key_t key = {.uid = uid};
-    for (key.bucket = 0; key.bucket <= (gNCpus - 1) / CPUS_PER_ENTRY; ++key.bucket) {
+    for (uint32_t i = 0; i <= (gNCpus - 1) / CPUS_PER_ENTRY; ++i) {
+        const time_key_t key = {.uid = uid, .bucket = i};
         if (findMapEntry(gConcurrentMapFd, &key, vals.data())) {
-            if (errno != ENOENT || getFirstMapKey(gConcurrentMapFd, &key)) return {};
+            time_key_t tmpKey;
+            if (errno != ENOENT || getFirstMapKey(gConcurrentMapFd, &tmpKey)) return {};
             continue;
         }
         auto offset = key.bucket * CPUS_PER_ENTRY;