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

Commit 35045cbc authored by David Chen's avatar David Chen
Browse files

Fix uidmap in statsd. 

Previously tried an optimization that results in corrupted proto
output. This changes to a safer approach of storing the snapshot data
in memory and only converting to proto output when the
ProtoOutputStream is provided.

Also fixes a security issue when trying to invoke triggerUidSnapshot
since we forgot to use SCS' permissions.

Test: Added a unit-test to verify output of StatsLogProcessor.
Bug: 76231867
Change-Id: Id410ce3505fda9d71caa71942ef3068b55872c66
parent 775e291c

cmds/statsd/src/packages/UidMap.cpp

+20 −48
Original line number Diff line number Diff line
@@ -116,32 +116,16 @@ void UidMap::updateMap(const int64_t& timestamp, const vector<int32_t>& uid, 
        lock_guard<mutex> lock(mMutex);  // Exclusively lock for updates.



        mMap.clear();

        ProtoOutputStream proto;

        uint64_t token = proto.start(FIELD_TYPE_MESSAGE | FIELD_COUNT_REPEATED |

                                      FIELD_ID_SNAPSHOT_PACKAGE_INFO);

        vector<const SnapshotPackageInfo> infos;

        for (size_t j = 0; j < uid.size(); j++) {

            string package = string(String8(packageName[j]).string());

            mMap.insert(make_pair(uid[j], AppData(package, versionCode[j])));

            proto.write(FIELD_TYPE_STRING | FIELD_ID_SNAPSHOT_PACKAGE_NAME, package);

            proto.write(FIELD_TYPE_INT32 | FIELD_ID_SNAPSHOT_PACKAGE_VERSION, (int)versionCode[j]);

            proto.write(FIELD_TYPE_INT32 | FIELD_ID_SNAPSHOT_PACKAGE_UID, (int)uid[j]);

        }

        proto.end(token);



        // Copy ProtoOutputStream output to

        auto iter = proto.data();

        size_t pos = 0;

        vector<char> outData(proto.size());

        while (iter.readBuffer() != NULL) {

            size_t toRead = iter.currentToRead();

            std::memcpy(&(outData[pos]), iter.readBuffer(), toRead);

            pos += toRead;

            iter.rp()->move(toRead);

        }

        SnapshotRecord record(timestamp, outData);

        mSnapshots.push_back(record);



        mBytesUsed += proto.size() + kBytesTimestampField;

            infos.emplace_back(package, versionCode[j], uid[j]);

        }



        mSnapshots.emplace_back(timestamp, infos);



        mBytesUsed += mSnapshots.back().bytes;

        ensureBytesUsedBelowLimit();

        StatsdStats::getInstance().setCurrentUidMapMemory(mBytesUsed);

        StatsdStats::getInstance().setUidMapSnapshots(mSnapshots.size());
@@ -212,7 +196,7 @@ void UidMap::ensureBytesUsedBelowLimit() { 
    while (mBytesUsed > limit) {

        ALOGI("Bytes used %zu is above limit %zu, need to delete something", mBytesUsed, limit);

        if (mSnapshots.size() > 0) {

            mBytesUsed -= mSnapshots.front().bytes.size() + kBytesTimestampField;

            mBytesUsed -= mSnapshots.front().bytes;

            mSnapshots.pop_front();

            StatsdStats::getInstance().noteUidMapDropped(1, 0);

        } else if (mChanges.size() > 0) {
@@ -365,8 +349,14 @@ void UidMap::appendUidMap(const int64_t& timestamp, const ConfigKey& key, 
            count++;

            proto->write(FIELD_TYPE_INT64 | FIELD_ID_SNAPSHOT_TIMESTAMP,

                         (long long)record.timestampNs);

            proto->write(FIELD_TYPE_MESSAGE | FIELD_ID_SNAPSHOT_PACKAGE_INFO, record.bytes.data(),

                         record.bytes.size());

            for (const SnapshotPackageInfo& info : record.infos) {

                uint64_t token = proto->start(FIELD_TYPE_MESSAGE | FIELD_COUNT_REPEATED |

                                              FIELD_ID_SNAPSHOT_PACKAGE_INFO);

                proto->write(FIELD_TYPE_STRING | FIELD_ID_SNAPSHOT_PACKAGE_NAME, info.package);

                proto->write(FIELD_TYPE_INT32 | FIELD_ID_SNAPSHOT_PACKAGE_VERSION, info.version);

                proto->write(FIELD_TYPE_INT32 | FIELD_ID_SNAPSHOT_PACKAGE_UID, info.uid);

                proto->end(token);

            }

            proto->end(snapshotsToken);

        }

    }
@@ -380,7 +370,7 @@ void UidMap::appendUidMap(const int64_t& timestamp, const ConfigKey& key, 
        int64_t cutoff_nanos = newMin;

        for (auto it_snapshots = mSnapshots.begin(); it_snapshots != mSnapshots.end();) {

            if (it_snapshots->timestampNs < cutoff_nanos) {

                mBytesUsed -= it_snapshots->bytes.size() + kBytesTimestampField;

                mBytesUsed -= it_snapshots->bytes;

                it_snapshots = mSnapshots.erase(it_snapshots);

            } else {

                ++it_snapshots;
@@ -399,31 +389,13 @@ void UidMap::appendUidMap(const int64_t& timestamp, const ConfigKey& key, 
            // Produce another snapshot. This results in extra data being uploaded but

            // helps ensure we can re-construct the UID->app name, versionCode mapping

            // in server.

            ProtoOutputStream snapshotProto;

            uint64_t token = snapshotProto.start(FIELD_TYPE_MESSAGE | FIELD_COUNT_REPEATED |

                                                 FIELD_ID_SNAPSHOT_PACKAGE_INFO);

            vector<const SnapshotPackageInfo> infos;

            for (const auto& it : mMap) {

                snapshotProto.write(FIELD_TYPE_STRING | FIELD_ID_SNAPSHOT_PACKAGE_NAME,

                                    it.second.packageName);

                snapshotProto.write(FIELD_TYPE_INT32 | FIELD_ID_SNAPSHOT_PACKAGE_VERSION,

                                    (int)it.second.versionCode);

                snapshotProto.write(FIELD_TYPE_INT32 | FIELD_ID_SNAPSHOT_PACKAGE_UID,

                                    (int)it.first);

            }

            snapshotProto.end(token);



            // Copy ProtoOutputStream output to

            auto iter = snapshotProto.data();

            vector<char> snapshotData(snapshotProto.size());

            size_t pos = 0;

            while (iter.readBuffer() != NULL) {

                size_t toRead = iter.currentToRead();

                std::memcpy(&(snapshotData[pos]), iter.readBuffer(), toRead);

                pos += toRead;

                iter.rp()->move(toRead);

            }

            mSnapshots.emplace_back(timestamp, snapshotData);

            mBytesUsed += kBytesTimestampField + snapshotData.size();

                infos.emplace_back(it.second.packageName, it.second.versionCode, it.first);

            }



            mSnapshots.emplace_back(timestamp, infos);

            mBytesUsed += mSnapshots.back().bytes;

        }

    }

    StatsdStats::getInstance().setCurrentUidMapMemory(mBytesUsed);

cmds/statsd/src/packages/UidMap.h

+24 −5
Original line number Diff line number Diff line
@@ -74,17 +74,35 @@ const unsigned int kBytesChangeRecord = sizeof(struct ChangeRecord); 
// less because of varint encoding).

const unsigned int kBytesTimestampField = 10;



struct SnapshotPackageInfo {

    const string package;

    const int32_t version;

    const int32_t uid;

    SnapshotPackageInfo(const string& package, const int32_t version, const int32_t uid)

        : package(package), version(version), uid(uid) {

    }

};



const unsigned int kBytesSnapshotInfo = sizeof(struct SnapshotPackageInfo);



// When calling appendUidMap, we retrieve all the snapshots since the last

// timestamp we called appendUidMap for this configuration key.

struct SnapshotRecord {

    const int64_t timestampNs;



    // For performance reasons, we convert the package_info field (which is a

    // repeated field of PackageInfo messages).

    vector<char> bytes;

    // All the package info known.

    vector<const SnapshotPackageInfo> infos;



    SnapshotRecord(const int64_t timestampNs, vector<char> bytes)

        : timestampNs(timestampNs), bytes(bytes) {

    // Tracks the number of bytes this snapshot consumes.

    uint32_t bytes;



    SnapshotRecord(const int64_t timestampNs, vector<const SnapshotPackageInfo>& infos)

        : timestampNs(timestampNs), infos(infos) {

        bytes = 0;

        for (auto info : infos) {

            bytes += info.package.size() + kBytesSnapshotInfo;

        }

        bytes += kBytesTimestampField;

    }

};
@@ -210,6 +228,7 @@ private: 


    // Allows unit-test to access private methods.

    FRIEND_TEST(UidMapTest, TestClearingOutput);

    FRIEND_TEST(UidMapTest, TestOutputIncludesAtLeastOneSnapshot);

    FRIEND_TEST(UidMapTest, TestMemoryComputed);

    FRIEND_TEST(UidMapTest, TestMemoryGuardrail);

};

cmds/statsd/tests/StatsLogProcessor_test.cpp

+27 −0
Original line number Diff line number Diff line
@@ -14,6 +14,7 @@ 


#include "StatsLogProcessor.h"

#include "config/ConfigKey.h"

#include "frameworks/base/cmds/statsd/src/stats_log.pb.h"

#include "frameworks/base/cmds/statsd/src/statsd_config.pb.h"

#include "guardrail/StatsdStats.h"

#include "logd/LogEvent.h"
@@ -122,6 +123,32 @@ TEST(StatsLogProcessorTest, TestDropWhenByteSizeTooLarge) { 
    EXPECT_EQ(0, broadcastCount);

}



TEST(StatsLogProcessorTest, TestUidMapHasSnapshot) {

    // Setup simple config key corresponding to empty config.

    sp<UidMap> m = new UidMap();

    m->updateMap({1, 2}, {1, 2}, {String16("p1"), String16("p2")});

    sp<AlarmMonitor> anomalyAlarmMonitor;

    sp<AlarmMonitor> subscriberAlarmMonitor;

    int broadcastCount = 0;

    StatsLogProcessor p(m, anomalyAlarmMonitor, subscriberAlarmMonitor, 0,

                        [&broadcastCount](const ConfigKey& key) { broadcastCount++; });

    ConfigKey key(3, 4);

    StatsdConfig config;

    config.add_allowed_log_source("AID_ROOT");

    p.OnConfigUpdated(key, config);



    // Expect to get no metrics, but snapshot specified above in uidmap.

    vector<uint8_t> bytes;

    p.onDumpReport(key, 1, &bytes);



    ConfigMetricsReportList output;

    output.ParseFromArray(bytes.data(), bytes.size());

    EXPECT_TRUE(output.reports_size() > 0);

    auto uidmap = output.reports(0).uid_map();

    EXPECT_TRUE(uidmap.snapshots_size() > 0);

    EXPECT_EQ(2, uidmap.snapshots(0).package_info_size());

}



#else

GTEST_LOG_(INFO) << "This test does nothing.\n";

#endif

cmds/statsd/tests/UidMap_test.cpp

+28 −1
Original line number Diff line number Diff line
@@ -173,6 +173,33 @@ static void protoOutputStreamToUidMapping(ProtoOutputStream* proto, UidMapping* 
    results->ParseFromArray(bytes.data(), bytes.size());

}



// Test that uid map returns at least one snapshot even if we already obtained

// this snapshot from a previous call to getData.

TEST(UidMapTest, TestOutputIncludesAtLeastOneSnapshot) {

    UidMap m;

    // Initialize single config key.

    ConfigKey config1(1, StringToId("config1"));

    m.OnConfigUpdated(config1);

    vector<int32_t> uids;

    vector<int64_t> versions;

    vector<String16> apps;

    uids.push_back(1000);

    apps.push_back(String16(kApp2.c_str()));

    versions.push_back(5);

    m.updateMap(1, uids, versions, apps);



    // Set the last timestamp for this config key to be newer.

    m.mLastUpdatePerConfigKey[config1] = 2;



    ProtoOutputStream proto;

    m.appendUidMap(3, config1, &proto);



    // Check there's still a uidmap attached this one.

    UidMapping results;

    protoOutputStreamToUidMapping(&proto, &results);

    EXPECT_EQ(1, results.snapshots_size());

}



TEST(UidMapTest, TestClearingOutput) {

    UidMap m;
@@ -199,7 +226,7 @@ TEST(UidMapTest, TestClearingOutput) { 
    protoOutputStreamToUidMapping(&proto, &results);

    EXPECT_EQ(1, results.snapshots_size());



    // It should be cleared now

    // We have to keep at least one snapshot in memory at all times.

    EXPECT_EQ(1U, m.mSnapshots.size());

    proto.clear();

    m.appendUidMap(2, config1, &proto);

services/core/java/com/android/server/stats/StatsCompanionService.java

+3 −0
Original line number Diff line number Diff line
@@ -972,10 +972,13 @@ public class StatsCompanionService extends IStatsCompanionService.Stub { 
    public void triggerUidSnapshot() {

        enforceCallingPermission();

        synchronized (sStatsdLock) {

            final long token = Binder.clearCallingIdentity();

            try {

                informAllUidsLocked(mContext);

            } catch (RemoteException e) {

                Slog.e(TAG, "Failed to trigger uid snapshot.", e);

            } finally {

                restoreCallingIdentity(token);

            }

        }

    }