From 094535fe02cbb97e05592fc5b17cd51d30025f3f Mon Sep 17 00:00:00 2001 From: Svetoslav Ganov Date: Wed, 16 Aug 2017 14:50:00 -0700 Subject: [PATCH] Properly compute default and system set flag on an upgrade We added the notions of a default value and whether this default is set by the system for a setting which is needed for experiments since in case a bad value is pushed we should be able to incrementally rollback to a stable state. If a system component sets a setting value this automatically becomes the default. System components are the platform package, the special UIDs (telephony, etc), apps singed with the platform cert, system installed persistent apps, and SUW. In N we did not have the notion of a default and during an upgrade need to adjust the default and whether this default is set by the system. This migration runs as the system UID and was incorrectly computing that the package that changed the settings last was a part of the system and setting the current value as its default set by the system. This resulted in taking more storage space as we also count the default which led the package which changed the setting to go over the quota and that throws. If the first caller into the settings provider is the system main thread (almost certain) we end up throwing an exception on the system main thread - crashing the system server. Test: flash N, install an app abusing sys settings, update to O bug:64718888 Change-Id: I82f0d52fd7984fb2d0da1fd91399a0c914dfa24b (cherry picked from commit bf91d4072f2b91969a17028f8b21d72f004cc6af) --- .../providers/settings/SettingsProvider.java | 83 ++++++++++++++++------ .../android/providers/settings/SettingsState.java | 24 ++++++- 2 files changed, 84 insertions(+), 23 deletions(-) diff --git a/packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java b/packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java index f5d7dd8d9acd..973d21c80616 100644 --- a/packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java +++ b/packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java @@ -2895,7 +2895,7 @@ public class SettingsProvider extends ContentProvider { } private final class UpgradeController { - private static final int SETTINGS_VERSION = 145; + private static final int SETTINGS_VERSION = 146; private final int mUserId; @@ -3421,22 +3421,11 @@ public class SettingsProvider extends ContentProvider { } if (currentVersion == 141) { - // Version 142: We added the notion of a default and whether the system set - // the setting. This is used for resetting the internal state and we need - // to make sure this value is updated for the existing settings, otherwise - // we would delete system set settings while they should stay unmodified. - SettingsState globalSettings = getGlobalSettingsLocked(); - ensureLegacyDefaultValueAndSystemSetUpdatedLocked(globalSettings); - globalSettings.persistSyncLocked(); - - SettingsState secureSettings = getSecureSettingsLocked(mUserId); - ensureLegacyDefaultValueAndSystemSetUpdatedLocked(secureSettings); - secureSettings.persistSyncLocked(); - - SettingsState systemSettings = getSystemSettingsLocked(mUserId); - ensureLegacyDefaultValueAndSystemSetUpdatedLocked(systemSettings); - systemSettings.persistSyncLocked(); - + // This implementation was incorrectly setting the current value of + // settings changed by non-system packages as the default which default + // is set by the system. We add a new upgrade step at the end to properly + // handle this case which would also fix incorrect changes made by the + // old implementation of this step. currentVersion = 142; } @@ -3496,6 +3485,31 @@ public class SettingsProvider extends ContentProvider { currentVersion = 145; } + if (currentVersion == 145) { + // Version 146: In step 142 we had a bug where incorrectly + // some settings were considered system set and as a result + // made the default and marked as the default being set by + // the system. Here reevaluate the default and default system + // set flags. This would both fix corruption by the old impl + // of step 142 and also properly handle devices which never + // run 142. + if (userId == UserHandle.USER_SYSTEM) { + SettingsState globalSettings = getGlobalSettingsLocked(); + ensureLegacyDefaultValueAndSystemSetUpdatedLocked(globalSettings, userId); + globalSettings.persistSyncLocked(); + } + + SettingsState secureSettings = getSecureSettingsLocked(mUserId); + ensureLegacyDefaultValueAndSystemSetUpdatedLocked(secureSettings, userId); + secureSettings.persistSyncLocked(); + + SettingsState systemSettings = getSystemSettingsLocked(mUserId); + ensureLegacyDefaultValueAndSystemSetUpdatedLocked(systemSettings, userId); + systemSettings.persistSyncLocked(); + + currentVersion = 146; + } + // vXXX: Add new settings above this point. if (currentVersion != newVersion) { @@ -3514,19 +3528,46 @@ public class SettingsProvider extends ContentProvider { } } - private void ensureLegacyDefaultValueAndSystemSetUpdatedLocked(SettingsState settings) { + private void ensureLegacyDefaultValueAndSystemSetUpdatedLocked(SettingsState settings, + int userId) { List names = settings.getSettingNamesLocked(); final int nameCount = names.size(); for (int i = 0; i < nameCount; i++) { String name = names.get(i); Setting setting = settings.getSettingLocked(name); - if (setting.getDefaultValue() == null) { - boolean systemSet = SettingsState.isSystemPackage(getContext(), - setting.getPackageName()); + + // In the upgrade case we pretend the call is made from the app + // that made the last change to the setting to properly determine + // whether the call has been made by a system component. + int callingUid = -1; + try { + callingUid = mPackageManager.getPackageUid(setting.getPackageName(), 0, userId); + } catch (RemoteException e) { + /* ignore - handled below */ + } + if (callingUid < 0) { + Slog.e(LOG_TAG, "Unknown package: " + setting.getPackageName()); + continue; + } + try { + final boolean systemSet = SettingsState.isSystemPackage(getContext(), + setting.getPackageName(), callingUid); if (systemSet) { settings.insertSettingLocked(name, setting.getValue(), setting.getTag(), true, setting.getPackageName()); + } else if (setting.getDefaultValue() != null && setting.isDefaultFromSystem()) { + // We had a bug where changes by non-system packages were marked + // as system made and as a result set as the default. Therefore, if + // the package changed the setting last is not a system one but the + // setting is marked as its default coming from the system we clear + // the default and clear the system set flag. + settings.resetSettingDefaultValueLocked(name); } + } catch (IllegalStateException e) { + // If the package goes over its quota during the upgrade, don't + // crash but just log the error as the system does the upgrade. + Slog.e(LOG_TAG, "Error upgrading setting: " + setting.getName(), e); + } } } diff --git a/packages/SettingsProvider/src/com/android/providers/settings/SettingsState.java b/packages/SettingsProvider/src/com/android/providers/settings/SettingsState.java index 5f4b2391a763..6c983601b5a9 100644 --- a/packages/SettingsProvider/src/com/android/providers/settings/SettingsState.java +++ b/packages/SettingsProvider/src/com/android/providers/settings/SettingsState.java @@ -298,6 +298,22 @@ final class SettingsState { } // The settings provider must hold its lock when calling here. + public void resetSettingDefaultValueLocked(String name) { + Setting oldSetting = getSettingLocked(name); + if (oldSetting != null && !oldSetting.isNull() && oldSetting.getDefaultValue() != null) { + String oldValue = oldSetting.getValue(); + String oldDefaultValue = oldSetting.getDefaultValue(); + Setting newSetting = new Setting(name, oldSetting.getValue(), null, + oldSetting.getPackageName(), oldSetting.getTag(), false, + oldSetting.getId()); + mSettings.put(name, newSetting); + updateMemoryUsagePerPackageLocked(newSetting.getPackageName(), oldValue, + newSetting.getValue(), oldDefaultValue, newSetting.getDefaultValue()); + scheduleWriteIfNeededLocked(); + } + } + + // The settings provider must hold its lock when calling here. public boolean insertSettingLocked(String name, String value, String tag, boolean makeDefault, String packageName) { if (TextUtils.isEmpty(name)) { @@ -1003,6 +1019,10 @@ final class SettingsState { } public static boolean isSystemPackage(Context context, String packageName) { + return isSystemPackage(context, packageName, Binder.getCallingUid()); + } + + public static boolean isSystemPackage(Context context, String packageName, int callingUid) { synchronized (sLock) { if (SYSTEM_PACKAGE_NAME.equals(packageName)) { return true; @@ -1015,7 +1035,7 @@ final class SettingsState { } // Native services running as a special UID get a pass - final int callingAppId = UserHandle.getAppId(Binder.getCallingUid()); + final int callingAppId = UserHandle.getAppId(callingUid); if (callingAppId < FIRST_APPLICATION_UID) { sSystemUids.put(callingAppId, callingAppId); return true; @@ -1026,7 +1046,7 @@ final class SettingsState { // profile for the purpose of determining whether the other end is a // system component we need to use the user id of the caller for // pulling information about the caller from the package manager. - final int callingUserId = UserHandle.getCallingUserId(); + final int callingUserId = UserHandle.getUserId(callingUid); final long identity = Binder.clearCallingIdentity(); try { -- 2.11.0