From 11e0d40bbeaeac913e6dd22cd8c7cebe1445e347 Mon Sep 17 00:00:00 2001 From: Max Dashouk Date: Thu, 16 May 2019 16:58:07 -0700 Subject: [PATCH] Fixes the bug that can occur when StatsCompanionService calls StatsService to update UID data and overflows kernel transfer buffer. In this case, the IPC call silently fails. The issue was discovered in Android Automotive Embedded use case that employs multiuser setup. This causes more uid data being sent via one-way StatsCompanionService::informAllUidData call than usual and can trigger the issue. As the result, uid map on statsd side is empty and many metrics are not captured. Bug: b/132444397 Fixes: b/132444397 Test: Did a clean build of master branch and flashed the device. adb shell cmd stats print-uid-map returned without any result. Repeated the steps after implementing the fix, print-uid-map returns the results now. Change-Id: I1451c13b36696449c145c51618c68d10e29a596a --- cmds/statsd/Android.bp | 1 + cmds/statsd/src/StatsService.cpp | 48 ++++++-- cmds/statsd/src/StatsService.h | 5 +- cmds/statsd/src/uid_data.proto | 36 ++++++ core/java/android/os/IStatsManager.aidl | 10 +- .../server/stats/StatsCompanionService.java | 104 +++++++++++++----- 6 files changed, 160 insertions(+), 44 deletions(-) create mode 100644 cmds/statsd/src/uid_data.proto diff --git a/cmds/statsd/Android.bp b/cmds/statsd/Android.bp index 15d248f06c7ca..d59d0e238bdcb 100644 --- a/cmds/statsd/Android.bp +++ b/cmds/statsd/Android.bp @@ -52,6 +52,7 @@ cc_defaults { ":statsd_aidl", "src/active_config_list.proto", "src/statsd_config.proto", + "src/uid_data.proto", "src/FieldValue.cpp", "src/hash.cpp", "src/stats_log_util.cpp", diff --git a/cmds/statsd/src/StatsService.cpp b/cmds/statsd/src/StatsService.cpp index 8191d37bb6038..f646f190f557b 100644 --- a/cmds/statsd/src/StatsService.cpp +++ b/cmds/statsd/src/StatsService.cpp @@ -35,6 +35,7 @@ #include #include #include +#include #include #include #include @@ -953,16 +954,49 @@ bool StatsService::getUidFromString(const char* s, int32_t& uid) { || (callingUid == AID_ROOT && goodUid == AID_SHELL); // ROOT can impersonate SHELL. } -Status StatsService::informAllUidData(const vector& uid, const vector& version, - const vector& version_string, - const vector& app, - const vector& installer) { +Status StatsService::informAllUidData(const ParcelFileDescriptor& fd) { ENFORCE_UID(AID_SYSTEM); + // Read stream into buffer. + string buffer; + if (!android::base::ReadFdToString(fd.get(), &buffer)) { + return exception(Status::EX_ILLEGAL_ARGUMENT, "Failed to read all data from the pipe."); + } - VLOG("StatsService::informAllUidData was called"); - mUidMap->updateMap(getElapsedRealtimeNs(), uid, version, version_string, app, installer); - VLOG("StatsService::informAllUidData succeeded"); + // Parse buffer. + UidData uidData; + if (!uidData.ParseFromString(buffer)) { + return exception(Status::EX_ILLEGAL_ARGUMENT, "Error parsing proto stream for UidData."); + } + vector versionStrings; + vector installers; + vector packageNames; + vector uids; + vector versions; + + const auto numEntries = uidData.app_info_size(); + versionStrings.reserve(numEntries); + installers.reserve(numEntries); + packageNames.reserve(numEntries); + uids.reserve(numEntries); + versions.reserve(numEntries); + + for (const auto& appInfo: uidData.app_info()) { + packageNames.emplace_back(String16(appInfo.package_name().c_str())); + uids.push_back(appInfo.uid()); + versions.push_back(appInfo.version()); + versionStrings.emplace_back(String16(appInfo.version_string().c_str())); + installers.emplace_back(String16(appInfo.installer().c_str())); + } + + mUidMap->updateMap(getElapsedRealtimeNs(), + uids, + versions, + versionStrings, + packageNames, + installers); + + VLOG("StatsService::informAllUidData UidData proto parsed successfully."); return Status::ok(); } diff --git a/cmds/statsd/src/StatsService.h b/cmds/statsd/src/StatsService.h index a4e6d7fec4cc3..9d50a0030a664 100644 --- a/cmds/statsd/src/StatsService.h +++ b/cmds/statsd/src/StatsService.h @@ -33,6 +33,7 @@ #include #include #include +#include #include #include @@ -72,9 +73,7 @@ public: virtual Status informPollAlarmFired(); virtual Status informAlarmForSubscriberTriggeringFired(); - virtual Status informAllUidData(const vector& uid, const vector& version, - const vector& version_string, - const vector& app, const vector& installer); + virtual Status informAllUidData(const ParcelFileDescriptor& fd); virtual Status informOnePackage(const String16& app, int32_t uid, int64_t version, const String16& version_string, const String16& installer); virtual Status informOnePackageRemoved(const String16& app, int32_t uid); diff --git a/cmds/statsd/src/uid_data.proto b/cmds/statsd/src/uid_data.proto new file mode 100644 index 0000000000000..a6fa26cd412e9 --- /dev/null +++ b/cmds/statsd/src/uid_data.proto @@ -0,0 +1,36 @@ +/* + * Copyright (C) 2019 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +syntax = "proto2"; + +package android.os.statsd; + +option java_package = "com.android.internal.os"; +option java_outer_classname = "UidDataProto"; + +message ApplicationInfo { + optional int32 uid = 1; + optional int64 version = 2; + optional string version_string = 3; + optional string package_name = 4; + optional string installer = 5; +} + +// StatsServiceCompanion uses the proto to supply statsd with uid-package +// mapping updates. +message UidData { + repeated ApplicationInfo app_info = 1; +} diff --git a/core/java/android/os/IStatsManager.aidl b/core/java/android/os/IStatsManager.aidl index 311c86d08211d..df0363cf9e769 100644 --- a/core/java/android/os/IStatsManager.aidl +++ b/core/java/android/os/IStatsManager.aidl @@ -17,6 +17,7 @@ package android.os; import android.os.IStatsPullerCallback; +import android.os.ParcelFileDescriptor; /** * Binder interface to communicate with the statistics management service. @@ -61,11 +62,12 @@ interface IStatsManager { void informDeviceShutdown(); /** - * Inform statsd what the version and package are for each uid. Note that each array should - * have the same number of elements, and version[i] and package[i] correspond to uid[i]. + * Inform statsd about a file descriptor for a pipe through which we will pipe version + * and package information for each uid. + * Versions and package information are supplied via UidData proto where info for each app + * is captured in its own element of a repeated ApplicationInfo message. */ - oneway void informAllUidData(in int[] uid, in long[] version, in String[] version_string, - in String[] app, in String[] installer); + oneway void informAllUidData(in ParcelFileDescriptor fd); /** * Inform statsd what the uid, version, version_string, and installer are for one app that was diff --git a/services/core/java/com/android/server/stats/StatsCompanionService.java b/services/core/java/com/android/server/stats/StatsCompanionService.java index 669656db97173..de03c4072fa8a 100644 --- a/services/core/java/com/android/server/stats/StatsCompanionService.java +++ b/services/core/java/com/android/server/stats/StatsCompanionService.java @@ -109,10 +109,12 @@ import android.util.Log; import android.util.Slog; import android.util.StatsLog; import android.util.proto.ProtoOutputStream; +import android.util.proto.ProtoStream; import com.android.internal.annotations.GuardedBy; import com.android.internal.app.procstats.IProcessStats; import com.android.internal.app.procstats.ProcessStats; +import com.android.internal.os.BackgroundThread; import com.android.internal.os.BatterySipper; import com.android.internal.os.BatteryStatsHelper; import com.android.internal.os.BinderCallsStats.ExportedCallStat; @@ -184,6 +186,16 @@ public class StatsCompanionService extends IStatsCompanionService.Stub { static final String TAG = "StatsCompanionService"; static final boolean DEBUG = false; + /** + * Hard coded field ids of frameworks/base/cmds/statsd/src/uid_data.proto + * to be used in ProtoOutputStream. + */ + private static final int APPLICATION_INFO_FIELD_ID = 1; + private static final int UID_FIELD_ID = 1; + private static final int VERSION_FIELD_ID = 2; + private static final int VERSION_STRING_FIELD_ID = 3; + private static final int PACKAGE_NAME_FIELD_ID = 4; + private static final int INSTALLER_FIELD_ID = 5; public static final int CODE_DATA_BROADCAST = 1; public static final int CODE_SUBSCRIBER_BROADCAST = 1; @@ -455,39 +467,71 @@ public class StatsCompanionService extends IStatsCompanionService.Stub { Slog.d(TAG, "Iterating over " + users.size() + " profiles."); } - List uids = new ArrayList<>(); - List versions = new ArrayList<>(); - List apps = new ArrayList<>(); - List versionStrings = new ArrayList<>(); - List installers = new ArrayList<>(); - - // Add in all the apps for every user/profile. - for (UserInfo profile : users) { - List pi = - pm.getInstalledPackagesAsUser(PackageManager.MATCH_KNOWN_PACKAGES, profile.id); - for (int j = 0; j < pi.size(); j++) { - if (pi.get(j).applicationInfo != null) { - String installer; - try { - installer = pm.getInstallerPackageName(pi.get(j).packageName); - } catch (IllegalArgumentException e) { - installer = ""; + ParcelFileDescriptor[] fds; + try { + fds = ParcelFileDescriptor.createPipe(); + } catch (IOException e) { + Slog.e(TAG, "Failed to create a pipe to send uid map data.", e); + return; + } + sStatsd.informAllUidData(fds[0]); + try { + fds[0].close(); + } catch (IOException e) { + Slog.e(TAG, "Failed to close the read side of the pipe.", e); + } + final ParcelFileDescriptor writeFd = fds[1]; + BackgroundThread.getHandler().post(() -> { + FileOutputStream fout = new ParcelFileDescriptor.AutoCloseOutputStream(writeFd); + try { + ProtoOutputStream output = new ProtoOutputStream(fout); + int numRecords = 0; + // Add in all the apps for every user/profile. + for (UserInfo profile : users) { + List pi = + pm.getInstalledPackagesAsUser(PackageManager.MATCH_KNOWN_PACKAGES, + profile.id); + for (int j = 0; j < pi.size(); j++) { + if (pi.get(j).applicationInfo != null) { + String installer; + try { + installer = pm.getInstallerPackageName(pi.get(j).packageName); + } catch (IllegalArgumentException e) { + installer = ""; + } + long applicationInfoToken = + output.start(ProtoStream.FIELD_TYPE_MESSAGE + | ProtoStream.FIELD_COUNT_REPEATED + | APPLICATION_INFO_FIELD_ID); + output.write(ProtoStream.FIELD_TYPE_INT32 + | ProtoStream.FIELD_COUNT_SINGLE | UID_FIELD_ID, + pi.get(j).applicationInfo.uid); + output.write(ProtoStream.FIELD_TYPE_INT64 + | ProtoStream.FIELD_COUNT_SINGLE + | VERSION_FIELD_ID, pi.get(j).getLongVersionCode()); + output.write(ProtoStream.FIELD_TYPE_STRING + | ProtoStream.FIELD_COUNT_SINGLE | VERSION_STRING_FIELD_ID, + pi.get(j).versionName); + output.write(ProtoStream.FIELD_TYPE_STRING + | ProtoStream.FIELD_COUNT_SINGLE + | PACKAGE_NAME_FIELD_ID, pi.get(j).packageName); + output.write(ProtoStream.FIELD_TYPE_STRING + | ProtoStream.FIELD_COUNT_SINGLE + | INSTALLER_FIELD_ID, + installer == null ? "" : installer); + numRecords++; + output.end(applicationInfoToken); + } } - installers.add(installer == null ? "" : installer); - uids.add(pi.get(j).applicationInfo.uid); - versions.add(pi.get(j).getLongVersionCode()); - versionStrings.add(pi.get(j).versionName); - apps.add(pi.get(j).packageName); } + output.flush(); + if (DEBUG) { + Slog.d(TAG, "Sent data for " + numRecords + " apps"); + } + } finally { + IoUtils.closeQuietly(fout); } - } - sStatsd.informAllUidData(toIntArray(uids), toLongArray(versions), - versionStrings.toArray(new String[versionStrings.size()]), - apps.toArray(new String[apps.size()]), - installers.toArray(new String[installers.size()])); - if (DEBUG) { - Slog.d(TAG, "Sent data for " + uids.size() + " apps"); - } + }); } private final static class AppUpdateReceiver extends BroadcastReceiver {