From ad213748428d111c13c7a71107384c374e27a7f9 Mon Sep 17 00:00:00 2001 From: Jeffrey Huang Date: Mon, 16 Dec 2019 13:50:06 -0800 Subject: [PATCH 1/3] Creating PendingIntentRef Update #setDataFetchOperation to avoid using intentsender. Bug: 146074295 Test: Ran GTS Tests Change-Id: I7df5c6441725aa4e46fac18925664c9455f50fb9 --- apex/statsd/aidl/Android.bp | 1 + .../aidl/android/os/IPendingIntentRef.aidl | 46 ++++ .../android/os/IStatsCompanionService.aidl | 3 - .../aidl/android/os/IStatsManagerService.aidl | 11 +- apex/statsd/aidl/android/os/IStatsd.aidl | 6 +- .../android/server/stats/StatsCompanion.java | 64 +++++- .../server/stats/StatsCompanionService.java | 56 ++--- .../server/stats/StatsManagerService.java | 215 +++++++++++++++--- cmds/statsd/src/StatsService.cpp | 59 +++-- cmds/statsd/src/StatsService.h | 7 +- cmds/statsd/src/config/ConfigManager.cpp | 25 +- cmds/statsd/src/config/ConfigManager.h | 10 +- core/java/android/app/StatsManager.java | 8 +- 13 files changed, 381 insertions(+), 130 deletions(-) create mode 100644 apex/statsd/aidl/android/os/IPendingIntentRef.aidl diff --git a/apex/statsd/aidl/Android.bp b/apex/statsd/aidl/Android.bp index aed6ad9fc08a1..f8325d47e2688 100644 --- a/apex/statsd/aidl/Android.bp +++ b/apex/statsd/aidl/Android.bp @@ -18,6 +18,7 @@ filegroup { name: "statsd_aidl", srcs: [ + "android/os/IPendingIntentRef.aidl", "android/os/IPullAtomCallback.aidl", "android/os/IPullAtomResultReceiver.aidl", "android/os/IStatsCompanionService.aidl", diff --git a/apex/statsd/aidl/android/os/IPendingIntentRef.aidl b/apex/statsd/aidl/android/os/IPendingIntentRef.aidl new file mode 100644 index 0000000000000..6b9e467a7e157 --- /dev/null +++ b/apex/statsd/aidl/android/os/IPendingIntentRef.aidl @@ -0,0 +1,46 @@ +/* + * 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. + */ + +package android.os; + +import android.os.StatsDimensionsValue; + +/** + * Binder interface to hold a PendingIntent for StatsCompanionService. + * {@hide} + */ +interface IPendingIntentRef { + + /** + * Sends a broadcast to the specified PendingIntent that it should getData now. + * This should be only called from StatsCompanionService. + */ + oneway void sendDataBroadcast(long lastReportTimeNs); + + /** + * Send a broadcast to the specified PendingIntent notifying it that the list of active configs + * has changed. This should be only called from StatsCompanionService. + */ + oneway void sendActiveConfigsChangedBroadcast(in long[] configIds); + + /** + * Send a broadcast to the specified PendingIntent, along with the other information + * specified. This should only be called from StatsCompanionService. + */ + oneway void sendSubscriberBroadcast(long configUid, long configId, long subscriptionId, + long subscriptionRuleId, in String[] cookies, + in StatsDimensionsValue dimensionsValue); +} \ No newline at end of file diff --git a/apex/statsd/aidl/android/os/IStatsCompanionService.aidl b/apex/statsd/aidl/android/os/IStatsCompanionService.aidl index 5a6118ef81ca3..ec3a226ab5420 100644 --- a/apex/statsd/aidl/android/os/IStatsCompanionService.aidl +++ b/apex/statsd/aidl/android/os/IStatsCompanionService.aidl @@ -66,9 +66,6 @@ interface IStatsCompanionService { /** Pull the specified data. Results will be sent to statsd when complete. */ StatsLogEventWrapper[] pullData(int pullCode); - /** Send a broadcast to the specified PendingIntent's as IBinder that it should getData now. */ - oneway void sendDataBroadcast(in IBinder intentSender, long lastReportTimeNs); - /** * Send a broadcast to the specified PendingIntent's as IBinder notifying it that the list * of active configs has changed. diff --git a/apex/statsd/aidl/android/os/IStatsManagerService.aidl b/apex/statsd/aidl/android/os/IStatsManagerService.aidl index 45ba3a21ed5bb..151cdbd8770cb 100644 --- a/apex/statsd/aidl/android/os/IStatsManagerService.aidl +++ b/apex/statsd/aidl/android/os/IStatsManagerService.aidl @@ -30,11 +30,18 @@ interface IStatsManagerService { * memory consumed by the metrics for this configuration approach the pre-defined limits. There * can be at most one listener per config key. * - * Requires Manifest.permission.DUMP. + * Requires Manifest.permission.DUMP and Manifest.permission.PACKAGE_USAGE_STATS. */ - void setDataFetchOperation(long configKey, in PendingIntent pendingIntent, + void setDataFetchOperation(long configId, in PendingIntent pendingIntent, in String packageName); + /** + * Removes the data fetch operation for the specified configuration. + * + * Requires Manifest.permission.DUMP and Manifest.permission.PACKAGE_USAGE_STATS. + */ + void removeDataFetchOperation(long configId, in String packageName); + /** * Registers the given pending intent for this packagename. This intent is invoked when the * active status of any of the configs sent by this package changes and will contain a list of diff --git a/apex/statsd/aidl/android/os/IStatsd.aidl b/apex/statsd/aidl/android/os/IStatsd.aidl index cce79fae75879..986d483301a42 100644 --- a/apex/statsd/aidl/android/os/IStatsd.aidl +++ b/apex/statsd/aidl/android/os/IStatsd.aidl @@ -17,6 +17,7 @@ package android.os; import android.os.IStatsPullerCallback; +import android.os.IPendingIntentRef; import android.os.IPullAtomCallback; import android.os.ParcelFileDescriptor; @@ -114,14 +115,15 @@ interface IStatsd { * * Requires Manifest.permission.DUMP. */ - void setDataFetchOperation(long configKey, in IBinder intentSender, in String packageName); + void setDataFetchOperation(long configId, in IPendingIntentRef pendingIntentRef, + int callingUid); /** * Removes the data fetch operation for the specified configuration. * * Requires Manifest.permission.DUMP. */ - void removeDataFetchOperation(long configKey, in String packageName); + void removeDataFetchOperation(long configId, int callingUid); /** * Registers the given pending intent for this packagename. This intent is invoked when the diff --git a/apex/statsd/service/java/com/android/server/stats/StatsCompanion.java b/apex/statsd/service/java/com/android/server/stats/StatsCompanion.java index 71b52e26b7db6..6a0bf629cf5c9 100644 --- a/apex/statsd/service/java/com/android/server/stats/StatsCompanion.java +++ b/apex/statsd/service/java/com/android/server/stats/StatsCompanion.java @@ -16,7 +16,13 @@ package com.android.server.stats; +import android.app.PendingIntent; import android.content.Context; +import android.content.Intent; +import android.os.Binder; +import android.os.IPendingIntentRef; +import android.os.Process; +import android.os.StatsDimensionsValue; import android.util.Slog; import com.android.server.SystemService; @@ -28,6 +34,13 @@ public class StatsCompanion { private static final String TAG = "StatsCompanion"; private static final boolean DEBUG = false; + static void enforceStatsCompanionPermission(Context context) { + if (Binder.getCallingPid() == Process.myPid()) { + return; + } + context.enforceCallingPermission(android.Manifest.permission.STATSCOMPANION, null); + } + /** * Lifecycle class for both {@link StatsCompanionService} and {@link StatsManagerService}. */ @@ -63,8 +76,57 @@ public class StatsCompanion { super.onBootPhase(phase); if (phase == PHASE_THIRD_PARTY_APPS_CAN_START) { mStatsCompanionService.systemReady(); - mStatsManagerService.systemReady(); } } } + + /** + * Wrapper for {@link PendingIntent}. Allows Statsd to send PendingIntents. + */ + public static class PendingIntentRef extends IPendingIntentRef.Stub { + + private static final String TAG = "PendingIntentRef"; + + /** + * The last report time is provided with each intent registered to + * StatsManager#setFetchReportsOperation. This allows easy de-duping in the receiver if + * statsd is requesting the client to retrieve the same statsd data. The last report time + * corresponds to the last_report_elapsed_nanos that will provided in the current + * ConfigMetricsReport, and this timestamp also corresponds to the + * current_report_elapsed_nanos of the most recently obtained ConfigMetricsReport. + */ + private static final String EXTRA_LAST_REPORT_TIME = "android.app.extra.LAST_REPORT_TIME"; + private static final int CODE_DATA_BROADCAST = 1; + + private final PendingIntent mPendingIntent; + private final Context mContext; + + public PendingIntentRef(PendingIntent pendingIntent, Context context) { + mPendingIntent = pendingIntent; + mContext = context; + } + + @Override + public void sendDataBroadcast(long lastReportTimeNs) { + enforceStatsCompanionPermission(mContext); + Intent intent = new Intent(); + intent.putExtra(EXTRA_LAST_REPORT_TIME, lastReportTimeNs); + try { + mPendingIntent.send(mContext, CODE_DATA_BROADCAST, intent, null, null); + } catch (PendingIntent.CanceledException e) { + Slog.w(TAG, "Unable to send PendingIntent"); + } + } + + @Override + public void sendActiveConfigsChangedBroadcast(long[] configIds) { + // no-op + } + + @Override + public void sendSubscriberBroadcast(long configUid, long configId, long subscriptionId, + long subscriptionRuleId, String[] cookies, StatsDimensionsValue dimensionsValue) { + // no-op + } + } } diff --git a/apex/statsd/service/java/com/android/server/stats/StatsCompanionService.java b/apex/statsd/service/java/com/android/server/stats/StatsCompanionService.java index e4f73000223f4..d160f22d583f8 100644 --- a/apex/statsd/service/java/com/android/server/stats/StatsCompanionService.java +++ b/apex/statsd/service/java/com/android/server/stats/StatsCompanionService.java @@ -85,7 +85,6 @@ import android.os.IThermalService; import android.os.Looper; import android.os.ParcelFileDescriptor; import android.os.Parcelable; -import android.os.Process; import android.os.RemoteException; import android.os.ServiceManager; import android.os.StatFs; @@ -203,18 +202,8 @@ public class StatsCompanionService extends IStatsCompanionService.Stub { 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; public static final int CODE_ACTIVE_CONFIGS_BROADCAST = 1; - /** - * The last report time is provided with each intent registered to - * StatsManager#setFetchReportsOperation. This allows easy de-duping in the receiver if - * statsd is requesting the client to retrieve the same statsd data. The last report time - * corresponds to the last_report_elapsed_nanos that will provided in the current - * ConfigMetricsReport, and this timestamp also corresponds to the - * current_report_elapsed_nanos of the most recently obtained ConfigMetricsReport. - */ - public static final String EXTRA_LAST_REPORT_TIME = "android.app.extra.LAST_REPORT_TIME"; public static final int DEATH_THRESHOLD = 10; /** * Which native processes to snapshot memory for. @@ -454,22 +443,9 @@ public class StatsCompanionService extends IStatsCompanionService.Stub { KernelCpuThreadReaderSettingsObserver.getSettingsModifiedReader(mContext); } - @Override - public void sendDataBroadcast(IBinder intentSenderBinder, long lastReportTimeNs) { - enforceCallingPermission(); - IntentSender intentSender = new IntentSender(intentSenderBinder); - Intent intent = new Intent(); - intent.putExtra(EXTRA_LAST_REPORT_TIME, lastReportTimeNs); - try { - intentSender.sendIntent(mContext, CODE_DATA_BROADCAST, intent, null, null); - } catch (IntentSender.SendIntentException e) { - Slog.w(TAG, "Unable to send using IntentSender"); - } - } - @Override public void sendActiveConfigsChangedBroadcast(IBinder intentSenderBinder, long[] configIds) { - enforceCallingPermission(); + StatsCompanion.enforceStatsCompanionPermission(mContext); IntentSender intentSender = new IntentSender(intentSenderBinder); Intent intent = new Intent(); intent.putExtra(StatsManager.EXTRA_STATS_ACTIVE_CONFIG_KEYS, configIds); @@ -487,7 +463,7 @@ public class StatsCompanionService extends IStatsCompanionService.Stub { public void sendSubscriberBroadcast(IBinder intentSenderBinder, long configUid, long configKey, long subscriptionId, long subscriptionRuleId, String[] cookies, StatsDimensionsValue dimensionsValue) { - enforceCallingPermission(); + StatsCompanion.enforceStatsCompanionPermission(mContext); IntentSender intentSender = new IntentSender(intentSenderBinder); Intent intent = new Intent() @@ -770,7 +746,7 @@ public class StatsCompanionService extends IStatsCompanionService.Stub { @Override // Binder call public void setAnomalyAlarm(long timestampMs) { - enforceCallingPermission(); + StatsCompanion.enforceStatsCompanionPermission(mContext); if (DEBUG) Slog.d(TAG, "Setting anomaly alarm for " + timestampMs); final long callingToken = Binder.clearCallingIdentity(); try { @@ -786,7 +762,7 @@ public class StatsCompanionService extends IStatsCompanionService.Stub { @Override // Binder call public void cancelAnomalyAlarm() { - enforceCallingPermission(); + StatsCompanion.enforceStatsCompanionPermission(mContext); if (DEBUG) Slog.d(TAG, "Cancelling anomaly alarm"); final long callingToken = Binder.clearCallingIdentity(); try { @@ -798,7 +774,7 @@ public class StatsCompanionService extends IStatsCompanionService.Stub { @Override // Binder call public void setAlarmForSubscriberTriggering(long timestampMs) { - enforceCallingPermission(); + StatsCompanion.enforceStatsCompanionPermission(mContext); if (DEBUG) { Slog.d(TAG, "Setting periodic alarm in about " + (timestampMs @@ -817,7 +793,7 @@ public class StatsCompanionService extends IStatsCompanionService.Stub { @Override // Binder call public void cancelAlarmForSubscriberTriggering() { - enforceCallingPermission(); + StatsCompanion.enforceStatsCompanionPermission(mContext); if (DEBUG) { Slog.d(TAG, "Cancelling periodic alarm"); } @@ -831,7 +807,7 @@ public class StatsCompanionService extends IStatsCompanionService.Stub { @Override // Binder call public void setPullingAlarm(long nextPullTimeMs) { - enforceCallingPermission(); + StatsCompanion.enforceStatsCompanionPermission(mContext); if (DEBUG) { Slog.d(TAG, "Setting pulling alarm in about " + (nextPullTimeMs - SystemClock.elapsedRealtime())); @@ -849,7 +825,7 @@ public class StatsCompanionService extends IStatsCompanionService.Stub { @Override // Binder call public void cancelPullingAlarm() { - enforceCallingPermission(); + StatsCompanion.enforceStatsCompanionPermission(mContext); if (DEBUG) { Slog.d(TAG, "Cancelling pulling alarm"); } @@ -2455,7 +2431,7 @@ public class StatsCompanionService extends IStatsCompanionService.Stub { */ @Override // Binder call public StatsLogEventWrapper[] pullData(int tagId) { - enforceCallingPermission(); + StatsCompanion.enforceStatsCompanionPermission(mContext); if (DEBUG) { Slog.d(TAG, "Pulling " + tagId); } @@ -2690,12 +2666,11 @@ public class StatsCompanionService extends IStatsCompanionService.Stub { @Override // Binder call public void statsdReady() { - enforceCallingPermission(); + StatsCompanion.enforceStatsCompanionPermission(mContext); if (DEBUG) { Slog.d(TAG, "learned that statsdReady"); } sayHiToStatsd(); // tell statsd that we're ready too and link to it - mStatsManagerService.systemReady(); mContext.sendBroadcastAsUser(new Intent(StatsManager.ACTION_STATSD_STARTED) .addFlags(Intent.FLAG_RECEIVER_INCLUDE_BACKGROUND), UserHandle.SYSTEM, android.Manifest.permission.DUMP); @@ -2703,7 +2678,7 @@ public class StatsCompanionService extends IStatsCompanionService.Stub { @Override public void triggerUidSnapshot() { - enforceCallingPermission(); + StatsCompanion.enforceStatsCompanionPermission(mContext); synchronized (sStatsdLock) { final long token = Binder.clearCallingIdentity(); try { @@ -2716,13 +2691,6 @@ public class StatsCompanionService extends IStatsCompanionService.Stub { } } - private void enforceCallingPermission() { - if (Binder.getCallingPid() == Process.myPid()) { - return; - } - mContext.enforceCallingPermission(android.Manifest.permission.STATSCOMPANION, null); - } - @Override public void registerPullAtomCallback(int atomTag, long coolDownNs, long timeoutNs, int[] additiveFields, IPullAtomCallback pullerCallback) { @@ -2817,6 +2785,7 @@ public class StatsCompanionService extends IStatsCompanionService.Stub { + "alive."); return; } + mStatsManagerService.statsdReady(sStatsd); if (DEBUG) Slog.d(TAG, "Saying hi to statsd"); try { sStatsd.statsCompanionReady(); @@ -2928,6 +2897,7 @@ public class StatsCompanionService extends IStatsCompanionService.Stub { if (looperStats != null) { looperStats.reset(); } + mStatsManagerService.statsdNotReady(); } @Override diff --git a/apex/statsd/service/java/com/android/server/stats/StatsManagerService.java b/apex/statsd/service/java/com/android/server/stats/StatsManagerService.java index 24b7978506d04..46fe597a7d53a 100644 --- a/apex/statsd/service/java/com/android/server/stats/StatsManagerService.java +++ b/apex/statsd/service/java/com/android/server/stats/StatsManagerService.java @@ -16,18 +16,27 @@ package com.android.server.stats; +import static com.android.server.stats.StatsCompanion.PendingIntentRef; + +import android.Manifest; +import android.app.AppOpsManager; import android.app.PendingIntent; import android.content.Context; -import android.os.IBinder; +import android.os.Binder; import android.os.IStatsManagerService; import android.os.IStatsd; +import android.os.Process; import android.os.RemoteException; -import android.os.ServiceManager; +import android.util.ArrayMap; import android.util.Slog; import com.android.internal.annotations.GuardedBy; +import java.util.Map; +import java.util.Objects; + /** + * Service for {@link android.app.StatsManager}. * @hide */ public class StatsManagerService extends IStatsManagerService.Stub { @@ -35,22 +44,99 @@ public class StatsManagerService extends IStatsManagerService.Stub { private static final String TAG = "StatsManagerService"; private static final boolean DEBUG = false; - @GuardedBy("sStatsdLock") - private static IStatsd sStatsd; - private static final Object sStatsdLock = new Object(); + private static final int STATSD_TIMEOUT_MILLIS = 5000; + + private static final String USAGE_STATS_PERMISSION_OPS = "android:get_usage_stats"; + + @GuardedBy("mLock") + private IStatsd mStatsd; + private final Object mLock = new Object(); private StatsCompanionService mStatsCompanionService; + private Context mContext; + + @GuardedBy("mLock") + private ArrayMap mDataFetchPirMap = new ArrayMap<>(); public StatsManagerService(Context context) { super(); + mContext = context; + } + + private static class ConfigKey { + private int mUid; + private long mConfigId; + + ConfigKey(int uid, long configId) { + mUid = uid; + mConfigId = configId; + } + + public int getUid() { + return mUid; + } + + public long getConfigId() { + return mConfigId; + } + + @Override + public int hashCode() { + return Objects.hash(mUid, mConfigId); + } + + @Override + public boolean equals(Object obj) { + if (obj instanceof ConfigKey) { + ConfigKey other = (ConfigKey) obj; + return this.mUid == other.getUid() && this.mConfigId == other.getConfigId(); + } + return false; + } } @Override - public void setDataFetchOperation(long configKey, PendingIntent pendingIntent, + public void setDataFetchOperation(long configId, PendingIntent pendingIntent, String packageName) { - // no-op - if (DEBUG) { - Slog.d(TAG, "setDataFetchOperation"); + enforceDumpAndUsageStatsPermission(packageName); + int callingUid = Binder.getCallingUid(); + final long token = Binder.clearCallingIdentity(); + PendingIntentRef pir = new PendingIntentRef(pendingIntent, mContext); + ConfigKey key = new ConfigKey(callingUid, configId); + // We add the PIR to a map so we can reregister if statsd is unavailable. + synchronized (mLock) { + mDataFetchPirMap.put(key, pir); + } + try { + IStatsd statsd = getStatsdNonblocking(); + if (statsd != null) { + statsd.setDataFetchOperation(configId, pir, callingUid); + } + } catch (RemoteException e) { + Slog.e(TAG, "Failed to setDataFetchOperation with statsd"); + } finally { + Binder.restoreCallingIdentity(token); + } + } + + @Override + public void removeDataFetchOperation(long configId, String packageName) { + enforceDumpAndUsageStatsPermission(packageName); + int callingUid = Binder.getCallingUid(); + final long token = Binder.clearCallingIdentity(); + ConfigKey key = new ConfigKey(callingUid, configId); + synchronized (mLock) { + mDataFetchPirMap.remove(key); + } + try { + IStatsd statsd = getStatsdNonblocking(); + if (statsd != null) { + statsd.removeDataFetchOperation(configId, callingUid); + } + } catch (RemoteException e) { + Slog.e(TAG, "Failed to removeDataFetchOperation with statsd"); + } finally { + Binder.restoreCallingIdentity(token); } } @@ -77,37 +163,100 @@ public class StatsManagerService extends IStatsManagerService.Stub { mStatsCompanionService = statsCompanionService; } - void systemReady() { - if (DEBUG) { - Slog.d(TAG, "statsdReady"); + private void enforceDumpAndUsageStatsPermission(String packageName) { + int callingUid = Binder.getCallingUid(); + int callingPid = Binder.getCallingPid(); + + if (callingPid == Process.myPid()) { + return; + } + mContext.enforceCallingPermission(Manifest.permission.DUMP, null); + mContext.enforceCallingPermission(Manifest.permission.PACKAGE_USAGE_STATS, null); + + AppOpsManager appOpsManager = (AppOpsManager) mContext + .getSystemService(Context.APP_OPS_SERVICE); + switch (appOpsManager.noteOp(USAGE_STATS_PERMISSION_OPS, + Binder.getCallingUid(), packageName, null, null)) { + case AppOpsManager.MODE_ALLOWED: + case AppOpsManager.MODE_DEFAULT: + break; + default: + throw new SecurityException( + String.format("UID %d / PID %d lacks app-op %s", + callingUid, callingPid, USAGE_STATS_PERMISSION_OPS) + ); } - setupStatsManagerService(); } - private void setupStatsManagerService() { - synchronized (sStatsdLock) { - if (sStatsd != null) { - if (DEBUG) { - Slog.e(TAG, "Trying to fetch statsd, but it was already fetched", - new IllegalStateException( - "sStatsd is not null when being fetched")); + /** + * Clients should call this if blocking until statsd to be ready is desired + * + * @return IStatsd object if statsd becomes ready within the timeout, null otherwise. + */ + private IStatsd waitForStatsd() { + synchronized (mLock) { + if (mStatsd == null) { + try { + mLock.wait(STATSD_TIMEOUT_MILLIS); + } catch (InterruptedException e) { + Slog.e(TAG, "wait for statsd interrupted"); } - return; } - sStatsd = IStatsd.Stub.asInterface(ServiceManager.getService("stats")); - if (sStatsd == null) { - if (DEBUG) { - Slog.d(TAG, "Failed to get stats service."); - } - return; - } - // Assume statsd is ready since this is called form statscompanion, link to statsd. + return mStatsd; + } + } + + /** + * Clients should call this to receive a reference to statsd. + * + * @return IStatsd object if statsd is ready, null otherwise. + */ + private IStatsd getStatsdNonblocking() { + synchronized (mLock) { + return mStatsd; + } + } + + /** + * Called from {@link StatsCompanionService}. + * + * Tells StatsManagerService that Statsd is ready and updates + * Statsd with the contents of our local cache. + */ + void statsdReady(IStatsd statsd) { + synchronized (mLock) { + mStatsd = statsd; + mLock.notify(); + } + sayHiToStatsd(statsd); + } + + /** + * Called from {@link StatsCompanionService}. + * + * Tells StatsManagerService that Statsd is no longer ready + * and we should no longer make binder calls with statsd. + */ + void statsdNotReady() { + synchronized (mLock) { + mStatsd = null; + } + } + + private void sayHiToStatsd(IStatsd statsd) { + if (statsd == null) { + return; + } + ArrayMap dataFetchCopy; + synchronized (mLock) { + dataFetchCopy = new ArrayMap<>(mDataFetchPirMap); + } + for (Map.Entry entry : dataFetchCopy.entrySet()) { + ConfigKey key = entry.getKey(); try { - sStatsd.asBinder().linkToDeath((IBinder.DeathRecipient) () -> { - sStatsd = null; - }, 0); + statsd.setDataFetchOperation(key.getConfigId(), entry.getValue(), key.getUid()); } catch (RemoteException e) { - Slog.e(TAG, "linkToDeath(StatsdDeathRecipient) failed", e); + Slog.e(TAG, "Failed to setDataFetchOperation from pirMap"); } } } diff --git a/cmds/statsd/src/StatsService.cpp b/cmds/statsd/src/StatsService.cpp index bb3a094af34a1..7fecf46c4ca5d 100644 --- a/cmds/statsd/src/StatsService.cpp +++ b/cmds/statsd/src/StatsService.cpp @@ -170,18 +170,18 @@ StatsService::StatsService(const sp& handlerLooper, shared_ptr sc = getStatsCompanionService(); - auto receiver = mConfigManager->GetConfigReceiver(key); - if (sc == nullptr) { - VLOG("Could not find StatsCompanionService"); + sp receiver = mConfigManager->GetConfigReceiver(key); + if (receiver == nullptr) { + VLOG("Could not find a broadcast receiver for %s", + key.ToString().c_str()); return false; - } else if (receiver == nullptr) { - VLOG("Statscompanion could not find a broadcast receiver for %s", - key.ToString().c_str()); - return false; - } else { - sc->sendDataBroadcast(receiver, mProcessor->getLastReportTimeNs(key)); + } else if (receiver->sendDataBroadcast( + mProcessor->getLastReportTimeNs(key)).isOk()) { return true; + } else { + VLOG("Failed to send a broadcast for receiver %s", + key.ToString().c_str()); + return false; } }, [this](const int& uid, const vector& activeConfigs) { @@ -574,18 +574,19 @@ status_t StatsService::cmd_trigger_broadcast(int out, Vector& args) { return UNKNOWN_ERROR; } ConfigKey key(uid, StrToInt64(name)); - auto receiver = mConfigManager->GetConfigReceiver(key); - sp sc = getStatsCompanionService(); - if (sc == nullptr) { - VLOG("Could not access statsCompanion"); - } else if (receiver == nullptr) { - VLOG("Could not find receiver for %s, %s", args[1].c_str(), args[2].c_str()) - } else { - sc->sendDataBroadcast(receiver, mProcessor->getLastReportTimeNs(key)); + sp receiver = mConfigManager->GetConfigReceiver(key); + if (receiver == nullptr) { + VLOG("Could not find receiver for %s, %s", args[1].c_str(), args[2].c_str()); + return UNKNOWN_ERROR; + } else if (receiver->sendDataBroadcast( + mProcessor->getLastReportTimeNs(key)).isOk()) { VLOG("StatsService::trigger broadcast succeeded to %s, %s", args[1].c_str(), args[2].c_str()); + } else { + VLOG("StatsService::trigger broadcast failed to %s, %s", args[1].c_str(), + args[2].c_str()); + return UNKNOWN_ERROR; } - return NO_ERROR; } @@ -1185,23 +1186,21 @@ bool StatsService::addConfigurationChecked(int uid, int64_t key, const vectorgetCallingUid(), key); +Status StatsService::removeDataFetchOperation(int64_t key, + const int32_t callingUid) { + ENFORCE_UID(AID_SYSTEM); + ConfigKey configKey(callingUid, key); mConfigManager->RemoveConfigReceiver(configKey); return Status::ok(); } Status StatsService::setDataFetchOperation(int64_t key, - const sp& intentSender, - const String16& packageName) { - ENFORCE_DUMP_AND_USAGE_STATS(packageName); + const sp& pir, + const int32_t callingUid) { + ENFORCE_UID(AID_SYSTEM); - IPCThreadState* ipc = IPCThreadState::self(); - ConfigKey configKey(ipc->getCallingUid(), key); - mConfigManager->SetConfigReceiver(configKey, intentSender); + ConfigKey configKey(callingUid, key); + mConfigManager->SetConfigReceiver(configKey, pir); if (StorageManager::hasConfigMetricsReport(configKey)) { VLOG("StatsService::setDataFetchOperation marking configKey %s to dump reports on disk", configKey.ToString().c_str()); diff --git a/cmds/statsd/src/StatsService.h b/cmds/statsd/src/StatsService.h index de55ca9c38cd1..9912d2fb2d387 100644 --- a/cmds/statsd/src/StatsService.h +++ b/cmds/statsd/src/StatsService.h @@ -30,6 +30,7 @@ #include #include #include +#include #include #include #include @@ -121,14 +122,14 @@ public: * Binder call to let clients register the data fetch operation for a configuration. */ virtual Status setDataFetchOperation(int64_t key, - const sp& intentSender, - const String16& packageName) override; + const sp& pir, + const int32_t callingUid) override; /** * Binder call to remove the data fetch operation for the specified config key. */ virtual Status removeDataFetchOperation(int64_t key, - const String16& packageName) override; + const int32_t callingUid) override; /** * Binder call to let clients register the active configs changed operation. diff --git a/cmds/statsd/src/config/ConfigManager.cpp b/cmds/statsd/src/config/ConfigManager.cpp index fc949b4941947..7bfb991ab1acf 100644 --- a/cmds/statsd/src/config/ConfigManager.cpp +++ b/cmds/statsd/src/config/ConfigManager.cpp @@ -46,6 +46,23 @@ using std::vector; using android::base::StringPrintf; using std::unique_ptr; +class ConfigReceiverDeathRecipient : public android::IBinder::DeathRecipient { + public: + ConfigReceiverDeathRecipient(sp configManager, const ConfigKey& configKey): + mConfigManager(configManager), + mConfigKey(configKey) {} + ~ConfigReceiverDeathRecipient() override = default; + private: + sp mConfigManager; + ConfigKey mConfigKey; + + void binderDied(const android::wp& who) override { + if (IInterface::asBinder(mConfigManager->GetConfigReceiver(mConfigKey)) == who.promote()) { + mConfigManager->RemoveConfigReceiver(mConfigKey); + } + } +}; + ConfigManager::ConfigManager() { } @@ -118,9 +135,11 @@ void ConfigManager::UpdateConfig(const ConfigKey& key, const StatsdConfig& confi } } -void ConfigManager::SetConfigReceiver(const ConfigKey& key, const sp& intentSender) { +void ConfigManager::SetConfigReceiver(const ConfigKey& key, + const sp& pir) { lock_guard lock(mMutex); - mConfigReceivers[key] = intentSender; + mConfigReceivers[key] = pir; + IInterface::asBinder(pir)->linkToDeath(new ConfigReceiverDeathRecipient(this, key)); } void ConfigManager::RemoveConfigReceiver(const ConfigKey& key) { @@ -266,7 +285,7 @@ vector ConfigManager::GetAllConfigKeys() const { return ret; } -const sp ConfigManager::GetConfigReceiver(const ConfigKey& key) const { +const sp ConfigManager::GetConfigReceiver(const ConfigKey& key) const { lock_guard lock(mMutex); auto it = mConfigReceivers.find(key); diff --git a/cmds/statsd/src/config/ConfigManager.h b/cmds/statsd/src/config/ConfigManager.h index c064a519f597b..1aeb355b1f0a9 100644 --- a/cmds/statsd/src/config/ConfigManager.h +++ b/cmds/statsd/src/config/ConfigManager.h @@ -20,6 +20,7 @@ #include "config/ConfigKey.h" #include "config/ConfigListener.h" +#include #include #include #include @@ -64,12 +65,12 @@ public: /** * Sets the broadcast receiver for a configuration key. */ - void SetConfigReceiver(const ConfigKey& key, const sp& intentSender); + void SetConfigReceiver(const ConfigKey& key, const sp& pir); /** * Returns the package name and class name representing the broadcast receiver for this config. */ - const sp GetConfigReceiver(const ConfigKey& key) const; + const sp GetConfigReceiver(const ConfigKey& key) const; /** * Returns all config keys registered. @@ -141,10 +142,9 @@ private: std::map> mConfigs; /** - * Each config key can be subscribed by up to one receiver, specified as IBinder from - * PendingIntent. + * Each config key can be subscribed by up to one receiver, specified as IPendingIntentRef. */ - std::map> mConfigReceivers; + std::map> mConfigReceivers; /** * Each uid can be subscribed by up to one receiver to notify that the list of active configs diff --git a/core/java/android/app/StatsManager.java b/core/java/android/app/StatsManager.java index b9893aaa66a48..a458a55962d88 100644 --- a/core/java/android/app/StatsManager.java +++ b/core/java/android/app/StatsManager.java @@ -309,18 +309,16 @@ public final class StatsManager { throws StatsUnavailableException { synchronized (sLock) { try { - IStatsd service = getIStatsdLocked(); + IStatsManagerService service = getIStatsManagerServiceLocked(); if (pendingIntent == null) { service.removeDataFetchOperation(configKey, mContext.getOpPackageName()); } else { - // Extracts IIntentSender from the PendingIntent and turns it into an IBinder. - IBinder intentSender = pendingIntent.getTarget().asBinder(); - service.setDataFetchOperation(configKey, intentSender, + service.setDataFetchOperation(configKey, pendingIntent, mContext.getOpPackageName()); } } catch (RemoteException e) { - Slog.e(TAG, "Failed to connect to statsd when registering data listener."); + Slog.e(TAG, "Failed to connect to statsmanager when registering data listener."); throw new StatsUnavailableException("could not connect", e); } catch (SecurityException e) { throw new StatsUnavailableException(e.getMessage(), e); From 47537a1c58833ceb0e48c2c4a33d109e362fae2f Mon Sep 17 00:00:00 2001 From: Jeffrey Huang Date: Mon, 6 Jan 2020 15:35:34 -0800 Subject: [PATCH 2/3] Update activeConfigsChangedBroadcast avoid using intentsender in #sendActiveConfigsChangedBroadcast and #removeActiveConfigsChangedBroadcast. Bug: 146074295 Test: Ran GTS Tests Change-Id: I9313299ea0bc89f092b1c62fbfc34e06a127eaa9 --- .../android/os/IStatsCompanionService.aidl | 6 -- .../aidl/android/os/IStatsManagerService.aidl | 7 +++ apex/statsd/aidl/android/os/IStatsd.aidl | 4 +- .../android/server/stats/StatsCompanion.java | 17 +++++- .../server/stats/StatsCompanionService.java | 17 ------ .../server/stats/StatsManagerService.java | 56 +++++++++++++++++-- cmds/statsd/src/StatsService.cpp | 48 ++++++++-------- cmds/statsd/src/StatsService.h | 6 +- cmds/statsd/src/config/ConfigManager.cpp | 26 ++++++++- cmds/statsd/src/config/ConfigManager.h | 9 ++- core/java/android/app/StatsManager.java | 10 ++-- 11 files changed, 133 insertions(+), 73 deletions(-) diff --git a/apex/statsd/aidl/android/os/IStatsCompanionService.aidl b/apex/statsd/aidl/android/os/IStatsCompanionService.aidl index ec3a226ab5420..32413e2f1703b 100644 --- a/apex/statsd/aidl/android/os/IStatsCompanionService.aidl +++ b/apex/statsd/aidl/android/os/IStatsCompanionService.aidl @@ -66,12 +66,6 @@ interface IStatsCompanionService { /** Pull the specified data. Results will be sent to statsd when complete. */ StatsLogEventWrapper[] pullData(int pullCode); - /** - * Send a broadcast to the specified PendingIntent's as IBinder notifying it that the list - * of active configs has changed. - */ - oneway void sendActiveConfigsChangedBroadcast(in IBinder intentSender, in long[] configIds); - /** * Requests StatsCompanionService to send a broadcast using the given intentSender * (which should cast to an IIntentSender), along with the other information specified. diff --git a/apex/statsd/aidl/android/os/IStatsManagerService.aidl b/apex/statsd/aidl/android/os/IStatsManagerService.aidl index 151cdbd8770cb..291140cb875c3 100644 --- a/apex/statsd/aidl/android/os/IStatsManagerService.aidl +++ b/apex/statsd/aidl/android/os/IStatsManagerService.aidl @@ -52,6 +52,13 @@ interface IStatsManagerService { */ long[] setActiveConfigsChangedOperation(in PendingIntent pendingIntent, in String packageName); + /** + * Removes the active configs changed operation for the specified package name. + * + * Requires Manifest.permission.DUMP and Manifest.permission.PACKAGE_USAGE_STATS. + */ + void removeActiveConfigsChangedOperation(in String packageName); + /** * Set the PendingIntent to be used when broadcasting subscriber * information to the given subscriberId within the given config. diff --git a/apex/statsd/aidl/android/os/IStatsd.aidl b/apex/statsd/aidl/android/os/IStatsd.aidl index 986d483301a42..323d56fa5484c 100644 --- a/apex/statsd/aidl/android/os/IStatsd.aidl +++ b/apex/statsd/aidl/android/os/IStatsd.aidl @@ -133,14 +133,14 @@ interface IStatsd { * * Requires Manifest.permission.DUMP and Manifest.permission.PACKAGE_USAGE_STATS. */ - long[] setActiveConfigsChangedOperation(in IBinder intentSender, in String packageName); + long[] setActiveConfigsChangedOperation(in IPendingIntentRef pendingIntentRef, int callingUid); /** * Removes the active configs changed operation for the specified package name. * * Requires Manifest.permission.DUMP and Manifest.permission.PACKAGE_USAGE_STATS. */ - void removeActiveConfigsChangedOperation(in String packageName); + void removeActiveConfigsChangedOperation(int callingUid); /** * Removes the configuration with the matching config key. No-op if this config key does not diff --git a/apex/statsd/service/java/com/android/server/stats/StatsCompanion.java b/apex/statsd/service/java/com/android/server/stats/StatsCompanion.java index 6a0bf629cf5c9..77ccfb6d7a299 100644 --- a/apex/statsd/service/java/com/android/server/stats/StatsCompanion.java +++ b/apex/statsd/service/java/com/android/server/stats/StatsCompanion.java @@ -17,6 +17,7 @@ package com.android.server.stats; import android.app.PendingIntent; +import android.app.StatsManager; import android.content.Context; import android.content.Intent; import android.os.Binder; @@ -27,6 +28,8 @@ import android.util.Slog; import com.android.server.SystemService; +import java.util.Arrays; + /** * @hide */ @@ -97,6 +100,8 @@ public class StatsCompanion { */ private static final String EXTRA_LAST_REPORT_TIME = "android.app.extra.LAST_REPORT_TIME"; private static final int CODE_DATA_BROADCAST = 1; + private static final int CODE_ACTIVE_CONFIGS_BROADCAST = 1; + private final PendingIntent mPendingIntent; private final Context mContext; @@ -120,7 +125,17 @@ public class StatsCompanion { @Override public void sendActiveConfigsChangedBroadcast(long[] configIds) { - // no-op + enforceStatsCompanionPermission(mContext); + Intent intent = new Intent(); + intent.putExtra(StatsManager.EXTRA_STATS_ACTIVE_CONFIG_KEYS, configIds); + try { + mPendingIntent.send(mContext, CODE_ACTIVE_CONFIGS_BROADCAST, intent, null, null); + if (DEBUG) { + Slog.d(TAG, "Sent broadcast with config ids " + Arrays.toString(configIds)); + } + } catch (PendingIntent.CanceledException e) { + Slog.w(TAG, "Unable to send active configs changed broadcast using PendingIntent"); + } } @Override diff --git a/apex/statsd/service/java/com/android/server/stats/StatsCompanionService.java b/apex/statsd/service/java/com/android/server/stats/StatsCompanionService.java index d160f22d583f8..befce218dc563 100644 --- a/apex/statsd/service/java/com/android/server/stats/StatsCompanionService.java +++ b/apex/statsd/service/java/com/android/server/stats/StatsCompanionService.java @@ -203,7 +203,6 @@ public class StatsCompanionService extends IStatsCompanionService.Stub { private static final int INSTALLER_FIELD_ID = 5; public static final int CODE_SUBSCRIBER_BROADCAST = 1; - public static final int CODE_ACTIVE_CONFIGS_BROADCAST = 1; public static final int DEATH_THRESHOLD = 10; /** * Which native processes to snapshot memory for. @@ -443,22 +442,6 @@ public class StatsCompanionService extends IStatsCompanionService.Stub { KernelCpuThreadReaderSettingsObserver.getSettingsModifiedReader(mContext); } - @Override - public void sendActiveConfigsChangedBroadcast(IBinder intentSenderBinder, long[] configIds) { - StatsCompanion.enforceStatsCompanionPermission(mContext); - IntentSender intentSender = new IntentSender(intentSenderBinder); - Intent intent = new Intent(); - intent.putExtra(StatsManager.EXTRA_STATS_ACTIVE_CONFIG_KEYS, configIds); - try { - intentSender.sendIntent(mContext, CODE_ACTIVE_CONFIGS_BROADCAST, intent, null, null); - if (DEBUG) { - Slog.d(TAG, "Sent broadcast with config ids " + Arrays.toString(configIds)); - } - } catch (IntentSender.SendIntentException e) { - Slog.w(TAG, "Unable to send active configs changed broadcast using IntentSender"); - } - } - @Override public void sendSubscriberBroadcast(IBinder intentSenderBinder, long configUid, long configKey, long subscriptionId, long subscriptionRuleId, String[] cookies, diff --git a/apex/statsd/service/java/com/android/server/stats/StatsManagerService.java b/apex/statsd/service/java/com/android/server/stats/StatsManagerService.java index 46fe597a7d53a..ef2b761a940e1 100644 --- a/apex/statsd/service/java/com/android/server/stats/StatsManagerService.java +++ b/apex/statsd/service/java/com/android/server/stats/StatsManagerService.java @@ -37,6 +37,7 @@ import java.util.Objects; /** * Service for {@link android.app.StatsManager}. + * * @hide */ public class StatsManagerService extends IStatsManagerService.Stub { @@ -57,6 +58,9 @@ public class StatsManagerService extends IStatsManagerService.Stub { @GuardedBy("mLock") private ArrayMap mDataFetchPirMap = new ArrayMap<>(); + @GuardedBy("mLock") + private ArrayMap mActiveConfigsPirMap = + new ArrayMap<>(); public StatsManagerService(Context context) { super(); @@ -143,11 +147,45 @@ public class StatsManagerService extends IStatsManagerService.Stub { @Override public long[] setActiveConfigsChangedOperation(PendingIntent pendingIntent, String packageName) { - // no-op - if (DEBUG) { - Slog.d(TAG, "setActiveConfigsChangedOperation"); + enforceDumpAndUsageStatsPermission(packageName); + int callingUid = Binder.getCallingUid(); + final long token = Binder.clearCallingIdentity(); + PendingIntentRef pir = new PendingIntentRef(pendingIntent, mContext); + // We add the PIR to a map so we can reregister if statsd is unavailable. + synchronized (mLock) { + mActiveConfigsPirMap.put(callingUid, pir); + } + try { + IStatsd statsd = getStatsdNonblocking(); + if (statsd != null) { + return statsd.setActiveConfigsChangedOperation(pir, callingUid); + } + } catch (RemoteException e) { + Slog.e(TAG, "Failed to setActiveConfigsChangedOperation with statsd"); + } finally { + Binder.restoreCallingIdentity(token); + } + return new long[] {}; + } + + @Override + public void removeActiveConfigsChangedOperation(String packageName) { + enforceDumpAndUsageStatsPermission(packageName); + int callingUid = Binder.getCallingUid(); + final long token = Binder.clearCallingIdentity(); + synchronized (mLock) { + mActiveConfigsPirMap.remove(callingUid); + } + try { + IStatsd statsd = getStatsdNonblocking(); + if (statsd != null) { + statsd.removeActiveConfigsChangedOperation(callingUid); + } + } catch (RemoteException e) { + Slog.e(TAG, "Failed to removeActiveConfigsChangedOperation with statsd"); + } finally { + Binder.restoreCallingIdentity(token); } - return new long[]{}; } @Override @@ -248,8 +286,10 @@ public class StatsManagerService extends IStatsManagerService.Stub { return; } ArrayMap dataFetchCopy; + ArrayMap activeConfigsChangedCopy; synchronized (mLock) { dataFetchCopy = new ArrayMap<>(mDataFetchPirMap); + activeConfigsChangedCopy = new ArrayMap<>(mActiveConfigsPirMap); } for (Map.Entry entry : dataFetchCopy.entrySet()) { ConfigKey key = entry.getKey(); @@ -259,5 +299,13 @@ public class StatsManagerService extends IStatsManagerService.Stub { Slog.e(TAG, "Failed to setDataFetchOperation from pirMap"); } } + for (Map.Entry entry + : activeConfigsChangedCopy.entrySet()) { + try { + statsd.setActiveConfigsChangedOperation(entry.getValue(), entry.getKey()); + } catch (RemoteException e) { + Slog.e(TAG, "Failed to setActiveConfigsChangedOperation from pirMap"); + } + } } } diff --git a/cmds/statsd/src/StatsService.cpp b/cmds/statsd/src/StatsService.cpp index 7fecf46c4ca5d..3a29b7271bbf6 100644 --- a/cmds/statsd/src/StatsService.cpp +++ b/cmds/statsd/src/StatsService.cpp @@ -185,18 +185,17 @@ StatsService::StatsService(const sp& handlerLooper, shared_ptr& activeConfigs) { - auto receiver = mConfigManager->GetActiveConfigsChangedReceiver(uid); - sp sc = getStatsCompanionService(); - if (sc == nullptr) { - VLOG("Could not access statsCompanion"); - return false; - } else if (receiver == nullptr) { + sp receiver = + mConfigManager->GetActiveConfigsChangedReceiver(uid); + if (receiver == nullptr) { VLOG("Could not find receiver for uid %d", uid); return false; - } else { - sc->sendActiveConfigsChangedBroadcast(receiver, activeConfigs); + } else if (receiver->sendActiveConfigsChangedBroadcast(activeConfigs).isOk()) { VLOG("StatsService::active configs broadcast succeeded for uid %d" , uid); return true; + } else { + VLOG("StatsService::active configs broadcast failed for uid %d" , uid); + return false; } }); @@ -630,15 +629,15 @@ status_t StatsService::cmd_trigger_active_config_broadcast(int out, VectorGetActiveConfigsChangedReceiver(uid); - sp sc = getStatsCompanionService(); - if (sc == nullptr) { - VLOG("Could not access statsCompanion"); - } else if (receiver == nullptr) { + sp receiver = mConfigManager->GetActiveConfigsChangedReceiver(uid); + if (receiver == nullptr) { VLOG("Could not find receiver for uid %d", uid); - } else { - sc->sendActiveConfigsChangedBroadcast(receiver, configIds); + return UNKNOWN_ERROR; + } else if (receiver->sendActiveConfigsChangedBroadcast(configIds).isOk()) { VLOG("StatsService::trigger active configs changed broadcast succeeded for uid %d" , uid); + } else { + VLOG("StatsService::trigger active configs changed broadcast failed for uid %d", uid); + return UNKNOWN_ERROR; } return NO_ERROR; } @@ -1209,27 +1208,24 @@ Status StatsService::setDataFetchOperation(int64_t key, return Status::ok(); } -Status StatsService::setActiveConfigsChangedOperation(const sp& intentSender, - const String16& packageName, +Status StatsService::setActiveConfigsChangedOperation(const sp& pir, + const int32_t callingUid, vector* output) { - ENFORCE_DUMP_AND_USAGE_STATS(packageName); + ENFORCE_UID(AID_SYSTEM); - IPCThreadState* ipc = IPCThreadState::self(); - int uid = ipc->getCallingUid(); - mConfigManager->SetActiveConfigsChangedReceiver(uid, intentSender); + mConfigManager->SetActiveConfigsChangedReceiver(callingUid, pir); if (output != nullptr) { - mProcessor->GetActiveConfigs(uid, *output); + mProcessor->GetActiveConfigs(callingUid, *output); } else { ALOGW("StatsService::setActiveConfigsChanged output was nullptr"); } return Status::ok(); } -Status StatsService::removeActiveConfigsChangedOperation(const String16& packageName) { - ENFORCE_DUMP_AND_USAGE_STATS(packageName); +Status StatsService::removeActiveConfigsChangedOperation(const int32_t callingUid) { + ENFORCE_UID(AID_SYSTEM); - IPCThreadState* ipc = IPCThreadState::self(); - mConfigManager->RemoveActiveConfigsChangedReceiver(ipc->getCallingUid()); + mConfigManager->RemoveActiveConfigsChangedReceiver(callingUid); return Status::ok(); } diff --git a/cmds/statsd/src/StatsService.h b/cmds/statsd/src/StatsService.h index 9912d2fb2d387..0e89a2b129e02 100644 --- a/cmds/statsd/src/StatsService.h +++ b/cmds/statsd/src/StatsService.h @@ -134,14 +134,14 @@ public: /** * Binder call to let clients register the active configs changed operation. */ - virtual Status setActiveConfigsChangedOperation(const sp& intentSender, - const String16& packageName, + virtual Status setActiveConfigsChangedOperation(const sp& pir, + const int32_t callingUid, vector* output) override; /** * Binder call to remove the active configs changed operation for the specified package.. */ - virtual Status removeActiveConfigsChangedOperation(const String16& packageName) override; + virtual Status removeActiveConfigsChangedOperation(const int32_t callingUid) override; /** * Binder call to allow clients to remove the specified configuration. */ diff --git a/cmds/statsd/src/config/ConfigManager.cpp b/cmds/statsd/src/config/ConfigManager.cpp index 7bfb991ab1acf..55d73c1ae3420 100644 --- a/cmds/statsd/src/config/ConfigManager.cpp +++ b/cmds/statsd/src/config/ConfigManager.cpp @@ -63,6 +63,24 @@ class ConfigReceiverDeathRecipient : public android::IBinder::DeathRecipient { } }; +class ActiveConfigChangedReceiverDeathRecipient : public android::IBinder::DeathRecipient { + public: + ActiveConfigChangedReceiverDeathRecipient(sp configManager, const int uid): + mConfigManager(configManager), + mUid(uid) {} + ~ActiveConfigChangedReceiverDeathRecipient() override = default; + private: + sp mConfigManager; + int mUid; + + void binderDied(const android::wp& who) override { + if (IInterface::asBinder(mConfigManager->GetActiveConfigsChangedReceiver(mUid)) + == who.promote()) { + mConfigManager->RemoveActiveConfigsChangedReceiver(mUid); + } + } +}; + ConfigManager::ConfigManager() { } @@ -148,9 +166,11 @@ void ConfigManager::RemoveConfigReceiver(const ConfigKey& key) { } void ConfigManager::SetActiveConfigsChangedReceiver(const int uid, - const sp& intentSender) { + const sp& pir) { lock_guard lock(mMutex); - mActiveConfigsChangedReceivers[uid] = intentSender; + mActiveConfigsChangedReceivers[uid] = pir; + IInterface::asBinder(pir)->linkToDeath( + new ActiveConfigChangedReceiverDeathRecipient(this, uid)); } void ConfigManager::RemoveActiveConfigsChangedReceiver(const int uid) { @@ -296,7 +316,7 @@ const sp ConfigManager::GetConfigReceiver(const ConfigKey& ke } } -const sp ConfigManager::GetActiveConfigsChangedReceiver(const int uid) const { +const sp ConfigManager::GetActiveConfigsChangedReceiver(const int uid) const { lock_guard lock(mMutex); auto it = mActiveConfigsChangedReceivers.find(uid); diff --git a/cmds/statsd/src/config/ConfigManager.h b/cmds/statsd/src/config/ConfigManager.h index 1aeb355b1f0a9..88e864a2520b0 100644 --- a/cmds/statsd/src/config/ConfigManager.h +++ b/cmds/statsd/src/config/ConfigManager.h @@ -16,7 +16,6 @@ #pragma once -#include "binder/IBinder.h" #include "config/ConfigKey.h" #include "config/ConfigListener.h" @@ -86,13 +85,13 @@ public: * Sets the broadcast receiver that is notified whenever the list of active configs * changes for this uid. */ - void SetActiveConfigsChangedReceiver(const int uid, const sp& intentSender); + void SetActiveConfigsChangedReceiver(const int uid, const sp& pir); /** * Returns the broadcast receiver for active configs changed for this uid. */ - const sp GetActiveConfigsChangedReceiver(const int uid) const; + const sp GetActiveConfigsChangedReceiver(const int uid) const; /** * Erase any active configs changed broadcast receiver associated with this uid. @@ -148,9 +147,9 @@ private: /** * Each uid can be subscribed by up to one receiver to notify that the list of active configs - * for this uid has changed. The receiver is specified as IBinder from PendingIntent. + * for this uid has changed. The receiver is specified as IPendingIntentRef. */ - std::map> mActiveConfigsChangedReceivers; + std::map> mActiveConfigsChangedReceivers; /** * The ConfigListeners that will be told about changes. diff --git a/core/java/android/app/StatsManager.java b/core/java/android/app/StatsManager.java index a458a55962d88..51b2d40894245 100644 --- a/core/java/android/app/StatsManager.java +++ b/core/java/android/app/StatsManager.java @@ -345,20 +345,18 @@ public final class StatsManager { throws StatsUnavailableException { synchronized (sLock) { try { - IStatsd service = getIStatsdLocked(); + IStatsManagerService service = getIStatsManagerServiceLocked(); if (pendingIntent == null) { service.removeActiveConfigsChangedOperation(mContext.getOpPackageName()); return new long[0]; } else { - // Extracts IIntentSender from the PendingIntent and turns it into an IBinder. - IBinder intentSender = pendingIntent.getTarget().asBinder(); - return service.setActiveConfigsChangedOperation(intentSender, + return service.setActiveConfigsChangedOperation(pendingIntent, mContext.getOpPackageName()); } } catch (RemoteException e) { - Slog.e(TAG, - "Failed to connect to statsd when registering active configs listener."); + Slog.e(TAG, "Failed to connect to statsmanager " + + "when registering active configs listener."); throw new StatsUnavailableException("could not connect", e); } catch (SecurityException e) { throw new StatsUnavailableException(e.getMessage(), e); From 4f2e6bd68d573ea9fd75d9584ecb5df8b0b4032a Mon Sep 17 00:00:00 2001 From: Jeffrey Huang Date: Mon, 6 Jan 2020 16:24:45 -0800 Subject: [PATCH 3/3] Update setBroadcastSubscriber change #setBroadcastSubscriber and #unsetBroadcastSubscriber to avoid using intentsender Bug: 146074295 Test: Ran GTS Tests Change-Id: I1510e44bcdf49b579fd49f51081c6a40618039fa --- .../android/os/IStatsCompanionService.aidl | 10 --- .../aidl/android/os/IStatsManagerService.aidl | 11 ++- apex/statsd/aidl/android/os/IStatsd.aidl | 23 +++--- .../android/server/stats/StatsCompanion.java | 34 ++++++++- .../server/stats/StatsCompanionService.java | 40 ---------- .../server/stats/StatsManagerService.java | 76 ++++++++++++++++++- cmds/statsd/src/StatsService.cpp | 20 ++--- cmds/statsd/src/StatsService.h | 11 ++- .../src/subscriber/SubscriberReporter.cpp | 48 +++++++++--- .../src/subscriber/SubscriberReporter.h | 23 ++---- core/java/android/app/StatsManager.java | 9 +-- 11 files changed, 186 insertions(+), 119 deletions(-) diff --git a/apex/statsd/aidl/android/os/IStatsCompanionService.aidl b/apex/statsd/aidl/android/os/IStatsCompanionService.aidl index 32413e2f1703b..21b7767e932d7 100644 --- a/apex/statsd/aidl/android/os/IStatsCompanionService.aidl +++ b/apex/statsd/aidl/android/os/IStatsCompanionService.aidl @@ -17,7 +17,6 @@ package android.os; import android.os.IPullAtomCallback; -import android.os.StatsDimensionsValue; import android.os.StatsLogEventWrapper; /** @@ -66,15 +65,6 @@ interface IStatsCompanionService { /** Pull the specified data. Results will be sent to statsd when complete. */ StatsLogEventWrapper[] pullData(int pullCode); - /** - * Requests StatsCompanionService to send a broadcast using the given intentSender - * (which should cast to an IIntentSender), along with the other information specified. - */ - oneway void sendSubscriberBroadcast(in IBinder intentSender, long configUid, long configId, - long subscriptionId, long subscriptionRuleId, - in String[] cookies, - in StatsDimensionsValue dimensionsValue); - /** Tells StatsCompaionService to grab the uid map snapshot and send it to statsd. */ oneway void triggerUidSnapshot(); diff --git a/apex/statsd/aidl/android/os/IStatsManagerService.aidl b/apex/statsd/aidl/android/os/IStatsManagerService.aidl index 291140cb875c3..0558367776787 100644 --- a/apex/statsd/aidl/android/os/IStatsManagerService.aidl +++ b/apex/statsd/aidl/android/os/IStatsManagerService.aidl @@ -72,8 +72,17 @@ interface IStatsManagerService { * This function can only be called by the owner (uid) of the config. It must be called each * time statsd starts. Later calls overwrite previous calls; only one PendingIntent is stored. * - * Requires Manifest.permission.DUMP. + * Requires Manifest.permission.DUMP and Manifest.permission.PACKAGE_USAGE_STATS. */ void setBroadcastSubscriber(long configKey, long subscriberId, in PendingIntent pendingIntent, in String packageName); + + /** + * Undoes setBroadcastSubscriber() for the (configKey, subscriberId) pair. + * Any broadcasts associated with subscriberId will henceforth not be sent. + * No-op if this (configKey, subscriberId) pair was not associated with an PendingIntent. + * + * Requires Manifest.permission.DUMP and Manifest.permission.PACKAGE_USAGE_STATS. + */ + void unsetBroadcastSubscriber(long configKey, long subscriberId, in String packageName); } \ No newline at end of file diff --git a/apex/statsd/aidl/android/os/IStatsd.aidl b/apex/statsd/aidl/android/os/IStatsd.aidl index 323d56fa5484c..935843921febd 100644 --- a/apex/statsd/aidl/android/os/IStatsd.aidl +++ b/apex/statsd/aidl/android/os/IStatsd.aidl @@ -151,34 +151,31 @@ interface IStatsd { void removeConfiguration(in long configKey, in String packageName); /** - * Set the IIntentSender (i.e. PendingIntent) to be used when broadcasting subscriber + * Set the PendingIntentRef to be used when broadcasting subscriber * information to the given subscriberId within the given config. * - * Suppose that the calling uid has added a config with key configKey, and that in this config + * Suppose that the calling uid has added a config with key configId, and that in this config * it is specified that when a particular anomaly is detected, a broadcast should be sent to - * a BroadcastSubscriber with id subscriberId. This function links the given intentSender with - * that subscriberId (for that config), so that this intentSender is used to send the broadcast + * a BroadcastSubscriber with id subscriberId. This function links the given pendingIntent with + * that subscriberId (for that config), so that this pendingIntent is used to send the broadcast * when the anomaly is detected. * * This function can only be called by the owner (uid) of the config. It must be called each - * time statsd starts. Later calls overwrite previous calls; only one intentSender is stored. - * - * intentSender must be convertible into an IntentSender using IntentSender(IBinder) - * and cannot be null. + * time statsd starts. Later calls overwrite previous calls; only one pendingIntent is stored. * * Requires Manifest.permission.DUMP. */ - void setBroadcastSubscriber(long configKey, long subscriberId, in IBinder intentSender, - in String packageName); + void setBroadcastSubscriber(long configId, long subscriberId, in IPendingIntentRef pir, + int callingUid); /** - * Undoes setBroadcastSubscriber() for the (configKey, subscriberId) pair. + * Undoes setBroadcastSubscriber() for the (configId, subscriberId) pair. * Any broadcasts associated with subscriberId will henceforth not be sent. - * No-op if this (configKey, subsriberId) pair was not associated with an IntentSender. + * No-op if this (configKey, subscriberId) pair was not associated with an PendingIntentRef. * * Requires Manifest.permission.DUMP. */ - void unsetBroadcastSubscriber(long configKey, long subscriberId, in String packageName); + void unsetBroadcastSubscriber(long configId, long subscriberId, int callingUid); /** * Apps can send an atom via this application breadcrumb with the specified label and state for diff --git a/apex/statsd/service/java/com/android/server/stats/StatsCompanion.java b/apex/statsd/service/java/com/android/server/stats/StatsCompanion.java index 77ccfb6d7a299..4383b503bfe77 100644 --- a/apex/statsd/service/java/com/android/server/stats/StatsCompanion.java +++ b/apex/statsd/service/java/com/android/server/stats/StatsCompanion.java @@ -28,6 +28,7 @@ import android.util.Slog; import com.android.server.SystemService; +import java.util.ArrayList; import java.util.Arrays; /** @@ -101,7 +102,7 @@ public class StatsCompanion { private static final String EXTRA_LAST_REPORT_TIME = "android.app.extra.LAST_REPORT_TIME"; private static final int CODE_DATA_BROADCAST = 1; private static final int CODE_ACTIVE_CONFIGS_BROADCAST = 1; - + private static final int CODE_SUBSCRIBER_BROADCAST = 1; private final PendingIntent mPendingIntent; private final Context mContext; @@ -141,7 +142,36 @@ public class StatsCompanion { @Override public void sendSubscriberBroadcast(long configUid, long configId, long subscriptionId, long subscriptionRuleId, String[] cookies, StatsDimensionsValue dimensionsValue) { - // no-op + enforceStatsCompanionPermission(mContext); + Intent intent = + new Intent() + .putExtra(StatsManager.EXTRA_STATS_CONFIG_UID, configUid) + .putExtra(StatsManager.EXTRA_STATS_CONFIG_KEY, configId) + .putExtra(StatsManager.EXTRA_STATS_SUBSCRIPTION_ID, subscriptionId) + .putExtra(StatsManager.EXTRA_STATS_SUBSCRIPTION_RULE_ID, + subscriptionRuleId) + .putExtra(StatsManager.EXTRA_STATS_DIMENSIONS_VALUE, dimensionsValue); + + ArrayList cookieList = new ArrayList<>(cookies.length); + cookieList.addAll(Arrays.asList(cookies)); + intent.putStringArrayListExtra( + StatsManager.EXTRA_STATS_BROADCAST_SUBSCRIBER_COOKIES, cookieList); + + if (DEBUG) { + Slog.d(TAG, + String.format( + "Statsd sendSubscriberBroadcast with params {%d %d %d %d %s %s}", + configUid, configId, subscriptionId, subscriptionRuleId, + Arrays.toString(cookies), + dimensionsValue)); + } + try { + mPendingIntent.send(mContext, CODE_SUBSCRIBER_BROADCAST, intent, null, null); + } catch (PendingIntent.CanceledException e) { + Slog.w(TAG, + "Unable to send using PendingIntent from uid " + configUid + + "; presumably it had been cancelled."); + } } } } diff --git a/apex/statsd/service/java/com/android/server/stats/StatsCompanionService.java b/apex/statsd/service/java/com/android/server/stats/StatsCompanionService.java index befce218dc563..d57afeeb71574 100644 --- a/apex/statsd/service/java/com/android/server/stats/StatsCompanionService.java +++ b/apex/statsd/service/java/com/android/server/stats/StatsCompanionService.java @@ -50,7 +50,6 @@ import android.content.BroadcastReceiver; import android.content.Context; import android.content.Intent; import android.content.IntentFilter; -import android.content.IntentSender; import android.content.pm.ApplicationInfo; import android.content.pm.PackageInfo; import android.content.pm.PackageManager; @@ -88,7 +87,6 @@ import android.os.Parcelable; import android.os.RemoteException; import android.os.ServiceManager; import android.os.StatFs; -import android.os.StatsDimensionsValue; import android.os.StatsLogEventWrapper; import android.os.SynchronousResultReceiver; import android.os.SystemClock; @@ -202,7 +200,6 @@ public class StatsCompanionService extends IStatsCompanionService.Stub { private static final int PACKAGE_NAME_FIELD_ID = 4; private static final int INSTALLER_FIELD_ID = 5; - public static final int CODE_SUBSCRIBER_BROADCAST = 1; public static final int DEATH_THRESHOLD = 10; /** * Which native processes to snapshot memory for. @@ -442,43 +439,6 @@ public class StatsCompanionService extends IStatsCompanionService.Stub { KernelCpuThreadReaderSettingsObserver.getSettingsModifiedReader(mContext); } - @Override - public void sendSubscriberBroadcast(IBinder intentSenderBinder, long configUid, long configKey, - long subscriptionId, long subscriptionRuleId, String[] cookies, - StatsDimensionsValue dimensionsValue) { - StatsCompanion.enforceStatsCompanionPermission(mContext); - IntentSender intentSender = new IntentSender(intentSenderBinder); - Intent intent = - new Intent() - .putExtra(StatsManager.EXTRA_STATS_CONFIG_UID, configUid) - .putExtra(StatsManager.EXTRA_STATS_CONFIG_KEY, configKey) - .putExtra(StatsManager.EXTRA_STATS_SUBSCRIPTION_ID, subscriptionId) - .putExtra(StatsManager.EXTRA_STATS_SUBSCRIPTION_RULE_ID, subscriptionRuleId) - .putExtra(StatsManager.EXTRA_STATS_DIMENSIONS_VALUE, dimensionsValue); - - ArrayList cookieList = new ArrayList<>(cookies.length); - for (String cookie : cookies) { - cookieList.add(cookie); - } - intent.putStringArrayListExtra( - StatsManager.EXTRA_STATS_BROADCAST_SUBSCRIBER_COOKIES, cookieList); - - if (DEBUG) { - Slog.d(TAG, - String.format("Statsd sendSubscriberBroadcast with params {%d %d %d %d %s %s}", - configUid, configKey, subscriptionId, subscriptionRuleId, - Arrays.toString(cookies), - dimensionsValue)); - } - try { - intentSender.sendIntent(mContext, CODE_SUBSCRIBER_BROADCAST, intent, null, null); - } catch (IntentSender.SendIntentException e) { - Slog.w(TAG, - "Unable to send using IntentSender from uid " + configUid - + "; presumably it had been cancelled."); - } - } - private final static int[] toIntArray(List list) { int[] ret = new int[list.size()]; for (int i = 0; i < ret.length; i++) { diff --git a/apex/statsd/service/java/com/android/server/stats/StatsManagerService.java b/apex/statsd/service/java/com/android/server/stats/StatsManagerService.java index ef2b761a940e1..1d03e3b702c43 100644 --- a/apex/statsd/service/java/com/android/server/stats/StatsManagerService.java +++ b/apex/statsd/service/java/com/android/server/stats/StatsManagerService.java @@ -61,6 +61,9 @@ public class StatsManagerService extends IStatsManagerService.Stub { @GuardedBy("mLock") private ArrayMap mActiveConfigsPirMap = new ArrayMap<>(); + @GuardedBy("mLock") + private ArrayMap> mBroadcastSubscriberPirMap = + new ArrayMap<>(); public StatsManagerService(Context context) { super(); @@ -189,11 +192,56 @@ public class StatsManagerService extends IStatsManagerService.Stub { } @Override - public void setBroadcastSubscriber(long configKey, long subscriberId, + public void setBroadcastSubscriber(long configId, long subscriberId, PendingIntent pendingIntent, String packageName) { - //no-op - if (DEBUG) { - Slog.d(TAG, "setBroadcastSubscriber"); + enforceDumpAndUsageStatsPermission(packageName); + int callingUid = Binder.getCallingUid(); + final long token = Binder.clearCallingIdentity(); + PendingIntentRef pir = new PendingIntentRef(pendingIntent, mContext); + ConfigKey key = new ConfigKey(callingUid, configId); + // We add the PIR to a map so we can reregister if statsd is unavailable. + synchronized (mLock) { + ArrayMap innerMap = mBroadcastSubscriberPirMap + .getOrDefault(key, new ArrayMap<>()); + innerMap.put(subscriberId, pir); + mBroadcastSubscriberPirMap.put(key, innerMap); + } + try { + IStatsd statsd = getStatsdNonblocking(); + if (statsd != null) { + statsd.setBroadcastSubscriber( + configId, subscriberId, pir, callingUid); + } + } catch (RemoteException e) { + Slog.e(TAG, "Failed to setBroadcastSubscriber with statsd"); + } finally { + Binder.restoreCallingIdentity(token); + } + } + + @Override + public void unsetBroadcastSubscriber(long configId, long subscriberId, String packageName) { + enforceDumpAndUsageStatsPermission(packageName); + int callingUid = Binder.getCallingUid(); + final long token = Binder.clearCallingIdentity(); + ConfigKey key = new ConfigKey(callingUid, configId); + synchronized (mLock) { + ArrayMap innerMap = mBroadcastSubscriberPirMap + .getOrDefault(key, new ArrayMap<>()); + innerMap.remove(subscriberId); + if (innerMap.isEmpty()) { + mBroadcastSubscriberPirMap.remove(key); + } + } + try { + IStatsd statsd = getStatsdNonblocking(); + if (statsd != null) { + statsd.unsetBroadcastSubscriber(configId, subscriberId, callingUid); + } + } catch (RemoteException e) { + Slog.e(TAG, "Failed to unsetBroadcastSubscriber with statsd"); + } finally { + Binder.restoreCallingIdentity(token); } } @@ -285,11 +333,19 @@ public class StatsManagerService extends IStatsManagerService.Stub { if (statsd == null) { return; } + // Since we do not want to make an IPC with the a lock held, we first create local deep + // copies of the data with the lock held before iterating through the maps. ArrayMap dataFetchCopy; ArrayMap activeConfigsChangedCopy; + ArrayMap> broadcastSubscriberCopy; synchronized (mLock) { dataFetchCopy = new ArrayMap<>(mDataFetchPirMap); activeConfigsChangedCopy = new ArrayMap<>(mActiveConfigsPirMap); + broadcastSubscriberCopy = new ArrayMap<>(); + for (Map.Entry> entry + : mBroadcastSubscriberPirMap.entrySet()) { + broadcastSubscriberCopy.put(entry.getKey(), new ArrayMap<>(entry.getValue())); + } } for (Map.Entry entry : dataFetchCopy.entrySet()) { ConfigKey key = entry.getKey(); @@ -307,5 +363,17 @@ public class StatsManagerService extends IStatsManagerService.Stub { Slog.e(TAG, "Failed to setActiveConfigsChangedOperation from pirMap"); } } + for (Map.Entry> entry + : broadcastSubscriberCopy.entrySet()) { + for (Map.Entry subscriberEntry : entry.getValue().entrySet()) { + ConfigKey configKey = entry.getKey(); + try { + statsd.setBroadcastSubscriber(configKey.getConfigId(), subscriberEntry.getKey(), + subscriberEntry.getValue(), configKey.getUid()); + } catch (RemoteException e) { + Slog.e(TAG, "Failed to setBroadcastSubscriber from pirMap"); + } + } + } } } diff --git a/cmds/statsd/src/StatsService.cpp b/cmds/statsd/src/StatsService.cpp index 3a29b7271bbf6..5ff0f97865b8f 100644 --- a/cmds/statsd/src/StatsService.cpp +++ b/cmds/statsd/src/StatsService.cpp @@ -1111,7 +1111,6 @@ Status StatsService::statsCompanionReady() { mPullerManager->SetStatsCompanionService(statsCompanion); mAnomalyAlarmMonitor->setStatsCompanionService(statsCompanion); mPeriodicAlarmMonitor->setStatsCompanionService(statsCompanion); - SubscriberReporter::getInstance().setStatsCompanionService(statsCompanion); return Status::ok(); } @@ -1241,26 +1240,24 @@ Status StatsService::removeConfiguration(int64_t key, const String16& packageNam Status StatsService::setBroadcastSubscriber(int64_t configId, int64_t subscriberId, - const sp& intentSender, - const String16& packageName) { - ENFORCE_DUMP_AND_USAGE_STATS(packageName); + const sp& pir, + const int32_t callingUid) { + ENFORCE_UID(AID_SYSTEM); VLOG("StatsService::setBroadcastSubscriber called."); - IPCThreadState* ipc = IPCThreadState::self(); - ConfigKey configKey(ipc->getCallingUid(), configId); + ConfigKey configKey(callingUid, configId); SubscriberReporter::getInstance() - .setBroadcastSubscriber(configKey, subscriberId, intentSender); + .setBroadcastSubscriber(configKey, subscriberId, pir); return Status::ok(); } Status StatsService::unsetBroadcastSubscriber(int64_t configId, int64_t subscriberId, - const String16& packageName) { - ENFORCE_DUMP_AND_USAGE_STATS(packageName); + const int32_t callingUid) { + ENFORCE_UID(AID_SYSTEM); VLOG("StatsService::unsetBroadcastSubscriber called."); - IPCThreadState* ipc = IPCThreadState::self(); - ConfigKey configKey(ipc->getCallingUid(), configId); + ConfigKey configKey(callingUid, configId); SubscriberReporter::getInstance() .unsetBroadcastSubscriber(configKey, subscriberId); return Status::ok(); @@ -1626,7 +1623,6 @@ void StatsService::binderDied(const wp & who) { } mAnomalyAlarmMonitor->setStatsCompanionService(nullptr); mPeriodicAlarmMonitor->setStatsCompanionService(nullptr); - SubscriberReporter::getInstance().setStatsCompanionService(nullptr); mPullerManager->SetStatsCompanionService(nullptr); } diff --git a/cmds/statsd/src/StatsService.h b/cmds/statsd/src/StatsService.h index 0e89a2b129e02..0565f3ce24843 100644 --- a/cmds/statsd/src/StatsService.h +++ b/cmds/statsd/src/StatsService.h @@ -149,20 +149,19 @@ public: const String16& packageName) override; /** - * Binder call to associate the given config's subscriberId with the given intentSender. - * intentSender must be convertible into an IntentSender (in Java) using IntentSender(IBinder). + * Binder call to associate the given config's subscriberId with the given pendingIntentRef. */ virtual Status setBroadcastSubscriber(int64_t configId, int64_t subscriberId, - const sp& intentSender, - const String16& packageName) override; + const sp& pir, + const int32_t callingUid) override; /** - * Binder call to unassociate the given config's subscriberId with any intentSender. + * Binder call to unassociate the given config's subscriberId with any pendingIntentRef. */ virtual Status unsetBroadcastSubscriber(int64_t configId, int64_t subscriberId, - const String16& packageName) override; + const int32_t callingUid) override; /** Inform statsCompanion that statsd is ready. */ virtual void sayHiToStatsCompanion(); diff --git a/cmds/statsd/src/subscriber/SubscriberReporter.cpp b/cmds/statsd/src/subscriber/SubscriberReporter.cpp index 25d2257c752b7..a9a105f0fda70 100644 --- a/cmds/statsd/src/subscriber/SubscriberReporter.cpp +++ b/cmds/statsd/src/subscriber/SubscriberReporter.cpp @@ -19,7 +19,6 @@ #include "SubscriberReporter.h" -using android::IBinder; using std::lock_guard; using std::unordered_map; @@ -29,12 +28,32 @@ namespace statsd { using std::vector; +class BroadcastSubscriberDeathRecipient : public android::IBinder::DeathRecipient { + public: + BroadcastSubscriberDeathRecipient(const ConfigKey& configKey, int64_t subscriberId): + mConfigKey(configKey), + mSubscriberId(subscriberId) {} + ~BroadcastSubscriberDeathRecipient() override = default; + private: + ConfigKey mConfigKey; + int64_t mSubscriberId; + + void binderDied(const android::wp& who) override { + if (IInterface::asBinder(SubscriberReporter::getInstance().getBroadcastSubscriber( + mConfigKey, mSubscriberId)) == who.promote()) { + SubscriberReporter::getInstance().unsetBroadcastSubscriber(mConfigKey, mSubscriberId); + } + } +}; + void SubscriberReporter::setBroadcastSubscriber(const ConfigKey& configKey, int64_t subscriberId, - const sp& intentSender) { + const sp& pir) { VLOG("SubscriberReporter::setBroadcastSubscriber called."); lock_guard lock(mLock); - mIntentMap[configKey][subscriberId] = intentSender; + mIntentMap[configKey][subscriberId] = pir; + IInterface::asBinder(pir)->linkToDeath( + new BroadcastSubscriberDeathRecipient(configKey, subscriberId)); } void SubscriberReporter::unsetBroadcastSubscriber(const ConfigKey& configKey, @@ -97,18 +116,13 @@ void SubscriberReporter::alertBroadcastSubscriber(const ConfigKey& configKey, sendBroadcastLocked(it2->second, configKey, subscription, cookies, dimKey); } -void SubscriberReporter::sendBroadcastLocked(const sp& intentSender, +void SubscriberReporter::sendBroadcastLocked(const sp& pir, const ConfigKey& configKey, const Subscription& subscription, const vector& cookies, const MetricDimensionKey& dimKey) const { VLOG("SubscriberReporter::sendBroadcastLocked called."); - if (mStatsCompanionService == nullptr) { - ALOGW("Failed to send subscriber broadcast: could not access StatsCompanionService."); - return; - } - mStatsCompanionService->sendSubscriberBroadcast( - intentSender, + pir->sendSubscriberBroadcast( configKey.GetUid(), configKey.GetId(), subscription.id(), @@ -117,6 +131,20 @@ void SubscriberReporter::sendBroadcastLocked(const sp& intentSender, getStatsDimensionsValue(dimKey.getDimensionKeyInWhat())); } +sp SubscriberReporter::getBroadcastSubscriber(const ConfigKey& configKey, + int64_t subscriberId) { + lock_guard lock(mLock); + auto subscriberMapIt = mIntentMap.find(configKey); + if (subscriberMapIt == mIntentMap.end()) { + return nullptr; + } + auto pirMapIt = subscriberMapIt->second.find(subscriberId); + if (pirMapIt == subscriberMapIt->second.end()) { + return nullptr; + } + return pirMapIt->second; +} + void getStatsDimensionsValueHelper(const vector& dims, size_t* index, int depth, int prefix, vector* output) { size_t count = dims.size(); diff --git a/cmds/statsd/src/subscriber/SubscriberReporter.h b/cmds/statsd/src/subscriber/SubscriberReporter.h index 2a7f771a0ba4c..8ccc8ee626d4d 100644 --- a/cmds/statsd/src/subscriber/SubscriberReporter.h +++ b/cmds/statsd/src/subscriber/SubscriberReporter.h @@ -16,6 +16,7 @@ #pragma once +#include #include #include @@ -46,24 +47,12 @@ public: SubscriberReporter(SubscriberReporter const&) = delete; void operator=(SubscriberReporter const&) = delete; - /** - * Tells SubscriberReporter what IStatsCompanionService to use. - * May be nullptr, but SubscriberReporter will not send broadcasts for any calls - * to alertBroadcastSubscriber that occur while nullptr. - */ - void setStatsCompanionService(sp statsCompanionService) { - std::lock_guard lock(mLock); - sp tmpForLock = mStatsCompanionService; - mStatsCompanionService = statsCompanionService; - } - /** * Stores the given intentSender, associating it with the given (configKey, subscriberId) pair. - * intentSender must be convertible into an IntentSender (in Java) using IntentSender(IBinder). */ void setBroadcastSubscriber(const ConfigKey& configKey, int64_t subscriberId, - const sp& intentSender); + const sp& pir); /** * Erases any intentSender information from the given (configKey, subscriberId) pair. @@ -82,6 +71,8 @@ public: const Subscription& subscription, const MetricDimensionKey& dimKey) const; + sp getBroadcastSubscriber(const ConfigKey& configKey, int64_t subscriberId); + static StatsDimensionsValue getStatsDimensionsValue(const HashableDimensionKey& dim); private: @@ -92,15 +83,15 @@ private: /** Binder interface for communicating with StatsCompanionService. */ sp mStatsCompanionService = nullptr; - /** Maps -> IBinder (which represents an IIntentSender). */ + /** Maps -> IPendingIntentRef (which represents a PendingIntent). */ std::unordered_map>> mIntentMap; + std::unordered_map>> mIntentMap; /** * Sends a broadcast via the given intentSender (using mStatsCompanionService), along * with the information in the other parameters. */ - void sendBroadcastLocked(const sp& intentSender, + void sendBroadcastLocked(const sp& pir, const ConfigKey& configKey, const Subscription& subscription, const std::vector& cookies, diff --git a/core/java/android/app/StatsManager.java b/core/java/android/app/StatsManager.java index 51b2d40894245..83d1de60cac7a 100644 --- a/core/java/android/app/StatsManager.java +++ b/core/java/android/app/StatsManager.java @@ -255,18 +255,17 @@ public final class StatsManager { throws StatsUnavailableException { synchronized (sLock) { try { - IStatsd service = getIStatsdLocked(); + IStatsManagerService service = getIStatsManagerServiceLocked(); if (pendingIntent != null) { - // Extracts IIntentSender from the PendingIntent and turns it into an IBinder. - IBinder intentSender = pendingIntent.getTarget().asBinder(); - service.setBroadcastSubscriber(configKey, subscriberId, intentSender, + service.setBroadcastSubscriber(configKey, subscriberId, pendingIntent, mContext.getOpPackageName()); } else { service.unsetBroadcastSubscriber(configKey, subscriberId, mContext.getOpPackageName()); } } catch (RemoteException e) { - Slog.e(TAG, "Failed to connect to statsd when adding broadcast subscriber", e); + Slog.e(TAG, "Failed to connect to statsmanager when adding broadcast subscriber", + e); throw new StatsUnavailableException("could not connect", e); } catch (SecurityException e) { throw new StatsUnavailableException(e.getMessage(), e);