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
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 36ff3ec..2b077f6 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -1558,7 +1558,8 @@
return NO_ERROR;
}
-status_t SurfaceFlinger::onPullAtom(const int32_t atomId, std::string* pulledData, bool* success) {
+status_t SurfaceFlinger::onPullAtom(const int32_t atomId, std::vector<uint8_t>* pulledData,
+ bool* success) {
*success = mTimeStats->onPullAtom(atomId, pulledData);
return NO_ERROR;
}
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index 6ddcfbc..bff210a 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -527,7 +527,7 @@
void setPowerMode(const sp<IBinder>& displayToken, int mode);
status_t overrideHdrTypes(const sp<IBinder>& displayToken,
const std::vector<ui::Hdr>& hdrTypes);
- status_t onPullAtom(const int32_t atomId, std::string* pulledData, bool* success);
+ status_t onPullAtom(const int32_t atomId, std::vector<uint8_t>* pulledData, bool* success);
status_t getLayerDebugInfo(std::vector<gui::LayerDebugInfo>* outLayers);
status_t getColorManagement(bool* outGetColorManagement) const;
status_t getCompositionPreference(ui::Dataspace* outDataspace, ui::PixelFormat* outPixelFormat,
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);
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,
diff --git a/services/surfaceflinger/fuzzer/surfaceflinger_fuzzers_utils.h b/services/surfaceflinger/fuzzer/surfaceflinger_fuzzers_utils.h
index c0a6bdb..a15aecd 100644
--- a/services/surfaceflinger/fuzzer/surfaceflinger_fuzzers_utils.h
+++ b/services/surfaceflinger/fuzzer/surfaceflinger_fuzzers_utils.h
@@ -419,7 +419,7 @@
void onPullAtom(FuzzedDataProvider *fdp) {
const int32_t atomId = fdp->ConsumeIntegral<uint8_t>();
- std::string pulledData = fdp->ConsumeRandomLengthString().c_str();
+ std::vector<uint8_t> pulledData = fdp->ConsumeRemainingBytes<uint8_t>();
bool success = fdp->ConsumeBool();
mFlinger->onPullAtom(atomId, &pulledData, &success);
}
diff --git a/services/surfaceflinger/tests/unittests/TimeStatsTest.cpp b/services/surfaceflinger/tests/unittests/TimeStatsTest.cpp
index 1dd4f25..0aaefe3 100644
--- a/services/surfaceflinger/tests/unittests/TimeStatsTest.cpp
+++ b/services/surfaceflinger/tests/unittests/TimeStatsTest.cpp
@@ -1099,8 +1099,10 @@
kGameMode, JankType::None, DISPLAY_DEADLINE_DELTA,
DISPLAY_PRESENT_JITTER, APP_DEADLINE_DELTA});
+ std::vector<uint8_t> pulledBytes;
+ EXPECT_TRUE(mTimeStats->onPullAtom(10062 /*SURFACEFLINGER_STATS_GLOBAL_INFO*/, &pulledBytes));
std::string pulledData;
- EXPECT_TRUE(mTimeStats->onPullAtom(10062 /*SURFACEFLINGER_STATS_GLOBAL_INFO*/, &pulledData));
+ pulledData.assign(pulledBytes.begin(), pulledBytes.end());
android::surfaceflinger::SurfaceflingerStatsGlobalInfoWrapper atomList;
ASSERT_TRUE(atomList.ParseFromString(pulledData));
@@ -1234,8 +1236,10 @@
GameMode::Standard, JankType::None, DISPLAY_DEADLINE_DELTA,
DISPLAY_PRESENT_JITTER, APP_DEADLINE_DELTA_3MS});
+ std::vector<uint8_t> pulledBytes;
+ EXPECT_TRUE(mTimeStats->onPullAtom(10063 /*SURFACEFLINGER_STATS_LAYER_INFO*/, &pulledBytes));
std::string pulledData;
- EXPECT_TRUE(mTimeStats->onPullAtom(10063 /*SURFACEFLINGER_STATS_LAYER_INFO*/, &pulledData));
+ pulledData.assign(pulledBytes.begin(), pulledBytes.end());
SurfaceflingerStatsLayerInfoWrapper atomList;
ASSERT_TRUE(atomList.ParseFromString(pulledData));
@@ -1322,8 +1326,10 @@
insertTimeRecord(NORMAL_SEQUENCE, LAYER_ID_0, 5, 4000000, {}, GameMode::Battery);
insertTimeRecord(NORMAL_SEQUENCE, LAYER_ID_0, 6, 5000000, {}, GameMode::Custom);
+ std::vector<uint8_t> pulledBytes;
+ EXPECT_TRUE(mTimeStats->onPullAtom(10063 /*SURFACEFLINGER_STATS_LAYER_INFO*/, &pulledBytes));
std::string pulledData;
- EXPECT_TRUE(mTimeStats->onPullAtom(10063 /*SURFACEFLINGER_STATS_LAYER_INFO*/, &pulledData));
+ pulledData.assign(pulledBytes.begin(), pulledBytes.end());
SurfaceflingerStatsLayerInfoWrapper atomList;
ASSERT_TRUE(atomList.ParseFromString(pulledData));
@@ -1412,8 +1418,10 @@
insertTimeRecord(NORMAL_SEQUENCE, LAYER_ID_1, 1, 2000000);
insertTimeRecord(NORMAL_SEQUENCE, LAYER_ID_1, 2, 3000000);
+ std::vector<uint8_t> pulledBytes;
+ EXPECT_TRUE(mTimeStats->onPullAtom(10063 /*SURFACEFLINGER_STATS_LAYER_INFO*/, &pulledBytes));
std::string pulledData;
- EXPECT_TRUE(mTimeStats->onPullAtom(10063 /*SURFACEFLINGER_STATS_LAYER_INFO*/, &pulledData));
+ pulledData.assign(pulledBytes.begin(), pulledBytes.end());
SurfaceflingerStatsLayerInfoWrapper atomList;
ASSERT_TRUE(atomList.ParseFromString(pulledData));
@@ -1437,8 +1445,10 @@
mTimeStats->setPresentFenceGlobal(std::make_shared<FenceTime>(3000000));
mTimeStats->setPresentFenceGlobal(std::make_shared<FenceTime>(5000000));
+ std::vector<uint8_t> pulledBytes;
+ EXPECT_TRUE(mTimeStats->onPullAtom(10063 /*SURFACEFLINGER_STATS_LAYER_INFO*/, &pulledBytes));
std::string pulledData;
- EXPECT_TRUE(mTimeStats->onPullAtom(10063 /*SURFACEFLINGER_STATS_LAYER_INFO*/, &pulledData));
+ pulledData.assign(pulledBytes.begin(), pulledBytes.end());
SurfaceflingerStatsLayerInfoWrapper atomList;
ASSERT_TRUE(atomList.ParseFromString(pulledData));
@@ -1456,8 +1466,10 @@
insertTimeRecord(NORMAL_SEQUENCE, LAYER_ID_0, 3, 4000000);
insertTimeRecord(NORMAL_SEQUENCE, LAYER_ID_0, 4, 5000000);
+ std::vector<uint8_t> pulledBytes;
+ EXPECT_TRUE(mTimeStats->onPullAtom(10063 /*SURFACEFLINGER_STATS_LAYER_INFO*/, &pulledBytes));
std::string pulledData;
- EXPECT_TRUE(mTimeStats->onPullAtom(10063 /*SURFACEFLINGER_STATS_LAYER_INFO*/, &pulledData));
+ pulledData.assign(pulledBytes.begin(), pulledBytes.end());
SurfaceflingerStatsLayerInfoWrapper atomList;
ASSERT_TRUE(atomList.ParseFromString(pulledData));
@@ -1476,8 +1488,10 @@
insertTimeRecord(NORMAL_SEQUENCE, LAYER_ID_1, 2, 3000000);
insertTimeRecord(NORMAL_SEQUENCE, LAYER_ID_1, 4, 5000000);
+ std::vector<uint8_t> pulledBytes;
+ EXPECT_TRUE(mTimeStats->onPullAtom(10063 /*SURFACEFLINGER_STATS_LAYER_INFO*/, &pulledBytes));
std::string pulledData;
- EXPECT_TRUE(mTimeStats->onPullAtom(10063 /*SURFACEFLINGER_STATS_LAYER_INFO*/, &pulledData));
+ pulledData.assign(pulledBytes.begin(), pulledBytes.end());
SurfaceflingerStatsLayerInfoWrapper atomList;
ASSERT_TRUE(atomList.ParseFromString(pulledData));
diff --git a/services/surfaceflinger/tests/unittests/mock/MockTimeStats.h b/services/surfaceflinger/tests/unittests/mock/MockTimeStats.h
index 0dee800..86fbadc 100644
--- a/services/surfaceflinger/tests/unittests/mock/MockTimeStats.h
+++ b/services/surfaceflinger/tests/unittests/mock/MockTimeStats.h
@@ -27,7 +27,7 @@
TimeStats();
~TimeStats() override;
- MOCK_METHOD2(onPullAtom, bool(const int, std::string*));
+ MOCK_METHOD2(onPullAtom, bool(const int, std::vector<uint8_t>*));
MOCK_METHOD3(parseArgs, void(bool, const Vector<String16>&, std::string&));
MOCK_METHOD0(isEnabled, bool());
MOCK_METHOD0(miniDump, std::string());