From eddeb49a734a524347587e7654025c489fb6331e Mon Sep 17 00:00:00 2001 From: Adam Lesinski Date: Mon, 8 Sep 2014 17:50:03 -0700 Subject: [PATCH 1/2] ActivityManager shouldn't return null for getCurrentUser There was a race where ActivityManager would return null for getCurrentUser() when switching between guest accounts. This is because the Guest account was marked for deletion while it was still active. Bug:17290209 Change-Id: I224fb4b6836380e5acb7dbeb8f3343d74505f88a --- core/java/android/content/pm/UserInfo.java | 4 ++ .../com/android/server/am/ActiveServices.java | 7 +++ .../android/server/pm/UserManagerService.java | 45 +++++++++++++++---- 3 files changed, 47 insertions(+), 9 deletions(-) diff --git a/core/java/android/content/pm/UserInfo.java b/core/java/android/content/pm/UserInfo.java index c0383a33da306..c03be32f2f010 100644 --- a/core/java/android/content/pm/UserInfo.java +++ b/core/java/android/content/pm/UserInfo.java @@ -89,6 +89,7 @@ public class UserInfo implements Parcelable { /** User is only partially created. */ public boolean partial; + public boolean guestToRemove; public UserInfo(int id, String name, int flags) { this(id, name, null, flags); @@ -147,6 +148,7 @@ public class UserInfo implements Parcelable { lastLoggedInTime = orig.lastLoggedInTime; partial = orig.partial; profileGroupId = orig.profileGroupId; + guestToRemove = orig.guestToRemove; } public UserHandle getUserHandle() { @@ -172,6 +174,7 @@ public class UserInfo implements Parcelable { dest.writeLong(lastLoggedInTime); dest.writeInt(partial ? 1 : 0); dest.writeInt(profileGroupId); + dest.writeInt(guestToRemove ? 1 : 0); } public static final Parcelable.Creator CREATOR @@ -194,5 +197,6 @@ public class UserInfo implements Parcelable { lastLoggedInTime = source.readLong(); partial = source.readInt() != 0; profileGroupId = source.readInt(); + guestToRemove = source.readInt() != 0; } } diff --git a/services/core/java/com/android/server/am/ActiveServices.java b/services/core/java/com/android/server/am/ActiveServices.java index 0bdb964a14092..599c3b9fb20fe 100755 --- a/services/core/java/com/android/server/am/ActiveServices.java +++ b/services/core/java/com/android/server/am/ActiveServices.java @@ -308,7 +308,14 @@ public final class ActiveServices { return new ComponentName("!", res.permission != null ? res.permission : "private to package"); } + ServiceRecord r = res.record; + + if (!mAm.getUserManagerLocked().exists(r.userId)) { + Slog.d(TAG, "Trying to start service with non-existent user! " + r.userId); + return null; + } + NeededUriGrants neededGrants = mAm.checkGrantUriPermissionFromIntentLocked( callingUid, r.packageName, service, service.getFlags(), null, r.userId); if (unscheduleServiceRestartLocked(r, callingUid, false)) { diff --git a/services/core/java/com/android/server/pm/UserManagerService.java b/services/core/java/com/android/server/pm/UserManagerService.java index 4a2cecea17f48..53d8ed0bd0d3f 100644 --- a/services/core/java/com/android/server/pm/UserManagerService.java +++ b/services/core/java/com/android/server/pm/UserManagerService.java @@ -21,7 +21,6 @@ import static android.text.format.DateUtils.MINUTE_IN_MILLIS; import android.app.Activity; import android.app.ActivityManager; import android.app.ActivityManagerNative; -import android.app.ActivityThread; import android.app.IStopUserCallback; import android.content.BroadcastReceiver; import android.content.Context; @@ -92,6 +91,7 @@ public class UserManagerService extends IUserManager.Stub { private static final String ATTR_SERIAL_NO = "serialNumber"; private static final String ATTR_NEXT_SERIAL_NO = "nextSerialNumber"; private static final String ATTR_PARTIAL = "partial"; + private static final String ATTR_GUEST_TO_REMOVE = "guestToRemove"; private static final String ATTR_USER_VERSION = "version"; private static final String ATTR_PROFILE_GROUP_ID = "profileGroupId"; private static final String TAG_GUEST_RESTRICTIONS = "guestRestrictions"; @@ -228,7 +228,7 @@ public class UserManagerService extends IUserManager.Stub { ArrayList partials = new ArrayList(); for (int i = 0; i < mUsers.size(); i++) { UserInfo ui = mUsers.valueAt(i); - if (ui.partial && i != 0) { + if ((ui.partial || ui.guestToRemove) && i != 0) { partials.add(ui); } } @@ -759,6 +759,9 @@ public class UserManagerService extends IUserManager.Stub { if (userInfo.partial) { serializer.attribute(null, ATTR_PARTIAL, "true"); } + if (userInfo.guestToRemove) { + serializer.attribute(null, ATTR_GUEST_TO_REMOVE, "true"); + } if (userInfo.profileGroupId != UserInfo.NO_PROFILE_GROUP_ID) { serializer.attribute(null, ATTR_PROFILE_GROUP_ID, Integer.toString(userInfo.profileGroupId)); @@ -806,7 +809,7 @@ public class UserManagerService extends IUserManager.Stub { serializer.attribute(null, ATTR_NEXT_SERIAL_NO, Integer.toString(mNextSerialNumber)); serializer.attribute(null, ATTR_USER_VERSION, Integer.toString(mUserVersion)); - serializer.startTag(null, TAG_GUEST_RESTRICTIONS); + serializer.startTag(null, TAG_GUEST_RESTRICTIONS); writeRestrictionsLocked(serializer, mGuestRestrictions); serializer.endTag(null, TAG_GUEST_RESTRICTIONS); for (int i = 0; i < mUsers.size(); i++) { @@ -871,6 +874,7 @@ public class UserManagerService extends IUserManager.Stub { int profileGroupId = UserInfo.NO_PROFILE_GROUP_ID; long lastAttemptTime = 0L; boolean partial = false; + boolean guestToRemove = false; Bundle restrictions = new Bundle(); FileInputStream fis = null; @@ -918,6 +922,10 @@ public class UserManagerService extends IUserManager.Stub { if ("true".equals(valueString)) { partial = true; } + valueString = parser.getAttributeValue(null, ATTR_GUEST_TO_REMOVE); + if ("true".equals(valueString)) { + guestToRemove = true; + } int outerDepth = parser.getDepth(); while ((type = parser.next()) != XmlPullParser.END_DOCUMENT @@ -942,6 +950,7 @@ public class UserManagerService extends IUserManager.Stub { userInfo.creationTime = creationTime; userInfo.lastLoggedInTime = lastLoggedInTime; userInfo.partial = partial; + userInfo.guestToRemove = guestToRemove; userInfo.profileGroupId = profileGroupId; mUserRestrictions.append(id, restrictions); if (salt != 0L) { @@ -1119,7 +1128,7 @@ public class UserManagerService extends IUserManager.Stub { return null; } // If we're adding a guest and there already exists one, bail. - if (isGuest && numberOfUsersOfTypeLocked(UserInfo.FLAG_GUEST, true) > 0) { + if (isGuest && findCurrentGuestUserLocked() != null) { return null; } // Limit number of managed profiles that can be created @@ -1179,6 +1188,23 @@ public class UserManagerService extends IUserManager.Stub { return count; } + /** + * Find the current guest user. If the Guest user is partial, + * then do not include it in the results as it is about to die. + * This is different than {@link #numberOfUsersOfTypeLocked(int, boolean)} due to + * the special handling of Guests being removed. + */ + private UserInfo findCurrentGuestUserLocked() { + final int size = mUsers.size(); + for (int i = 0; i < size; i++) { + final UserInfo user = mUsers.valueAt(i); + if (user.isGuest() && !user.guestToRemove && !mRemovingUserIds.get(user.id)) { + return user; + } + } + return null; + } + /** * Mark this guest user for deletion to allow us to create another guest * and switch to that user before actually removing this guest. @@ -1204,14 +1230,15 @@ public class UserManagerService extends IUserManager.Stub { if (!user.isGuest()) { return false; } - // Set this to a partially created user, so that the user will be purged - // on next startup, in case the runtime stops now before stopping and - // removing the user completely. - user.partial = true; + // We set this to a guest user that is to be removed. This is a temporary state + // where we are allowed to add new Guest users, even if this one is still not + // removed. This user will still show up in getUserInfo() calls. + // If we don't get around to removing this Guest user, it will be purged on next + // startup. + user.guestToRemove = true; // Mark it as disabled, so that it isn't returned any more when // profiles are queried. user.flags |= UserInfo.FLAG_DISABLED; - user.flags &= ~UserInfo.FLAG_GUEST; writeUserLocked(user); } } finally { From 6590d7c75c1d706f70ebf6a5594d6a7601c080df Mon Sep 17 00:00:00 2001 From: Adam Lesinski Date: Mon, 8 Sep 2014 18:31:21 -0700 Subject: [PATCH 2/2] Make UsageStats API default on only for the system This changes the PACKAGE_USAGE_STATS permission to be a signature only permission, meaning that only apps signed with the system signature have access to the UsageStats API enabled by default. Bug:17428130 Change-Id: I79ba54d7102c770ea2490f014a1f6736f0f46b3a --- core/res/AndroidManifest.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/res/AndroidManifest.xml b/core/res/AndroidManifest.xml index 72dd93017d811..8a1d5afc05adb 100644 --- a/core/res/AndroidManifest.xml +++ b/core/res/AndroidManifest.xml @@ -2502,7 +2502,7 @@ + android:protectionLevel="signature|development|appop" />