Fix runtime permissinos toggling and relax XML parsing.
1. Fixed the case where runtime permissons can be toggled by a
developer via a system property.
2. Relaxed the runtime permission XML parsing to be more fault
toelrant and consistent wiht the reset of the package manager
parse code.
3. Fixed a deadlock due to calling in to the activity manager
with the package manager lock held to kill an app.
Change-Id: I11dfb57ad4d8119baea79227dc2a3fe5e2208515
diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java
index 1b8ed22..c7dc74f 100644
--- a/services/core/java/com/android/server/pm/PackageManagerService.java
+++ b/services/core/java/com/android/server/pm/PackageManagerService.java
@@ -1349,7 +1349,7 @@
mOnlyCore = onlyCore;
mLazyDexOpt = "eng".equals(SystemProperties.get("ro.build.type"));
mMetrics = new DisplayMetrics();
- mSettings = new Settings(mContext, mPackages);
+ mSettings = new Settings(mPackages);
mSettings.addSharedUserLPw("android.uid.system", Process.SYSTEM_UID,
ApplicationInfo.FLAG_SYSTEM, ApplicationInfo.PRIVATE_FLAG_PRIVILEGED);
mSettings.addSharedUserLPw("android.uid.phone", RADIO_UID,
@@ -1812,23 +1812,11 @@
+ "; regranting permissions for internal storage");
mSettings.mInternalSdkPlatform = mSdkVersion;
-
- // We keep track for which users we granted permissions to be able
- // to grant runtime permissions to system apps for newly appeared
- // users. If we supported runtime permissions during the previous
- // boot, then we already granted permissions for all device users.
- // In such a case we set the users for which we granted permissions
- // to avoid clobbering of runtime permissions we granted to system
- // apps but the user revoked later.
- if (PackageManagerService.RUNTIME_PERMISSIONS_ENABLED &&
- mSettings.mRuntimePermissionEnabled) {
- final int[] userIds = UserManagerService.getInstance().getUserIds();
- for (PackageSetting ps : mSettings.mPackages.values()) {
- ps.setPermissionsUpdatedForUserIds(userIds);
- }
- for (SharedUserSetting sus : mSettings.mSharedUsers.values()) {
- sus.setPermissionsUpdatedForUserIds(userIds);
- }
+ // For now runtime permissions are toggled via a system property.
+ if (!RUNTIME_PERMISSIONS_ENABLED) {
+ // Remove the runtime permissions state if the feature
+ // was disabled by flipping the system property.
+ mSettings.deleteRuntimePermissionsFiles();
}
updatePermissionsLPw(null, null, UPDATE_PERMISSIONS_ALL
@@ -2714,6 +2702,10 @@
@Override
public boolean grantPermission(String packageName, String name, int userId) {
+ if (!RUNTIME_PERMISSIONS_ENABLED) {
+ return false;
+ }
+
if (!sUserManager.exists(userId)) {
return false;
}
@@ -2725,6 +2717,9 @@
enforceCrossUserPermission(Binder.getCallingUid(), userId, true, false,
"grantPermission");
+ boolean gidsChanged = false;
+ final SettingBase sb;
+
synchronized (mPackages) {
final PackageParser.Package pkg = mPackages.get(packageName);
if (pkg == null) {
@@ -2738,7 +2733,7 @@
enforceDeclaredAsUsedAndRuntimePermission(pkg, bp);
- final SettingBase sb = (SettingBase) pkg.mExtras;
+ sb = (SettingBase) pkg.mExtras;
if (sb == null) {
throw new IllegalArgumentException("Unknown package: " + packageName);
}
@@ -2752,19 +2747,27 @@
}
case PermissionsState.PERMISSION_OPERATION_SUCCESS_GIDS_CHANGED: {
- killSettingPackagesForUser(sb, userId, KILL_APP_REASON_GIDS_CHANGED);
+ gidsChanged = true;
} break;
}
// Not critical if that is lost - app has to request again.
mSettings.writeRuntimePermissionsForUserLPr(userId, false);
-
- return true;
}
+
+ if (gidsChanged) {
+ killSettingPackagesForUser(sb, userId, KILL_APP_REASON_GIDS_CHANGED);
+ }
+
+ return true;
}
@Override
public boolean revokePermission(String packageName, String name, int userId) {
+ if (!RUNTIME_PERMISSIONS_ENABLED) {
+ return false;
+ }
+
if (!sUserManager.exists(userId)) {
return false;
}
@@ -2776,6 +2779,8 @@
enforceCrossUserPermission(Binder.getCallingUid(), userId, true, false,
"revokePermission");
+ final SettingBase sb;
+
synchronized (mPackages) {
final PackageParser.Package pkg = mPackages.get(packageName);
if (pkg == null) {
@@ -2789,7 +2794,7 @@
enforceDeclaredAsUsedAndRuntimePermission(pkg, bp);
- final SettingBase sb = (SettingBase) pkg.mExtras;
+ sb = (SettingBase) pkg.mExtras;
if (sb == null) {
throw new IllegalArgumentException("Unknown package: " + packageName);
}
@@ -2801,13 +2806,13 @@
return false;
}
- killSettingPackagesForUser(sb, userId, KILL_APP_REASON_PERMISSIONS_REVOKED);
-
// Critical, after this call all should never have the permission.
mSettings.writeRuntimePermissionsForUserLPr(userId, true);
-
- return true;
}
+
+ killSettingPackagesForUser(sb, userId, KILL_APP_REASON_PERMISSIONS_REVOKED);
+
+ return true;
}
@Override
@@ -7067,7 +7072,7 @@
<= Build.VERSION_CODES.LOLLIPOP_MR1) {
// For legacy apps dangerous permissions are install time ones.
grant = GRANT_INSTALL;
- } else if ((pkg.applicationInfo.flags & ApplicationInfo.FLAG_SYSTEM) != 0) {
+ } else if (ps.isSystem()) {
final int[] updatedUserIds = ps.getPermissionsUpdatedForUserIds();
if (origPermissions.hasInstallPermission(bp.name)) {
// If a system app had an install permission, then the app was
@@ -7200,11 +7205,13 @@
ps.installPermissionsFixed = true;
}
- ps.setPermissionsUpdatedForUserIds(changedRuntimePermissionUserIds);
+ ps.setPermissionsUpdatedForUserIds(currentUserIds);
// Persist the runtime permissions state for users with changes.
- for (int userId : changedRuntimePermissionUserIds) {
- mSettings.writeRuntimePermissionsForUserLPr(userId, true);
+ if (RUNTIME_PERMISSIONS_ENABLED) {
+ for (int userId : changedRuntimePermissionUserIds) {
+ mSettings.writeRuntimePermissionsForUserLPr(userId, true);
+ }
}
}
@@ -11043,15 +11050,18 @@
for (int userId : UserManagerService.getInstance().getUserIds()) {
final int userIdToKill = mSettings.updateSharedUserPermsLPw(deletedPs,
userId);
- if (userIdToKill == userId) {
+ if (userIdToKill == UserHandle.USER_ALL
+ || userIdToKill >= UserHandle.USER_OWNER) {
// If gids changed for this user, kill all affected packages.
- killSettingPackagesForUser(deletedPs, userIdToKill,
- KILL_APP_REASON_GIDS_CHANGED);
- } else if (userIdToKill == UserHandle.USER_ALL) {
- // If gids changed for all users, kill them all - done.
- killSettingPackagesForUser(deletedPs, userIdToKill,
- KILL_APP_REASON_GIDS_CHANGED);
- break;
+ mHandler.post(new Runnable() {
+ @Override
+ public void run() {
+ // This has to happen with no lock held.
+ killSettingPackagesForUser(deletedPs, userIdToKill,
+ KILL_APP_REASON_GIDS_CHANGED);
+ }
+ });
+ break;
}
}
}
diff --git a/services/core/java/com/android/server/pm/PackageSetting.java b/services/core/java/com/android/server/pm/PackageSetting.java
index 889164c..a3f4c0b 100644
--- a/services/core/java/com/android/server/pm/PackageSetting.java
+++ b/services/core/java/com/android/server/pm/PackageSetting.java
@@ -70,4 +70,8 @@
public boolean isForwardLocked() {
return (pkgPrivateFlags & ApplicationInfo.PRIVATE_FLAG_FORWARD_LOCK) != 0;
}
+
+ public boolean isSystem() {
+ return (pkgFlags & ApplicationInfo.FLAG_SYSTEM) != 0;
+ }
}
diff --git a/services/core/java/com/android/server/pm/Settings.java b/services/core/java/com/android/server/pm/Settings.java
index 0a2389f..8f185ec 100644
--- a/services/core/java/com/android/server/pm/Settings.java
+++ b/services/core/java/com/android/server/pm/Settings.java
@@ -25,7 +25,6 @@
import static android.os.Process.SYSTEM_UID;
import static android.os.Process.PACKAGE_INFO_GID;
-import android.content.Context;
import android.content.IntentFilter;
import android.content.pm.ActivityInfo;
import android.content.pm.ResolveInfo;
@@ -175,10 +174,8 @@
private static final String ATTR_HIDDEN = "hidden";
private static final String ATTR_INSTALLED = "inst";
private static final String ATTR_BLOCK_UNINSTALL = "blockUninstall";
- private static final String ATTR_RUNTIME_PERMSISSIONS_ENABLED = "runtime-permissions-enabled";
private final Object mLock;
- private final Context mContext;
private final RuntimePermissionPersistence mRuntimePermissionsPersistence;
@@ -202,10 +199,6 @@
int mInternalSdkPlatform;
int mExternalSdkPlatform;
-
- // Whether runtime permissions are enabled.
- boolean mRuntimePermissionEnabled;
-
/**
* The current database version for apps on internal storage. This is
* used to upgrade the format of the packages.xml database not necessarily
@@ -282,12 +275,11 @@
public final KeySetManagerService mKeySetManagerService = new KeySetManagerService(mPackages);
- Settings(Context context, Object lock) {
- this(context, Environment.getDataDirectory(), lock);
+ Settings(Object lock) {
+ this(Environment.getDataDirectory(), lock);
}
- Settings(Context context, File dataDir, Object lock) {
- mContext = context;
+ Settings(File dataDir, Object lock) {
mLock = lock;
mRuntimePermissionsPersistence = new RuntimePermissionPersistence(mLock);
@@ -953,6 +945,17 @@
return new File(userDir, RUNTIME_PERMISSIONS_FILE_NAME);
}
+ boolean isFirstRuntimePermissionsBoot() {
+ return !getUserRuntimePermissionsFile(UserHandle.USER_OWNER).exists();
+ }
+
+ void deleteRuntimePermissionsFiles() {
+ for (int userId : UserManagerService.getInstance().getUserIds()) {
+ File file = getUserRuntimePermissionsFile(userId);
+ file.delete();
+ }
+ }
+
private File getUserPackagesStateBackupFile(int userId) {
return new File(Environment.getUserSystemDirectory(userId),
"package-restrictions-backup.xml");
@@ -1650,8 +1653,6 @@
serializer.attribute(null, "internal", Integer.toString(mInternalSdkPlatform));
serializer.attribute(null, "external", Integer.toString(mExternalSdkPlatform));
serializer.attribute(null, "fingerprint", mFingerprint);
- serializer.attribute(null, ATTR_RUNTIME_PERMSISSIONS_ENABLED,
- String.valueOf(PackageManagerService.RUNTIME_PERMISSIONS_ENABLED));
serializer.endTag(null, "last-platform-version");
serializer.startTag(null, "database-version");
@@ -2148,8 +2149,6 @@
} catch (NumberFormatException e) {
}
mFingerprint = parser.getAttributeValue(null, "fingerprint");
- mRuntimePermissionEnabled = XmlUtils.readBooleanAttribute(parser,
- ATTR_RUNTIME_PERMSISSIONS_ENABLED);
} else if (tagName.equals("database-version")) {
mInternalDatabaseVersion = mExternalDatabaseVersion = 0;
try {
@@ -2739,6 +2738,18 @@
}
}
+ // We keep track for which users we granted permissions to be able
+ // to grant runtime permissions to system apps for newly appeared
+ // users or newly appeared system apps. If we supported runtime
+ // permissions during the previous boot, then we already granted
+ // permissions for all device users. In such a case we set the users
+ // for which we granted permissions to avoid clobbering of runtime
+ // permissions we granted to system apps but the user revoked later.
+ if (!isFirstRuntimePermissionsBoot()) {
+ final int[] userIds = UserManagerService.getInstance().getUserIds();
+ ps.setPermissionsUpdatedForUserIds(userIds);
+ }
+
mDisabledSysPackages.put(name, ps);
}
@@ -3019,6 +3030,18 @@
XmlUtils.skipCurrentTag(parser);
}
}
+
+ // We keep track for which users we granted permissions to be able
+ // to grant runtime permissions to system apps for newly appeared
+ // users or newly appeared system apps. If we supported runtime
+ // permissions during the previous boot, then we already granted
+ // permissions for all device users. In such a case we set the users
+ // for which we granted permissions to avoid clobbering of runtime
+ // permissions we granted to system apps but the user revoked later.
+ if (!isFirstRuntimePermissionsBoot()) {
+ final int[] userIds = UserManagerService.getInstance().getUserIds();
+ packageSetting.setPermissionsUpdatedForUserIds(userIds);
+ }
} else {
XmlUtils.skipCurrentTag(parser);
}
@@ -3136,6 +3159,18 @@
XmlUtils.skipCurrentTag(parser);
}
}
+
+ // We keep track for which users we granted permissions to be able
+ // to grant runtime permissions to system apps for newly appeared
+ // users or newly appeared system apps. If we supported runtime
+ // permissions during the previous boot, then we already granted
+ // permissions for all device users. In such a case we set the users
+ // for which we granted permissions to avoid clobbering of runtime
+ // permissions we granted to system apps but the user revoked later.
+ if (!isFirstRuntimePermissionsBoot()) {
+ final int[] userIds = UserManagerService.getInstance().getUserIds();
+ su.setPermissionsUpdatedForUserIds(userIds);
+ }
} else {
XmlUtils.skipCurrentTag(parser);
}
@@ -3850,11 +3885,19 @@
}
public void writePermissionsForUserSyncLPr(int userId) {
+ if (!PackageManagerService.RUNTIME_PERMISSIONS_ENABLED) {
+ return;
+ }
+
mHandler.removeMessages(userId);
writePermissionsSync(userId);
}
public void writePermissionsForUserAsyncLPr(int userId) {
+ if (!PackageManagerService.RUNTIME_PERMISSIONS_ENABLED) {
+ return;
+ }
+
final long currentTimeMillis = SystemClock.uptimeMillis();
if (mWriteScheduled.get(userId)) {
@@ -4014,128 +4057,67 @@
private void parseRuntimePermissionsLPr(XmlPullParser parser, int userId)
throws IOException, XmlPullParserException {
- parser.next();
- skipEmptyTextTags(parser);
- if (!accept(parser, XmlPullParser.START_TAG, TAG_RUNTIME_PERMISSIONS)) {
- return;
- }
+ final int outerDepth = parser.getDepth();
+ int type;
+ while ((type = parser.next()) != XmlPullParser.END_DOCUMENT
+ && (type != XmlPullParser.END_TAG || parser.getDepth() > outerDepth)) {
+ if (type == XmlPullParser.END_TAG || type == XmlPullParser.TEXT) {
+ continue;
+ }
- parser.next();
+ switch (parser.getName()) {
+ case TAG_PACKAGE: {
+ String name = parser.getAttributeValue(null, ATTR_NAME);
+ PackageSetting ps = mPackages.get(name);
+ if (ps == null) {
+ Slog.w(PackageManagerService.TAG, "Unknown package:" + name);
+ XmlUtils.skipCurrentTag(parser);
+ continue;
+ }
+ parsePermissionsLPr(parser, ps.getPermissionsState(), userId);
+ } break;
- while (parsePackageLPr(parser, userId)
- || parseSharedUserLPr(parser, userId)) {
- parser.next();
- }
-
- skipEmptyTextTags(parser);
- expect(parser, XmlPullParser.END_TAG, TAG_RUNTIME_PERMISSIONS);
- }
-
- private boolean parsePackageLPr(XmlPullParser parser, int userId)
- throws IOException, XmlPullParserException {
- skipEmptyTextTags(parser);
- if (!accept(parser, XmlPullParser.START_TAG, TAG_PACKAGE)) {
- return false;
- }
-
- String name = parser.getAttributeValue(null, ATTR_NAME);
-
- parser.next();
-
- PackageSetting ps = mPackages.get(name);
- if (ps != null) {
- while (parsePermissionLPr(parser, ps.getPermissionsState(), userId)) {
- parser.next();
+ case TAG_SHARED_USER: {
+ String name = parser.getAttributeValue(null, ATTR_NAME);
+ SharedUserSetting sus = mSharedUsers.get(name);
+ if (sus == null) {
+ Slog.w(PackageManagerService.TAG, "Unknown shared user:" + name);
+ XmlUtils.skipCurrentTag(parser);
+ continue;
+ }
+ parsePermissionsLPr(parser, sus.getPermissionsState(), userId);
+ } break;
}
}
-
- skipEmptyTextTags(parser);
- expect(parser, XmlPullParser.END_TAG, TAG_PACKAGE);
-
- return true;
}
- private boolean parseSharedUserLPr(XmlPullParser parser, int userId)
- throws IOException, XmlPullParserException {
- skipEmptyTextTags(parser);
- if (!accept(parser, XmlPullParser.START_TAG, TAG_SHARED_USER)) {
- return false;
- }
-
- String name = parser.getAttributeValue(null, ATTR_NAME);
-
- parser.next();
-
- SharedUserSetting sus = mSharedUsers.get(name);
- if (sus != null) {
- while (parsePermissionLPr(parser, sus.getPermissionsState(), userId)) {
- parser.next();
- }
- }
-
- skipEmptyTextTags(parser);
- expect(parser, XmlPullParser.END_TAG, TAG_SHARED_USER);
-
- return true;
- }
-
- private boolean parsePermissionLPr(XmlPullParser parser, PermissionsState permissionsState,
+ private void parsePermissionsLPr(XmlPullParser parser, PermissionsState permissionsState,
int userId) throws IOException, XmlPullParserException {
- skipEmptyTextTags(parser);
- if (!accept(parser, XmlPullParser.START_TAG, TAG_ITEM)) {
- return false;
- }
-
- String name = parser.getAttributeValue(null, ATTR_NAME);
-
- parser.next();
-
- BasePermission bp = mPermissions.get(name);
- if (bp != null) {
- if (permissionsState.grantRuntimePermission(bp, userId) ==
- PermissionsState.PERMISSION_OPERATION_FAILURE) {
- Slog.w(PackageManagerService.TAG, "Duplicate permission:" + name);
+ final int outerDepth = parser.getDepth();
+ int type;
+ while ((type = parser.next()) != XmlPullParser.END_DOCUMENT
+ && (type != XmlPullParser.END_TAG || parser.getDepth() > outerDepth)) {
+ if (type == XmlPullParser.END_TAG || type == XmlPullParser.TEXT) {
+ continue;
}
- } else {
- Slog.w(PackageManagerService.TAG, "Unknown permission:" + name);
- }
- skipEmptyTextTags(parser);
- expect(parser, XmlPullParser.END_TAG, TAG_ITEM);
+ switch (parser.getName()) {
+ case TAG_ITEM: {
+ String name = parser.getAttributeValue(null, ATTR_NAME);
+ BasePermission bp = mPermissions.get(name);
+ if (bp == null) {
+ Slog.w(PackageManagerService.TAG, "Unknown permission:" + name);
+ XmlUtils.skipCurrentTag(parser);
+ continue;
+ }
- return true;
- }
-
- private void expect(XmlPullParser parser, int type, String tag)
- throws IOException, XmlPullParserException {
- if (!accept(parser, type, tag)) {
- throw new XmlPullParserException("Expected event: " + type
- + " and tag: " + tag + " but got event: " + parser.getEventType()
- + " and tag:" + parser.getName());
- }
- }
-
- private void skipEmptyTextTags(XmlPullParser parser)
- throws IOException, XmlPullParserException {
- while (accept(parser, XmlPullParser.TEXT, null)
- && parser.isWhitespace()) {
- parser.next();
- }
- }
-
- private boolean accept(XmlPullParser parser, int type, String tag)
- throws IOException, XmlPullParserException {
- if (parser.getEventType() != type) {
- return false;
- }
- if (tag != null) {
- if (!tag.equals(parser.getName())) {
- return false;
+ if (permissionsState.grantRuntimePermission(bp, userId) ==
+ PermissionsState.PERMISSION_OPERATION_FAILURE) {
+ Slog.w(PackageManagerService.TAG, "Duplicate permission:" + name);
+ }
+ } break;
}
- } else if (parser.getName() != null) {
- return false;
}
- return true;
}
private void writePermissions(XmlSerializer serializer, Set<String> permissions)
diff --git a/services/tests/servicestests/src/com/android/server/pm/PackageManagerSettingsTests.java b/services/tests/servicestests/src/com/android/server/pm/PackageManagerSettingsTests.java
index 4dc1131..6a471fe 100644
--- a/services/tests/servicestests/src/com/android/server/pm/PackageManagerSettingsTests.java
+++ b/services/tests/servicestests/src/com/android/server/pm/PackageManagerSettingsTests.java
@@ -134,7 +134,7 @@
public void testSettingsReadOld() {
// Write the package files and make sure they're parsed properly the first time
writeOldFiles();
- Settings settings = new Settings(getContext(), getContext().getFilesDir(), new Object());
+ Settings settings = new Settings(getContext().getFilesDir(), new Object());
assertEquals(true, settings.readLPw(null, null, 0, false));
assertNotNull(settings.peekPackageLPr(PACKAGE_NAME_3));
assertNotNull(settings.peekPackageLPr(PACKAGE_NAME_1));
@@ -152,7 +152,7 @@
public void testNewPackageRestrictionsFile() {
// Write the package files and make sure they're parsed properly the first time
writeOldFiles();
- Settings settings = new Settings(getContext(), getContext().getFilesDir(), new Object());
+ Settings settings = new Settings(getContext().getFilesDir(), new Object());
assertEquals(true, settings.readLPw(null, null, 0, false));
settings.writeLPr();