diff options
3 files changed, 159 insertions, 48 deletions
diff --git a/services/core/java/com/android/server/appop/AppOpsService.java b/services/core/java/com/android/server/appop/AppOpsService.java index 0a7684553c08..7569363a7134 100644 --- a/services/core/java/com/android/server/appop/AppOpsService.java +++ b/services/core/java/com/android/server/appop/AppOpsService.java @@ -201,7 +201,7 @@ public class AppOpsService extends IAppOpsService.Stub { @VisibleForTesting final SparseArray<UidState> mUidStates = new SparseArray<>(); - private final HistoricalRegistry mHistoricalRegistry = new HistoricalRegistry(this); + final HistoricalRegistry mHistoricalRegistry = new HistoricalRegistry(this); long mLastRealtime; diff --git a/services/core/java/com/android/server/appop/HistoricalRegistry.java b/services/core/java/com/android/server/appop/HistoricalRegistry.java index 69a1c9f584cb..9cf342c0e4fb 100644 --- a/services/core/java/com/android/server/appop/HistoricalRegistry.java +++ b/services/core/java/com/android/server/appop/HistoricalRegistry.java @@ -108,6 +108,12 @@ import java.util.concurrent.TimeUnit; * must be called with the mInMemoryLock, xxxDMLocked suffix means the method * must be called with the mOnDiskLock and mInMemoryLock locks acquired in that * exact order. + * <p> + * INITIALIZATION: We can initialize persistence only after the system is ready + * as we need to check the optional configuration override from the settings + * database which is not initialized at the time the app ops service is created. + * This means that all entry points that touch persistence should be short + * circuited via isPersistenceInitialized() check. */ // TODO (bug:122218838): Make sure we handle start of epoch time // TODO (bug:122218838): Validate changed time is handled correctly @@ -177,14 +183,33 @@ final class HistoricalRegistry { // Object managing persistence (read/write) @GuardedBy("mOnDiskLock") - private Persistence mPersistence = new Persistence(mBaseSnapshotInterval, - mIntervalCompressionMultiplier); + private Persistence mPersistence; HistoricalRegistry(@NonNull Object lock) { mInMemoryLock = lock; - if (mMode != AppOpsManager.HISTORICAL_MODE_DISABLED) { - synchronized (mOnDiskLock) { - synchronized (mInMemoryLock) { + } + + void systemReady(@NonNull ContentResolver resolver) { + final Uri uri = Settings.Global.getUriFor(Settings.Global.APPOP_HISTORY_PARAMETERS); + resolver.registerContentObserver(uri, false, new ContentObserver( + FgThread.getHandler()) { + @Override + public void onChange(boolean selfChange) { + updateParametersFromSetting(resolver); + } + }); + + updateParametersFromSetting(resolver); + + synchronized (mOnDiskLock) { + synchronized (mInMemoryLock) { + if (mMode != AppOpsManager.HISTORICAL_MODE_DISABLED) { + // Can be uninitialized if there is no config in the settings table. + if (!isPersistenceInitializedMLocked()) { + mPersistence = new Persistence(mBaseSnapshotInterval, + mIntervalCompressionMultiplier); + } + // When starting always adjust history to now. final long lastPersistTimeMills = mPersistence.getLastPersistTimeMillisDLocked(); @@ -197,16 +222,8 @@ final class HistoricalRegistry { } } - void systemReady(@NonNull ContentResolver resolver) { - updateParametersFromSetting(resolver); - final Uri uri = Settings.Global.getUriFor(Settings.Global.APPOP_HISTORY_PARAMETERS); - resolver.registerContentObserver(uri, false, new ContentObserver( - FgThread.getHandler()) { - @Override - public void onChange(boolean selfChange) { - updateParametersFromSetting(resolver); - } - }); + private boolean isPersistenceInitializedMLocked() { + return mPersistence != null; } private void updateParametersFromSetting(@NonNull ContentResolver resolver) { @@ -274,6 +291,11 @@ final class HistoricalRegistry { makeRelativeToEpochStart(currentOps, nowMillis); currentOps.accept(visitor); + if(isPersistenceInitializedMLocked()) { + Slog.e(LOG_TAG, "Interaction before persistence initialized"); + return; + } + final List<HistoricalOps> ops = mPersistence.readHistoryDLocked(); if (ops != null) { // TODO (bug:122218838): Make sure this is properly dumped @@ -302,12 +324,21 @@ final class HistoricalRegistry { void getHistoricalOpsFromDiskRaw(int uid, @NonNull String packageName, @Nullable String[] opNames, long beginTimeMillis, long endTimeMillis, @OpFlags int flags, @NonNull RemoteCallback callback) { - final HistoricalOps result = new HistoricalOps(beginTimeMillis, endTimeMillis); - mPersistence.collectHistoricalOpsDLocked(result, uid, packageName, opNames, - beginTimeMillis, endTimeMillis, flags); - final Bundle payload = new Bundle(); - payload.putParcelable(AppOpsManager.KEY_HISTORICAL_OPS, result); - callback.sendResult(payload); + synchronized (mOnDiskLock) { + synchronized (mInMemoryLock) { + if (!isPersistenceInitializedMLocked()) { + Slog.e(LOG_TAG, "Interaction before persistence initialized"); + callback.sendResult(new Bundle()); + return; + } + final HistoricalOps result = new HistoricalOps(beginTimeMillis, endTimeMillis); + mPersistence.collectHistoricalOpsDLocked(result, uid, packageName, opNames, + beginTimeMillis, endTimeMillis, flags); + final Bundle payload = new Bundle(); + payload.putParcelable(AppOpsManager.KEY_HISTORICAL_OPS, result); + callback.sendResult(payload); + } + } } void getHistoricalOps(int uid, @NonNull String packageName, @@ -331,6 +362,12 @@ final class HistoricalRegistry { boolean collectOpsFromDisk; synchronized (mInMemoryLock) { + if (!isPersistenceInitializedMLocked()) { + Slog.e(LOG_TAG, "Interaction before persistence initialized"); + callback.sendResult(new Bundle()); + return; + } + currentOps = getUpdatedPendingHistoricalOpsMLocked(currentTimeMillis); if (!(inMemoryAdjBeginTimeMillis >= currentOps.getEndTimeMillis() || inMemoryAdjEndTimeMillis <= currentOps.getBeginTimeMillis())) { @@ -374,6 +411,10 @@ final class HistoricalRegistry { @UidState int uidState, @OpFlags int flags) { synchronized (mInMemoryLock) { if (mMode == AppOpsManager.HISTORICAL_MODE_ENABLED_ACTIVE) { + if (!isPersistenceInitializedMLocked()) { + Slog.e(LOG_TAG, "Interaction before persistence initialized"); + return; + } getUpdatedPendingHistoricalOpsMLocked(System.currentTimeMillis()) .increaseAccessCount(op, uid, packageName, uidState, flags, 1); } @@ -384,6 +425,10 @@ final class HistoricalRegistry { @UidState int uidState, @OpFlags int flags) { synchronized (mInMemoryLock) { if (mMode == AppOpsManager.HISTORICAL_MODE_ENABLED_ACTIVE) { + if (!isPersistenceInitializedMLocked()) { + Slog.e(LOG_TAG, "Interaction before persistence initialized"); + return; + } getUpdatedPendingHistoricalOpsMLocked(System.currentTimeMillis()) .increaseRejectCount(op, uid, packageName, uidState, flags, 1); } @@ -394,6 +439,10 @@ final class HistoricalRegistry { @UidState int uidState, @OpFlags int flags, long increment) { synchronized (mInMemoryLock) { if (mMode == AppOpsManager.HISTORICAL_MODE_ENABLED_ACTIVE) { + if (!isPersistenceInitializedMLocked()) { + Slog.e(LOG_TAG, "Interaction before persistence initialized"); + return; + } getUpdatedPendingHistoricalOpsMLocked(System.currentTimeMillis()) .increaseAccessDuration(op, uid, packageName, uidState, flags, increment); } @@ -404,6 +453,8 @@ final class HistoricalRegistry { long baseSnapshotInterval, long intervalCompressionMultiplier) { synchronized (mOnDiskLock) { synchronized (mInMemoryLock) { + // NOTE: We allow this call if persistence is not initialized as + // it is a part of the persistence initialization process. boolean resampleHistory = false; Slog.i(LOG_TAG, "New history parameters: mode:" + AppOpsManager.historicalModeToString(mMode) + " baseSnapshotInterval:" @@ -412,7 +463,7 @@ final class HistoricalRegistry { if (mMode != mode) { mMode = mode; if (mMode == AppOpsManager.HISTORICAL_MODE_DISABLED) { - clearHistoryOnDiskLocked(); + clearHistoryOnDiskDLocked(); } } if (mBaseSnapshotInterval != baseSnapshotInterval) { @@ -433,6 +484,10 @@ final class HistoricalRegistry { void offsetHistory(long offsetMillis) { synchronized (mOnDiskLock) { synchronized (mInMemoryLock) { + if (!isPersistenceInitializedMLocked()) { + Slog.e(LOG_TAG, "Interaction before persistence initialized"); + return; + } final List<HistoricalOps> history = mPersistence.readHistoryDLocked(); clearHistory(); if (history != null) { @@ -453,6 +508,10 @@ final class HistoricalRegistry { void addHistoricalOps(HistoricalOps ops) { final List<HistoricalOps> pendingWrites; synchronized (mInMemoryLock) { + if (!isPersistenceInitializedMLocked()) { + Slog.e(LOG_TAG, "Interaction before persistence initialized"); + return; + } // The history files start from mBaseSnapshotInterval - take this into account. ops.offsetBeginAndEndTime(mBaseSnapshotInterval); mPendingWrites.offerFirst(ops); @@ -468,6 +527,10 @@ final class HistoricalRegistry { } void resetHistoryParameters() { + if (!isPersistenceInitializedMLocked()) { + Slog.e(LOG_TAG, "Interaction before persistence initialized"); + return; + } setHistoryParameters(DEFAULT_MODE, DEFAULT_SNAPSHOT_INTERVAL_MILLIS, DEFAULT_COMPRESSION_STEP); } @@ -475,6 +538,10 @@ final class HistoricalRegistry { void clearHistory(int uid, String packageName) { synchronized (mOnDiskLock) { synchronized (mInMemoryLock) { + if (!isPersistenceInitializedMLocked()) { + Slog.e(LOG_TAG, "Interaction before persistence initialized"); + return; + } if (mMode != AppOpsManager.HISTORICAL_MODE_ENABLED_ACTIVE) { return; } @@ -493,18 +560,24 @@ final class HistoricalRegistry { void clearHistory() { synchronized (mOnDiskLock) { - clearHistoryOnDiskLocked(); + synchronized (mInMemoryLock) { + if (!isPersistenceInitializedMLocked()) { + Slog.e(LOG_TAG, "Interaction before persistence initialized"); + return; + } + clearHistoryOnDiskDLocked(); + } } } - private void clearHistoryOnDiskLocked() { + private void clearHistoryOnDiskDLocked() { BackgroundThread.getHandler().removeMessages(MSG_WRITE_PENDING_HISTORY); synchronized (mInMemoryLock) { mCurrentHistoricalOps = null; mNextPersistDueTimeMillis = System.currentTimeMillis(); mPendingWrites.clear(); } - mPersistence.clearHistoryDLocked(); + Persistence.clearHistoryDLocked(); } private @NonNull HistoricalOps getUpdatedPendingHistoricalOpsMLocked(long now) { @@ -639,7 +712,7 @@ final class HistoricalRegistry { mIntervalCompressionMultiplier = intervalCompressionMultiplier; } - private final AtomicDirectory mHistoricalAppOpsDir = new AtomicDirectory( + private static final AtomicDirectory sHistoricalAppOpsDir = new AtomicDirectory( new File(new File(Environment.getDataSystemDirectory(), "appops"), "history")); private File generateFile(@NonNull File baseDir, int depth) { @@ -663,8 +736,8 @@ final class HistoricalRegistry { persistHistoricalOpsDLocked(historicalOps); } - void clearHistoryDLocked() { - mHistoricalAppOpsDir.delete(); + static void clearHistoryDLocked() { + sHistoricalAppOpsDir.delete(); } void persistHistoricalOpsDLocked(@NonNull List<HistoricalOps> ops) { @@ -673,8 +746,8 @@ final class HistoricalRegistry { enforceOpsWellFormed(ops); } try { - final File newBaseDir = mHistoricalAppOpsDir.startWrite(); - final File oldBaseDir = mHistoricalAppOpsDir.getBackupDirectory(); + final File newBaseDir = sHistoricalAppOpsDir.startWrite(); + final File oldBaseDir = sHistoricalAppOpsDir.getBackupDirectory(); final HistoricalFilesInvariant filesInvariant; if (DEBUG) { filesInvariant = new HistoricalFilesInvariant(); @@ -686,10 +759,10 @@ final class HistoricalRegistry { if (DEBUG) { filesInvariant.stopTracking(newBaseDir); } - mHistoricalAppOpsDir.finishWrite(); + sHistoricalAppOpsDir.finishWrite(); } catch (Throwable t) { wtf("Failed to write historical app ops, restoring backup", t, null); - mHistoricalAppOpsDir.failWrite(); + sHistoricalAppOpsDir.failWrite(); } } @@ -715,22 +788,36 @@ final class HistoricalRegistry { long getLastPersistTimeMillisDLocked() { File baseDir = null; try { - baseDir = mHistoricalAppOpsDir.startRead(); + baseDir = sHistoricalAppOpsDir.startRead(); final File[] files = baseDir.listFiles(); if (files != null && files.length > 0) { - final Set<File> historyFiles = new ArraySet<>(); - Collections.addAll(historyFiles, files); - for (int i = 0;; i++) { - final File file = generateFile(baseDir, i); - if (historyFiles.contains(file)) { - return file.lastModified(); + File shortestFile = null; + for (File candidate : files) { + final String candidateName = candidate.getName(); + if (!candidateName.endsWith(HISTORY_FILE_SUFFIX)) { + continue; + } + if (shortestFile == null) { + shortestFile = candidate; + } else if (candidateName.length() < shortestFile.getName().length()) { + shortestFile = candidate; } } + if (shortestFile == null) { + return 0; + } + final String shortestNameNoExtension = shortestFile.getName() + .replace(HISTORY_FILE_SUFFIX, ""); + try { + return Long.parseLong(shortestNameNoExtension); + } catch (NumberFormatException e) { + return 0; + } } - mHistoricalAppOpsDir.finishRead(); + sHistoricalAppOpsDir.finishRead(); } catch (Throwable e) { wtf("Error reading historical app ops. Deleting history.", e, baseDir); - mHistoricalAppOpsDir.delete(); + sHistoricalAppOpsDir.delete(); } return 0; } @@ -755,7 +842,7 @@ final class HistoricalRegistry { long filterBeginTimeMillis, long filterEndTimeMillis, @OpFlags int filterFlags) { File baseDir = null; try { - baseDir = mHistoricalAppOpsDir.startRead(); + baseDir = sHistoricalAppOpsDir.startRead(); final HistoricalFilesInvariant filesInvariant; if (DEBUG) { filesInvariant = new HistoricalFilesInvariant(); @@ -770,11 +857,11 @@ final class HistoricalRegistry { if (DEBUG) { filesInvariant.stopTracking(baseDir); } - mHistoricalAppOpsDir.finishRead(); + sHistoricalAppOpsDir.finishRead(); return ops; } catch (Throwable t) { wtf("Error reading historical app ops. Deleting history.", t, baseDir); - mHistoricalAppOpsDir.delete(); + sHistoricalAppOpsDir.delete(); } return null; } @@ -1241,7 +1328,7 @@ final class HistoricalRegistry { private void writeHistoricalOpsDLocked(@Nullable List<HistoricalOps> allOps, long intervalOverflowMillis, @NonNull File file) throws IOException { - final FileOutputStream output = mHistoricalAppOpsDir.openWrite(file); + final FileOutputStream output = sHistoricalAppOpsDir.openWrite(file); try { final XmlSerializer serializer = Xml.newSerializer(); serializer.setOutput(output, StandardCharsets.UTF_8.name()); @@ -1263,9 +1350,9 @@ final class HistoricalRegistry { } serializer.endTag(null, TAG_HISTORY); serializer.endDocument(); - mHistoricalAppOpsDir.closeWrite(output); + sHistoricalAppOpsDir.closeWrite(output); } catch (IOException e) { - mHistoricalAppOpsDir.failWrite(output); + sHistoricalAppOpsDir.failWrite(output); throw e; } } diff --git a/services/tests/servicestests/src/com/android/server/appop/AppOpsServiceTest.java b/services/tests/servicestests/src/com/android/server/appop/AppOpsServiceTest.java index d90117905de6..552058f6e8c2 100644 --- a/services/tests/servicestests/src/com/android/server/appop/AppOpsServiceTest.java +++ b/services/tests/servicestests/src/com/android/server/appop/AppOpsServiceTest.java @@ -37,12 +37,15 @@ import android.os.Handler; import android.os.HandlerThread; import android.os.Process; import android.os.RemoteCallback; +import android.provider.Settings; import androidx.test.InstrumentationRegistry; import androidx.test.filters.SmallTest; import androidx.test.runner.AndroidJUnit4; +import org.junit.AfterClass; import org.junit.Before; +import org.junit.BeforeClass; import org.junit.Test; import org.junit.runner.RunWith; @@ -68,11 +71,14 @@ public class AppOpsServiceTest { private File mAppOpsFile; private Context mContext; private Handler mHandler; + private AppOpsManager mAppOpsManager; private AppOpsService mAppOpsService; private String mMyPackageName; private int mMyUid; private long mTestStartMillis; + private static String sDefaultAppopHistoryParameters; + @Before public void setUp() { mContext = InstrumentationRegistry.getTargetContext(); @@ -88,11 +94,29 @@ public class AppOpsServiceTest { mMyPackageName = mContext.getOpPackageName(); mMyUid = Process.myUid(); + mAppOpsManager = mContext.getSystemService(AppOpsManager.class); mAppOpsService = new AppOpsService(mAppOpsFile, mHandler); + mAppOpsService.mHistoricalRegistry.systemReady(mContext.getContentResolver()); mAppOpsService.mContext = mContext; mTestStartMillis = System.currentTimeMillis(); } + @BeforeClass + public static void configureDesiredAppopHistoryParameters() { + final Context context = InstrumentationRegistry.getTargetContext(); + sDefaultAppopHistoryParameters = Settings.Global.getString(context.getContentResolver(), + Settings.Global.APPOP_HISTORY_PARAMETERS); + Settings.Global.putString(InstrumentationRegistry.getTargetContext().getContentResolver(), + Settings.Global.APPOP_HISTORY_PARAMETERS, null); + } + + @AfterClass + public static void restoreDefaultAppopHistoryParameters() { + Settings.Global.putString(InstrumentationRegistry.getTargetContext().getContentResolver(), + Settings.Global.APPOP_HISTORY_PARAMETERS, + sDefaultAppopHistoryParameters); + } + @Test public void testGetOpsForPackage_noOpsLogged() { assertThat(getLoggedOps()).isNull(); |