From 185ddf644b4200325a869711fe7e3144e80b3980 Mon Sep 17 00:00:00 2001 From: Svet Ganov Date: Thu, 4 Jun 2020 21:40:26 -0700 Subject: [PATCH] Correctly offset app op history on reboot The offset was computed based off the offset represented by the most recent historical file while we have to use the time this file was written. Also now we persist the history on reboot and shutdown, significantly minimizing the possibility of data loss. Added test API to emulate reboot of the history to allow for precise and tightly controlled test to prevent flakes due to boot time deviations. Test: atest android.app.appops.cts.HistoricalAppopsTest#testRebootHistory Fixes:156853195 Change-Id: I4142371f8bc2b1d710cc8c300e7e79cb03764c04 --- api/test-current.txt | 1 + core/java/android/app/AppOpsManager.java | 19 +++++++++ .../android/internal/app/IAppOpsService.aidl | 1 + .../android/server/appop/AppOpsService.java | 23 ++++++++++- .../server/appop/HistoricalRegistry.java | 39 ++++++++++++++----- 5 files changed, 72 insertions(+), 11 deletions(-) diff --git a/api/test-current.txt b/api/test-current.txt index 66b5015902f0b..1142fb6318917 100644 --- a/api/test-current.txt +++ b/api/test-current.txt @@ -193,6 +193,7 @@ package android.app { method public static int opToDefaultMode(@NonNull String); method public static String opToPermission(int); method public static int permissionToOpCode(String); + method @RequiresPermission("android.permission.MANAGE_APPOPS") public void rebootHistory(long); method @RequiresPermission("android.permission.MANAGE_APPOPS") public void reloadNonHistoricalState(); method @RequiresPermission("android.permission.MANAGE_APPOPS") public void resetHistoryParameters(); method @RequiresPermission("android.permission.MANAGE_APPOPS") public void setHistoryParameters(int, long, int); diff --git a/core/java/android/app/AppOpsManager.java b/core/java/android/app/AppOpsManager.java index b058dcd714dd6..50bb9a38f8e8e 100644 --- a/core/java/android/app/AppOpsManager.java +++ b/core/java/android/app/AppOpsManager.java @@ -8576,6 +8576,25 @@ public class AppOpsManager { } } + /** + * Reboots the ops history. + * + * @param offlineDurationMillis The duration to wait between + * tearing down and initializing the history. Must be greater + * than or equal to zero. + * + * @hide + */ + @TestApi + @RequiresPermission(Manifest.permission.MANAGE_APPOPS) + public void rebootHistory(long offlineDurationMillis) { + try { + mService.rebootHistory(offlineDurationMillis); + } catch (RemoteException e) { + throw e.rethrowFromSystemServer(); + } + } + /** * Pulls current AppOps access report and picks package and op to watch for next access report * Returns null if no reports were collected since last call. There is no guarantee of report diff --git a/core/java/com/android/internal/app/IAppOpsService.aidl b/core/java/com/android/internal/app/IAppOpsService.aidl index 06c21ab8832d7..e614a0eb996b9 100644 --- a/core/java/com/android/internal/app/IAppOpsService.aidl +++ b/core/java/com/android/internal/app/IAppOpsService.aidl @@ -76,6 +76,7 @@ interface IAppOpsService { void addHistoricalOps(in AppOpsManager.HistoricalOps ops); void resetHistoryParameters(); void clearHistory(); + void rebootHistory(long offlineDurationMillis); List getUidOps(int uid, in int[] ops); void setUidMode(int code, int uid, int mode); @UnsupportedAppUsage diff --git a/services/core/java/com/android/server/appop/AppOpsService.java b/services/core/java/com/android/server/appop/AppOpsService.java index a33ad7dc0ea12..a2b2ed3ce6d31 100644 --- a/services/core/java/com/android/server/appop/AppOpsService.java +++ b/services/core/java/com/android/server/appop/AppOpsService.java @@ -322,7 +322,7 @@ public class AppOpsService extends IAppOpsService.Stub { @VisibleForTesting final SparseArray mUidStates = new SparseArray<>(); - final HistoricalRegistry mHistoricalRegistry = new HistoricalRegistry(this); + volatile @NonNull HistoricalRegistry mHistoricalRegistry = new HistoricalRegistry(this); long mLastRealtime; @@ -1978,6 +1978,8 @@ public class AppOpsService extends IAppOpsService.Stub { if (AppOpsManager.NOTE_OP_COLLECTION_ENABLED && mWriteNoteOpsScheduled) { writeNoteOps(); } + + mHistoricalRegistry.shutdown(); } private ArrayList collectOps(Ops pkgOps, int[] ops) { @@ -5881,6 +5883,25 @@ public class AppOpsService extends IAppOpsService.Stub { mHistoricalRegistry.clearHistory(); } + @Override + public void rebootHistory(long offlineDurationMillis) { + mContext.enforceCallingOrSelfPermission(android.Manifest.permission.MANAGE_APPOPS, + "rebootHistory"); + + Preconditions.checkArgument(offlineDurationMillis >= 0); + + // Must not hold the appops lock + mHistoricalRegistry.shutdown(); + + if (offlineDurationMillis > 0) { + SystemClock.sleep(offlineDurationMillis); + } + + mHistoricalRegistry = new HistoricalRegistry(mHistoricalRegistry); + mHistoricalRegistry.systemReady(mContext.getContentResolver()); + mHistoricalRegistry.persistPendingHistory(); + } + /** * Report runtime access to AppOp together with message (including stack trace) * diff --git a/services/core/java/com/android/server/appop/HistoricalRegistry.java b/services/core/java/com/android/server/appop/HistoricalRegistry.java index ed4506902f0ab..3d22a156b91c6 100644 --- a/services/core/java/com/android/server/appop/HistoricalRegistry.java +++ b/services/core/java/com/android/server/appop/HistoricalRegistry.java @@ -199,6 +199,13 @@ final class HistoricalRegistry { mInMemoryLock = lock; } + HistoricalRegistry(@NonNull HistoricalRegistry other) { + this(other.mInMemoryLock); + mMode = other.mMode; + mBaseSnapshotInterval = other.mBaseSnapshotInterval; + mIntervalCompressionMultiplier = other.mIntervalCompressionMultiplier; + } + void systemReady(@NonNull ContentResolver resolver) { final Uri uri = Settings.Global.getUriFor(Settings.Global.APPOP_HISTORY_PARAMETERS); resolver.registerContentObserver(uri, false, new ContentObserver( @@ -223,9 +230,16 @@ final class HistoricalRegistry { // When starting always adjust history to now. final long lastPersistTimeMills = mPersistence.getLastPersistTimeMillisDLocked(); + if (lastPersistTimeMills > 0) { - mPendingHistoryOffsetMillis = - System.currentTimeMillis() - lastPersistTimeMills; + mPendingHistoryOffsetMillis = System.currentTimeMillis() + - lastPersistTimeMills; + + if (DEBUG) { + Slog.i(LOG_TAG, "Time since last write: " + + TimeUtils.formatDuration(mPendingHistoryOffsetMillis) + + " by which to push history on next write"); + } } } } @@ -597,6 +611,9 @@ final class HistoricalRegistry { return; } clearHistoryOnDiskDLocked(); + mNextPersistDueTimeMillis = 0; + mPendingHistoryOffsetMillis = 0; + mCurrentHistoricalOps = null; } } } @@ -650,7 +667,15 @@ final class HistoricalRegistry { return mCurrentHistoricalOps; } - private void persistPendingHistory() { + void shutdown() { + synchronized (mInMemoryLock) { + if (mMode != AppOpsManager.HISTORICAL_MODE_DISABLED) { + persistPendingHistory(); + } + } + } + + void persistPendingHistory() { final List pendingWrites; synchronized (mOnDiskLock) { synchronized (mInMemoryLock) { @@ -844,13 +869,7 @@ final class HistoricalRegistry { if (shortestFile == null) { return 0; } - final String shortestNameNoExtension = shortestFile.getName() - .replace(HISTORY_FILE_SUFFIX, ""); - try { - return Long.parseLong(shortestNameNoExtension); - } catch (NumberFormatException e) { - return 0; - } + return shortestFile.lastModified(); } sHistoricalAppOpsDir.finishRead(); } catch (Throwable e) {