commit 30aa4375d1a6e0045449ea2cdd43500f39b06d89
author Huihong Luo <huisinro@google.com> Mon Oct 03 14:54:12 2022 -0700
committer Huihong Luo <huisinro@google.com> Wed Dec 07 23:57:18 2022 -0800
tree f733addd19c9ac71feb61c788d627031812b09ee
parent a84abc85435e3b3519e77cbebf396bf3e2d14d6e

Fix data type error in AIDL

utf8InCpp string should not be used to pass arbitrary data, as it drops
invalid chars during utf8-utf16 conversion.
byte[] is the correct type to use.

Bug: 250401609
Test: atest libgui_test libsurfaceflinger_unittest SurfaceFlinger_test
Change-Id: Ia1bf7e2cdc134a392d7b4edace9b2799a4c0d23b

services/surfaceflinger/TimeStats/TimeStats.cpp

diff --git a/services/surfaceflinger/TimeStats/TimeStats.cpp b/services/surfaceflinger/TimeStats/TimeStats.cpp
index e860d88..9368edd 100644
--- a/services/surfaceflinger/TimeStats/TimeStats.cpp
+++ b/services/surfaceflinger/TimeStats/TimeStats.cpp
@@ -90,7 +90,7 @@
 }
 } // namespace
 
-bool TimeStats::populateGlobalAtom(std::string* pulledData) {
+bool TimeStats::populateGlobalAtom(std::vector<uint8_t>* pulledData) {
     std::lock_guard<std::mutex> lock(mMutex);
 
     if (mTimeStats.statsStartLegacy == 0) {
@@ -138,10 +138,11 @@
     // Always clear data.
     clearGlobalLocked();
 
-    return atomList.SerializeToString(pulledData);
+    pulledData->resize(atomList.ByteSizeLong());
+    return atomList.SerializeToArray(pulledData->data(), atomList.ByteSizeLong());
 }
 
-bool TimeStats::populateLayerAtom(std::string* pulledData) {
+bool TimeStats::populateLayerAtom(std::vector<uint8_t>* pulledData) {
     std::lock_guard<std::mutex> lock(mMutex);
 
     std::vector<TimeStatsHelper::TimeStatsLayer*> dumpStats;
@@ -229,7 +230,8 @@
     // Always clear data.
     clearLayersLocked();
 
-    return atomList.SerializeToString(pulledData);
+    pulledData->resize(atomList.ByteSizeLong());
+    return atomList.SerializeToArray(pulledData->data(), atomList.ByteSizeLong());
 }
 
 TimeStats::TimeStats() : TimeStats(std::nullopt, std::nullopt) {}
@@ -245,7 +247,7 @@
     }
 }
 
-bool TimeStats::onPullAtom(const int atomId, std::string* pulledData) {
+bool TimeStats::onPullAtom(const int atomId, std::vector<uint8_t>* pulledData) {
     bool success = false;
     if (atomId == 10062) { // SURFACEFLINGER_STATS_GLOBAL_INFO
         success = populateGlobalAtom(pulledData);

services/surfaceflinger/TimeStats/TimeStats.h

diff --git a/services/surfaceflinger/TimeStats/TimeStats.h b/services/surfaceflinger/TimeStats/TimeStats.h
index 61d7c22..1872d0e 100644
--- a/services/surfaceflinger/TimeStats/TimeStats.h
+++ b/services/surfaceflinger/TimeStats/TimeStats.h
@@ -47,7 +47,7 @@
     virtual ~TimeStats() = default;
 
     // Process a pull request from statsd.
-    virtual bool onPullAtom(const int atomId, std::string* pulledData) = 0;
+    virtual bool onPullAtom(const int atomId, std::vector<uint8_t>* pulledData) = 0;
 
     virtual void parseArgs(bool asProto, const Vector<String16>& args, std::string& result) = 0;
     virtual bool isEnabled() = 0;
@@ -244,7 +244,7 @@
     TimeStats(std::optional<size_t> maxPulledLayers,
               std::optional<size_t> maxPulledHistogramBuckets);
 
-    bool onPullAtom(const int atomId, std::string* pulledData) override;
+    bool onPullAtom(const int atomId, std::vector<uint8_t>* pulledData) override;
     void parseArgs(bool asProto, const Vector<String16>& args, std::string& result) override;
     bool isEnabled() override;
     std::string miniDump() override;
@@ -292,8 +292,8 @@
     static const size_t MAX_NUM_TIME_RECORDS = 64;
 
 private:
-    bool populateGlobalAtom(std::string* pulledData);
-    bool populateLayerAtom(std::string* pulledData);
+    bool populateGlobalAtom(std::vector<uint8_t>* pulledData);
+    bool populateLayerAtom(std::vector<uint8_t>* pulledData);
     bool recordReadyLocked(int32_t layerId, TimeRecord* timeRecord);
     void flushAvailableRecordsToStatsLocked(int32_t layerId, Fps displayRefreshRate,
                                             std::optional<Fps> renderRate, SetFrameRateVote,