From 7b720288b5c1a1259d687c9467f63ddf71e6512a Mon Sep 17 00:00:00 2001 From: Jeongsik Mun Date: Sun, 12 Sep 2021 13:08:08 +0900 Subject: [PATCH] Fix the overrides file corruption due to shutdown The dynamic overrides file could be corrupted by shutdown during writes. This CL introduces the backup file to avoid corruption. Test: manual 1. push a backup file to /data/misc/appcompat/ 2. push a corrupted file to /data/misc/appcompat/ 3. Reboot and see if it reads the backup file Bug: 196909332 Change-Id: If13547f04a4712ab18dcb97ffee698327797f97b --- .../android/server/compat/CompatConfig.java | 46 ++++++++++++++++--- 1 file changed, 39 insertions(+), 7 deletions(-) diff --git a/services/core/java/com/android/server/compat/CompatConfig.java b/services/core/java/com/android/server/compat/CompatConfig.java index 2bf1ccd519390..fb7417084baea 100644 --- a/services/core/java/com/android/server/compat/CompatConfig.java +++ b/services/core/java/com/android/server/compat/CompatConfig.java @@ -81,8 +81,11 @@ final class CompatConfig { private final OverrideValidatorImpl mOverrideValidator; private final AndroidBuildClassifier mAndroidBuildClassifier; private Context mContext; - @GuardedBy("mOverridesFile") + private final Object mOverridesFileLock = new Object(); + @GuardedBy("mOverridesFileLock") private File mOverridesFile; + @GuardedBy("mOverridesFileLock") + private File mBackupOverridesFile; @VisibleForTesting CompatConfig(AndroidBuildClassifier androidBuildClassifier, Context context) { @@ -550,8 +553,15 @@ final class CompatConfig { loadOverrides(staticOverridesFile); - mOverridesFile = dynamicOverridesFile; - loadOverrides(dynamicOverridesFile); + synchronized (mOverridesFileLock) { + mOverridesFile = dynamicOverridesFile; + mBackupOverridesFile = makeBackupFile(dynamicOverridesFile); + if (mBackupOverridesFile.exists()) { + mOverridesFile.delete(); + mBackupOverridesFile.renameTo(mOverridesFile); + } + loadOverrides(mOverridesFile); + } if (staticOverridesFile.exists()) { // Only save overrides if there is a static overrides file. @@ -559,6 +569,10 @@ final class CompatConfig { } } + private File makeBackupFile(File overridesFile) { + return new File(overridesFile.getPath() + ".bak"); + } + private void loadOverrides(File overridesFile) { if (!overridesFile.exists()) { // Overrides file doesn't exist. @@ -591,10 +605,11 @@ final class CompatConfig { * Persist compat framework overrides to /data/misc/appcompat/compat_framework_overrides.xml */ void saveOverrides() { - if (mOverridesFile == null) { - return; - } - synchronized (mOverridesFile) { + synchronized (mOverridesFileLock) { + if (mOverridesFile == null || mBackupOverridesFile == null) { + return; + } + Overrides overrides = new Overrides(); List changeOverridesList = overrides.getChangeOverrides(); for (CompatChange c : mChanges.values()) { @@ -603,6 +618,20 @@ final class CompatConfig { changeOverridesList.add(changeOverrides); } } + + // Rename the file to the backup. + if (mOverridesFile.exists()) { + if (mBackupOverridesFile.exists()) { + mOverridesFile.delete(); + } else { + if (!mOverridesFile.renameTo(mBackupOverridesFile)) { + Slog.e(TAG, "Couldn't rename file " + mOverridesFile + + " to " + mBackupOverridesFile); + return; + } + } + } + // Create the file if it doesn't already exist try { mOverridesFile.createNewFile(); @@ -616,6 +645,9 @@ final class CompatConfig { } catch (IOException e) { Slog.e(TAG, e.toString()); } + + // Remove the backup if the write succeeds. + mBackupOverridesFile.delete(); } }