From c5f09f687b452157e486d352340cdd6cc6780abd Mon Sep 17 00:00:00 2001 From: Stanislav Zholnin Date: Tue, 27 Apr 2021 18:31:30 +0100 Subject: [PATCH] Bug fixes and testing support for DiscreteRegistry Fixed following bugs: - in certain cases deduplication could fail - while requesting discrete ops, beginTimeMillis parameter was ignored - device_config parameters above guardrails were still applied after reboot Added support tests (coming in next CL): - ability to offsetHistory for test - ability to change quantization parameter to any value for tests Test: coming in next CL Bug: 183238944 Bug: 183239281 Bug: 176965672 Change-Id: I838a7897058b38aa0957a3666d4b37a5378e36be --- core/java/android/app/AppOpsManager.java | 23 +++-- .../server/appop/DiscreteRegistry.java | 95 ++++++++++++++----- .../server/appop/HistoricalRegistry.java | 6 ++ 3 files changed, 96 insertions(+), 28 deletions(-) diff --git a/core/java/android/app/AppOpsManager.java b/core/java/android/app/AppOpsManager.java index 1cb46b1b63501..d7986201c02ba 100644 --- a/core/java/android/app/AppOpsManager.java +++ b/core/java/android/app/AppOpsManager.java @@ -6511,13 +6511,21 @@ public class AppOpsManager { NoteOpEvent note = new NoteOpEvent(discreteAccessTime, discreteAccessDuration, null); accessEvents.append(key, note); AttributedOpEntry access = new AttributedOpEntry(mOp, false, accessEvents, null); - for (int i = discreteAccesses.size() - 1; i >= 0; i--) { - if (discreteAccesses.get(i).getLastAccessTime(OP_FLAGS_ALL) < discreteAccessTime) { - discreteAccesses.add(i + 1, access); - return; + int insertionPoint = discreteAccesses.size() - 1; + for (; insertionPoint >= 0; insertionPoint--) { + if (discreteAccesses.get(insertionPoint).getLastAccessTime(OP_FLAGS_ALL) + < discreteAccessTime) { + break; } } - discreteAccesses.add(0, access); + insertionPoint++; + if (insertionPoint < discreteAccesses.size() && discreteAccesses.get( + insertionPoint).getLastAccessTime(OP_FLAGS_ALL) == discreteAccessTime) { + discreteAccesses.set(insertionPoint, mergeAttributedOpEntries( + Arrays.asList(discreteAccesses.get(insertionPoint), access))); + } else { + discreteAccesses.add(insertionPoint, access); + } } /** @@ -9858,7 +9866,10 @@ public class AppOpsManager { NoteOpEvent reject = a.getLastRejectEvent(uidState, uidState, flags); if (access != null) { - accessEvents.append(key, access); + NoteOpEvent existingAccess = accessEvents.get(key); + if (existingAccess == null || existingAccess.getDuration() == -1) { + accessEvents.append(key, access); + } } if (reject != null) { rejectEvents.append(key, reject); diff --git a/services/core/java/com/android/server/appop/DiscreteRegistry.java b/services/core/java/com/android/server/appop/DiscreteRegistry.java index f2e422d3ee7cb..63b41b7892809 100644 --- a/services/core/java/com/android/server/appop/DiscreteRegistry.java +++ b/services/core/java/com/android/server/appop/DiscreteRegistry.java @@ -92,8 +92,8 @@ final class DiscreteRegistry { "discrete_history_quantization_millis"; private static final String PROPERTY_DISCRETE_FLAGS = "discrete_history_op_flags"; private static final String PROPERTY_DISCRETE_OPS_LIST = "discrete_history_ops_cslist"; - private static final String DEFAULT_DISCRETE_OPS = OP_CAMERA + "," + OP_RECORD_AUDIO + "," - + OP_FINE_LOCATION + "," + OP_COARSE_LOCATION; + private static final String DEFAULT_DISCRETE_OPS = OP_FINE_LOCATION + "," + OP_COARSE_LOCATION + + "," + OP_CAMERA + "," + OP_RECORD_AUDIO; private static final long DEFAULT_DISCRETE_HISTORY_CUTOFF = Duration.ofHours(24).toMillis(); private static final long MAXIMUM_DISCRETE_HISTORY_CUTOFF = Duration.ofDays(30).toMillis(); private static final long DEFAULT_DISCRETE_HISTORY_QUANTIZATION = @@ -104,7 +104,6 @@ final class DiscreteRegistry { private static int[] sDiscreteOps; private static int sDiscreteFlags; - private static final String TAG_HISTORY = "h"; private static final String ATTR_VERSION = "v"; private static final int CURRENT_VERSION = 1; @@ -144,6 +143,8 @@ final class DiscreteRegistry { @GuardedBy("mOnDiskLock") private DiscreteOps mCachedOps = null; + private boolean mDebugMode = false; + DiscreteRegistry(Object inMemoryLock) { mInMemoryLock = inMemoryLock; } @@ -159,40 +160,35 @@ final class DiscreteRegistry { AsyncTask.THREAD_POOL_EXECUTOR, (DeviceConfig.Properties p) -> { setDiscreteHistoryParameters(p); }); - sDiscreteHistoryCutoff = DeviceConfig.getLong(DeviceConfig.NAMESPACE_PRIVACY, - PROPERTY_DISCRETE_HISTORY_CUTOFF, DEFAULT_DISCRETE_HISTORY_CUTOFF); - sDiscreteHistoryQuantization = DeviceConfig.getLong(DeviceConfig.NAMESPACE_PRIVACY, - PROPERTY_DISCRETE_HISTORY_QUANTIZATION, DEFAULT_DISCRETE_HISTORY_QUANTIZATION); - sDiscreteFlags = DeviceConfig.getInt(DeviceConfig.NAMESPACE_PRIVACY, - PROPERTY_DISCRETE_FLAGS, OP_FLAGS_DISCRETE); - sDiscreteOps = parseOpsList(DeviceConfig.getString(DeviceConfig.NAMESPACE_PRIVACY, - PROPERTY_DISCRETE_OPS_LIST, DEFAULT_DISCRETE_OPS)); + setDiscreteHistoryParameters(DeviceConfig.getProperties(DeviceConfig.NAMESPACE_PRIVACY)); } private void setDiscreteHistoryParameters(DeviceConfig.Properties p) { if (p.getKeyset().contains(PROPERTY_DISCRETE_HISTORY_CUTOFF)) { sDiscreteHistoryCutoff = p.getLong(PROPERTY_DISCRETE_HISTORY_CUTOFF, DEFAULT_DISCRETE_HISTORY_CUTOFF); - if (!Build.IS_DEBUGGABLE) { + if (!Build.IS_DEBUGGABLE && !mDebugMode) { sDiscreteHistoryCutoff = min(MAXIMUM_DISCRETE_HISTORY_CUTOFF, sDiscreteHistoryCutoff); } + } else { + sDiscreteHistoryCutoff = DEFAULT_DISCRETE_HISTORY_CUTOFF; } if (p.getKeyset().contains(PROPERTY_DISCRETE_HISTORY_QUANTIZATION)) { sDiscreteHistoryQuantization = p.getLong(PROPERTY_DISCRETE_HISTORY_QUANTIZATION, DEFAULT_DISCRETE_HISTORY_QUANTIZATION); - if (!Build.IS_DEBUGGABLE) { + if (!Build.IS_DEBUGGABLE && !mDebugMode) { sDiscreteHistoryQuantization = max(DEFAULT_DISCRETE_HISTORY_QUANTIZATION, sDiscreteHistoryQuantization); } + } else { + sDiscreteHistoryQuantization = DEFAULT_DISCRETE_HISTORY_QUANTIZATION; } - if (p.getKeyset().contains(PROPERTY_DISCRETE_FLAGS)) { - sDiscreteFlags = p.getInt(PROPERTY_DISCRETE_FLAGS, OP_FLAGS_DISCRETE); - } - if (p.getKeyset().contains(PROPERTY_DISCRETE_OPS_LIST)) { - sDiscreteOps = parseOpsList(p.getString(PROPERTY_DISCRETE_OPS_LIST, - DEFAULT_DISCRETE_OPS)); - } + sDiscreteFlags = p.getKeyset().contains(PROPERTY_DISCRETE_FLAGS) ? sDiscreteFlags = + p.getInt(PROPERTY_DISCRETE_FLAGS, OP_FLAGS_DISCRETE) : OP_FLAGS_DISCRETE; + sDiscreteOps = p.getKeyset().contains(PROPERTY_DISCRETE_OPS_LIST) ? parseOpsList( + p.getString(PROPERTY_DISCRETE_OPS_LIST, DEFAULT_DISCRETE_OPS)) : parseOpsList( + DEFAULT_DISCRETE_OPS); } void recordDiscreteAccess(int uid, String packageName, int op, @Nullable String attributionTag, @@ -232,6 +228,8 @@ final class DiscreteRegistry { @Nullable String packageNameFilter, @Nullable String[] opNamesFilter, @Nullable String attributionTagFilter, @AppOpsManager.OpFlags int flagsFilter) { DiscreteOps discreteOps = getAllDiscreteOps(); + beginTimeMillis = max(beginTimeMillis, Instant.now().minus(sDiscreteHistoryCutoff, + ChronoUnit.MILLIS).toEpochMilli()); discreteOps.filter(beginTimeMillis, endTimeMillis, filter, uidFilter, packageNameFilter, opNamesFilter, attributionTagFilter, flagsFilter); discreteOps.applyToHistoricalOps(result); @@ -282,6 +280,18 @@ final class DiscreteRegistry { } } + void offsetHistory(long offset) { + synchronized (mOnDiskLock) { + DiscreteOps discreteOps; + synchronized (mInMemoryLock) { + discreteOps = getAllDiscreteOps(); + clearHistory(); + } + discreteOps.offsetHistory(offset); + persistDiscreteOpsLocked(discreteOps); + } + } + void dump(@NonNull PrintWriter pw, int uidFilter, @Nullable String packageNameFilter, @Nullable String attributionTagFilter, @AppOpsManager.HistoricalOpsRequestFilter int filter, int dumpOp, @@ -363,6 +373,13 @@ final class DiscreteRegistry { } } + private void offsetHistory(long offset) { + int nUids = mUids.size(); + for (int i = 0; i < nUids; i++) { + mUids.valueAt(i).offsetHistory(offset); + } + } + private void clearHistory(int uid, String packageName) { if (mUids.containsKey(uid)) { mUids.get(uid).clearPackage(packageName); @@ -549,6 +566,13 @@ final class DiscreteRegistry { } } + private void offsetHistory(long offset) { + int nPackages = mPackages.size(); + for (int i = 0; i < nPackages; i++) { + mPackages.valueAt(i).offsetHistory(offset); + } + } + private void clearPackage(String packageName) { mPackages.remove(packageName); } @@ -656,6 +680,13 @@ final class DiscreteRegistry { } } + private void offsetHistory(long offset) { + int nOps = mPackageOps.size(); + for (int i = 0; i < nOps; i++) { + mPackageOps.valueAt(i).offsetHistory(offset); + } + } + private DiscreteOp getOrCreateDiscreteOp(int op) { DiscreteOp result = mPackageOps.get(op); if (result == null) { @@ -749,12 +780,29 @@ final class DiscreteRegistry { } } + private void offsetHistory(long offset) { + int nTags = mAttributedOps.size(); + for (int i = 0; i < nTags; i++) { + List list = mAttributedOps.valueAt(i); + + int n = list.size(); + for (int j = 0; j < n; j++) { + DiscreteOpEvent event = list.get(j); + list.set(j, new DiscreteOpEvent(event.mNoteTime - offset, event.mNoteDuration, + event.mUidState, event.mOpFlag)); + } + } + } + void addDiscreteAccess(@Nullable String attributionTag, @AppOpsManager.OpFlags int flags, @AppOpsManager.UidState int uidState, long accessTime, long accessDuration) { List attributedOps = getOrCreateDiscreteOpEventsList( attributionTag); accessTime = accessTime / sDiscreteHistoryQuantization * sDiscreteHistoryQuantization; + accessDuration = accessDuration == -1 ? -1 + : (accessDuration + sDiscreteHistoryQuantization - 1) + / sDiscreteHistoryQuantization * sDiscreteHistoryQuantization; int nAttributedOps = attributedOps.size(); int i = nAttributedOps; @@ -764,8 +812,7 @@ final class DiscreteRegistry { break; } if (previousOp.mOpFlag == flags && previousOp.mUidState == uidState) { - if (accessDuration != previousOp.mNoteDuration - && accessDuration > sDiscreteHistoryQuantization) { + if (accessDuration != previousOp.mNoteDuration) { break; } else { return; @@ -983,5 +1030,9 @@ final class DiscreteRegistry { } return true; } + + void setDebugMode(boolean debugMode) { + this.mDebugMode = debugMode; + } } diff --git a/services/core/java/com/android/server/appop/HistoricalRegistry.java b/services/core/java/com/android/server/appop/HistoricalRegistry.java index ffd24587618f7..72e582e594b4e 100644 --- a/services/core/java/com/android/server/appop/HistoricalRegistry.java +++ b/services/core/java/com/android/server/appop/HistoricalRegistry.java @@ -553,6 +553,11 @@ final class HistoricalRegistry { if (mMode == AppOpsManager.HISTORICAL_MODE_DISABLED) { clearHistoryOnDiskDLocked(); } + if (mMode == AppOpsManager.HISTORICAL_MODE_ENABLED_PASSIVE) { + mDiscreteRegistry.setDebugMode(true); + } else { + mDiscreteRegistry.setDebugMode(false); + } } if (mBaseSnapshotInterval != baseSnapshotInterval) { mBaseSnapshotInterval = baseSnapshotInterval; @@ -591,6 +596,7 @@ final class HistoricalRegistry { mPersistence.persistHistoricalOpsDLocked(history); } } + mDiscreteRegistry.offsetHistory(offsetMillis); } void addHistoricalOps(HistoricalOps ops) {