diff options
author | Sudheer Shanka <sudheersai@google.com> | 2021-03-24 03:32:28 -0700 |
---|---|---|
committer | Sudheer Shanka <sudheersai@google.com> | 2021-04-12 16:00:43 -0700 |
commit | 523643c45ca3624d56906d2ee7df0bcb766db562 (patch) | |
tree | c625fc402c2e247d5abd77681d58b72da7c7962b | |
parent | 086a0be6b2a360f9c823a596cba50867101a5a0f (diff) |
Allow certain apps to access blobs across users.
BlobStoreManagerService is updated to store only
one copy of any blob on device and to allow apps
with new ACCESS_BLOBS_ACROSS_USERS priv permission
to access blobs commited by their instances on other
users.
Bug: 175844032
Test: atest --test-mapping apex/blobstore
Change-Id: If6d10d90b17bde33baf9e83eeaae75d09d0b50ae
8 files changed, 381 insertions, 268 deletions
diff --git a/apex/blobstore/framework/java/android/app/blob/BlobHandle.java b/apex/blobstore/framework/java/android/app/blob/BlobHandle.java index 113f8fe9e248..6dbbcb564b4c 100644 --- a/apex/blobstore/framework/java/android/app/blob/BlobHandle.java +++ b/apex/blobstore/framework/java/android/app/blob/BlobHandle.java @@ -26,8 +26,8 @@ import android.annotation.NonNull; import android.os.Parcel; import android.os.Parcelable; import android.util.Base64; +import android.util.IndentingPrintWriter; -import com.android.internal.util.IndentingPrintWriter; import com.android.internal.util.Preconditions; import com.android.internal.util.XmlUtils; diff --git a/apex/blobstore/service/java/com/android/server/blob/BlobAccessMode.java b/apex/blobstore/service/java/com/android/server/blob/BlobAccessMode.java index ca588c509594..09260b775444 100644 --- a/apex/blobstore/service/java/com/android/server/blob/BlobAccessMode.java +++ b/apex/blobstore/service/java/com/android/server/blob/BlobAccessMode.java @@ -37,9 +37,9 @@ import android.permission.PermissionManager; import android.util.ArraySet; import android.util.Base64; import android.util.DebugUtils; +import android.util.IndentingPrintWriter; import android.util.Slog; -import com.android.internal.util.IndentingPrintWriter; import com.android.internal.util.XmlUtils; import org.xmlpull.v1.XmlPullParser; diff --git a/apex/blobstore/service/java/com/android/server/blob/BlobMetadata.java b/apex/blobstore/service/java/com/android/server/blob/BlobMetadata.java index 8b12beb57195..e47715685323 100644 --- a/apex/blobstore/service/java/com/android/server/blob/BlobMetadata.java +++ b/apex/blobstore/service/java/com/android/server/blob/BlobMetadata.java @@ -15,6 +15,7 @@ */ package com.android.server.blob; +import static android.Manifest.permission.ACCESS_BLOBS_ACROSS_USERS; import static android.app.blob.XmlTags.ATTR_COMMIT_TIME_MS; import static android.app.blob.XmlTags.ATTR_DESCRIPTION; import static android.app.blob.XmlTags.ATTR_DESCRIPTION_RES_NAME; @@ -36,6 +37,7 @@ import static com.android.server.blob.BlobStoreConfig.TAG; import static com.android.server.blob.BlobStoreConfig.XML_VERSION_ADD_COMMIT_TIME; import static com.android.server.blob.BlobStoreConfig.XML_VERSION_ADD_DESC_RES_NAME; import static com.android.server.blob.BlobStoreConfig.XML_VERSION_ADD_STRING_DESC; +import static com.android.server.blob.BlobStoreConfig.XML_VERSION_ALLOW_ACCESS_ACROSS_USERS; import static com.android.server.blob.BlobStoreConfig.hasLeaseWaitTimeElapsed; import static com.android.server.blob.BlobStoreUtils.getDescriptionResourceId; import static com.android.server.blob.BlobStoreUtils.getPackageResources; @@ -45,15 +47,18 @@ import android.annotation.Nullable; import android.app.blob.BlobHandle; import android.app.blob.LeaseInfo; import android.content.Context; +import android.content.pm.PackageManager; import android.content.res.ResourceId; import android.content.res.Resources; import android.os.ParcelFileDescriptor; import android.os.RevocableFileDescriptor; import android.os.UserHandle; +import android.permission.PermissionManager; import android.system.ErrnoException; import android.system.Os; import android.util.ArrayMap; import android.util.ArraySet; +import android.util.IndentingPrintWriter; import android.util.Slog; import android.util.SparseArray; import android.util.StatsEvent; @@ -62,7 +67,6 @@ import android.util.proto.ProtoOutputStream; import com.android.internal.annotations.GuardedBy; import com.android.internal.annotations.VisibleForTesting; import com.android.internal.util.FrameworkStatsLog; -import com.android.internal.util.IndentingPrintWriter; import com.android.internal.util.XmlUtils; import com.android.server.blob.BlobStoreManagerService.DumpArgs; @@ -85,7 +89,6 @@ class BlobMetadata { private final long mBlobId; private final BlobHandle mBlobHandle; - private final int mUserId; @GuardedBy("mMetadataLock") private final ArraySet<Committer> mCommitters = new ArraySet<>(); @@ -94,24 +97,23 @@ class BlobMetadata { private final ArraySet<Leasee> mLeasees = new ArraySet<>(); /** - * Contains packageName -> {RevocableFileDescriptors}. + * Contains Accessor -> {RevocableFileDescriptors}. * * Keep track of RevocableFileDescriptors given to clients which are not yet revoked/closed so * that when clients access is revoked or the blob gets deleted, we can be sure that clients * do not have any reference to the blob and the space occupied by the blob can be freed. */ @GuardedBy("mRevocableFds") - private final ArrayMap<String, ArraySet<RevocableFileDescriptor>> mRevocableFds = + private final ArrayMap<Accessor, ArraySet<RevocableFileDescriptor>> mRevocableFds = new ArrayMap<>(); // Do not access this directly, instead use #getBlobFile(). private File mBlobFile; - BlobMetadata(Context context, long blobId, BlobHandle blobHandle, int userId) { + BlobMetadata(Context context, long blobId, BlobHandle blobHandle) { mContext = context; this.mBlobId = blobId; this.mBlobHandle = blobHandle; - this.mUserId = userId; } long getBlobId() { @@ -122,10 +124,6 @@ class BlobMetadata { return mBlobHandle; } - int getUserId() { - return mUserId; - } - void addOrReplaceCommitter(@NonNull Committer committer) { synchronized (mMetadataLock) { // We need to override the committer data, so first remove any existing @@ -155,13 +153,24 @@ class BlobMetadata { } } - void removeCommittersFromUnknownPkgs(SparseArray<String> knownPackages) { + void removeCommittersFromUnknownPkgs(SparseArray<SparseArray<String>> knownPackages) { synchronized (mMetadataLock) { - mCommitters.removeIf(committer -> - !committer.packageName.equals(knownPackages.get(committer.uid))); + mCommitters.removeIf(committer -> { + final int userId = UserHandle.getUserId(committer.uid); + final SparseArray<String> userPackages = knownPackages.get(userId); + if (userPackages == null) { + return true; + } + return !committer.packageName.equals(userPackages.get(committer.uid)); + }); } } + void addCommittersAndLeasees(BlobMetadata blobMetadata) { + mCommitters.addAll(blobMetadata.mCommitters); + mLeasees.addAll(blobMetadata.mLeasees); + } + @Nullable Committer getExistingCommitter(@NonNull String packageName, int uid) { synchronized (mCommitters) { @@ -201,10 +210,16 @@ class BlobMetadata { } } - void removeLeaseesFromUnknownPkgs(SparseArray<String> knownPackages) { + void removeLeaseesFromUnknownPkgs(SparseArray<SparseArray<String>> knownPackages) { synchronized (mMetadataLock) { - mLeasees.removeIf(leasee -> - !leasee.packageName.equals(knownPackages.get(leasee.uid))); + mLeasees.removeIf(leasee -> { + final int userId = UserHandle.getUserId(leasee.uid); + final SparseArray<String> userPackages = knownPackages.get(userId); + if (userPackages == null) { + return true; + } + return !leasee.packageName.equals(userPackages.get(leasee.uid)); + }); } } @@ -214,6 +229,25 @@ class BlobMetadata { } } + void removeDataForUser(int userId) { + synchronized (mMetadataLock) { + mCommitters.removeIf(committer -> (userId == UserHandle.getUserId(committer.uid))); + mLeasees.removeIf(leasee -> (userId == UserHandle.getUserId(leasee.uid))); + mRevocableFds.entrySet().removeIf(entry -> { + final Accessor accessor = entry.getKey(); + final ArraySet<RevocableFileDescriptor> rFds = entry.getValue(); + if (userId != UserHandle.getUserId(accessor.uid)) { + return false; + } + for (int i = 0, fdCount = rFds.size(); i < fdCount; ++i) { + rFds.valueAt(i).revoke(); + } + rFds.clear(); + return true; + }); + } + } + boolean hasValidLeases() { synchronized (mMetadataLock) { for (int i = 0, size = mLeasees.size(); i < size; ++i) { @@ -244,8 +278,12 @@ class BlobMetadata { } } + final int callingUserId = UserHandle.getUserId(callingUid); for (int i = 0, size = mCommitters.size(); i < size; ++i) { final Committer committer = mCommitters.valueAt(i); + if (callingUserId != UserHandle.getUserId(committer.uid)) { + continue; + } // Check if the caller is the same package that committed the blob. if (committer.equals(callingPackage, callingUid)) { @@ -259,38 +297,105 @@ class BlobMetadata { return true; } } + + final boolean canCallerAccessBlobsAcrossUsers = checkCallerCanAccessBlobsAcrossUsers( + callingPackage, callingUserId); + if (!canCallerAccessBlobsAcrossUsers) { + return false; + } + for (int i = 0, size = mCommitters.size(); i < size; ++i) { + final Committer committer = mCommitters.valueAt(i); + final int committerUserId = UserHandle.getUserId(committer.uid); + if (callingUserId == committerUserId) { + continue; + } + if (!checkCallerCanAccessBlobsAcrossUsers(callingPackage, committerUserId)) { + continue; + } + + // Check if the caller is allowed access as per the access mode specified + // by the committer. + if (committer.blobAccessMode.isAccessAllowedForCaller(mContext, + callingPackage, committer.packageName, callingUid, attributionTag)) { + return true; + } + } + + } + return false; + } + + private static boolean checkCallerCanAccessBlobsAcrossUsers( + String callingPackage, int callingUserId) { + return PermissionManager.checkPackageNamePermission(ACCESS_BLOBS_ACROSS_USERS, + callingPackage, callingUserId) == PackageManager.PERMISSION_GRANTED; + } + + boolean hasACommitterOrLeaseeInUser(int userId) { + return hasACommitterInUser(userId) || hasALeaseeInUser(userId); + } + + boolean hasACommitterInUser(int userId) { + synchronized (mMetadataLock) { + for (int i = 0, size = mCommitters.size(); i < size; ++i) { + final Committer committer = mCommitters.valueAt(i); + if (userId == UserHandle.getUserId(committer.uid)) { + return true; + } + } + } + return false; + } + + private boolean hasALeaseeInUser(int userId) { + synchronized (mMetadataLock) { + for (int i = 0, size = mLeasees.size(); i < size; ++i) { + final Leasee leasee = mLeasees.valueAt(i); + if (userId == UserHandle.getUserId(leasee.uid)) { + return true; + } + } } return false; } boolean isACommitter(@NonNull String packageName, int uid) { synchronized (mMetadataLock) { - return isAnAccessor(mCommitters, packageName, uid); + return isAnAccessor(mCommitters, packageName, uid, UserHandle.getUserId(uid)); } } boolean isALeasee(@Nullable String packageName, int uid) { synchronized (mMetadataLock) { - final Leasee leasee = getAccessor(mLeasees, packageName, uid); + final Leasee leasee = getAccessor(mLeasees, packageName, uid, + UserHandle.getUserId(uid)); + return leasee != null && leasee.isStillValid(); + } + } + + private boolean isALeaseeInUser(@Nullable String packageName, int uid, int userId) { + synchronized (mMetadataLock) { + final Leasee leasee = getAccessor(mLeasees, packageName, uid, userId); return leasee != null && leasee.isStillValid(); } } private static <T extends Accessor> boolean isAnAccessor(@NonNull ArraySet<T> accessors, - @Nullable String packageName, int uid) { + @Nullable String packageName, int uid, int userId) { // Check if the package is an accessor of the data blob. - return getAccessor(accessors, packageName, uid) != null; + return getAccessor(accessors, packageName, uid, userId) != null; } private static <T extends Accessor> T getAccessor(@NonNull ArraySet<T> accessors, - @Nullable String packageName, int uid) { + @Nullable String packageName, int uid, int userId) { // Check if the package is an accessor of the data blob. for (int i = 0, size = accessors.size(); i < size; ++i) { final Accessor accessor = accessors.valueAt(i); if (packageName != null && uid != INVALID_UID && accessor.equals(packageName, uid)) { return (T) accessor; - } else if (packageName != null && accessor.packageName.equals(packageName)) { + } else if (packageName != null && accessor.packageName.equals(packageName) + && userId == UserHandle.getUserId(accessor.uid)) { return (T) accessor; } else if (uid != INVALID_UID && accessor.uid == uid) { return (T) accessor; @@ -299,23 +404,29 @@ class BlobMetadata { return null; } - boolean isALeasee(@NonNull String packageName) { - return isALeasee(packageName, INVALID_UID); - } - - boolean isALeasee(int uid) { - return isALeasee(null, uid); - } - - boolean hasOtherLeasees(@NonNull String packageName) { - return hasOtherLeasees(packageName, INVALID_UID); + boolean shouldAttributeToLeasee(@NonNull String packageName, int userId, + boolean callerHasStatsPermission) { + if (!isALeaseeInUser(packageName, INVALID_UID, userId)) { + return false; + } + if (!callerHasStatsPermission || !hasOtherLeasees(packageName, INVALID_UID, userId)) { + return true; + } + return false; } - boolean hasOtherLeasees(int uid) { - return hasOtherLeasees(null, uid); + boolean shouldAttributeToLeasee(int uid, boolean callerHasStatsPermission) { + final int userId = UserHandle.getUserId(uid); + if (!isALeaseeInUser(null, uid, userId)) { + return false; + } + if (!callerHasStatsPermission || !hasOtherLeasees(null, uid, userId)) { + return true; + } + return false; } - private boolean hasOtherLeasees(@Nullable String packageName, int uid) { + private boolean hasOtherLeasees(@Nullable String packageName, int uid, int userId) { synchronized (mMetadataLock) { for (int i = 0, size = mLeasees.size(); i < size; ++i) { final Leasee leasee = mLeasees.valueAt(i); @@ -326,7 +437,8 @@ class BlobMetadata { if (packageName != null && uid != INVALID_UID && !leasee.equals(packageName, uid)) { return true; - } else if (packageName != null && !leasee.packageName.equals(packageName)) { + } else if (packageName != null && (!leasee.packageName.equals(packageName) + || userId != UserHandle.getUserId(leasee.uid))) { return true; } else if (uid != INVALID_UID && leasee.uid != uid) { return true; @@ -371,7 +483,7 @@ class BlobMetadata { return mBlobFile; } - ParcelFileDescriptor openForRead(String callingPackage) throws IOException { + ParcelFileDescriptor openForRead(String callingPackage, int callingUid) throws IOException { // TODO: Add limit on opened fds FileDescriptor fd; try { @@ -381,7 +493,7 @@ class BlobMetadata { } try { if (BlobStoreConfig.shouldUseRevocableFdForReads()) { - return createRevocableFd(fd, callingPackage); + return createRevocableFd(fd, callingPackage, callingUid); } else { return new ParcelFileDescriptor(fd); } @@ -393,26 +505,28 @@ class BlobMetadata { @NonNull private ParcelFileDescriptor createRevocableFd(FileDescriptor fd, - String callingPackage) throws IOException { + String callingPackage, int callingUid) throws IOException { final RevocableFileDescriptor revocableFd = new RevocableFileDescriptor(mContext, fd); + final Accessor accessor; synchronized (mRevocableFds) { - ArraySet<RevocableFileDescriptor> revocableFdsForPkg = - mRevocableFds.get(callingPackage); - if (revocableFdsForPkg == null) { - revocableFdsForPkg = new ArraySet<>(); - mRevocableFds.put(callingPackage, revocableFdsForPkg); + accessor = new Accessor(callingPackage, callingUid); + ArraySet<RevocableFileDescriptor> revocableFdsForAccessor = + mRevocableFds.get(accessor); + if (revocableFdsForAccessor == null) { + revocableFdsForAccessor = new ArraySet<>(); + mRevocableFds.put(accessor, revocableFdsForAccessor); } - revocableFdsForPkg.add(revocableFd); + revocableFdsForAccessor.add(revocableFd); } revocableFd.addOnCloseListener((e) -> { synchronized (mRevocableFds) { - final ArraySet<RevocableFileDescriptor> revocableFdsForPkg = - mRevocableFds.get(callingPackage); - if (revocableFdsForPkg != null) { - revocableFdsForPkg.remove(revocableFd); - if (revocableFdsForPkg.isEmpty()) { - mRevocableFds.remove(callingPackage); + final ArraySet<RevocableFileDescriptor> revocableFdsForAccessor = + mRevocableFds.get(accessor); + if (revocableFdsForAccessor != null) { + revocableFdsForAccessor.remove(revocableFd); + if (revocableFdsForAccessor.isEmpty()) { + mRevocableFds.remove(accessor); } } } @@ -421,22 +535,23 @@ class BlobMetadata { } void destroy() { - revokeAllFds(); + revokeAndClearAllFds(); getBlobFile().delete(); } - private void revokeAllFds() { + private void revokeAndClearAllFds() { synchronized (mRevocableFds) { - for (int i = 0, pkgCount = mRevocableFds.size(); i < pkgCount; ++i) { - final ArraySet<RevocableFileDescriptor> packageFds = + for (int i = 0, accessorCount = mRevocableFds.size(); i < accessorCount; ++i) { + final ArraySet<RevocableFileDescriptor> rFds = mRevocableFds.valueAt(i); - if (packageFds == null) { + if (rFds == null) { continue; } - for (int j = 0, fdCount = packageFds.size(); j < fdCount; ++j) { - packageFds.valueAt(j).revoke(); + for (int j = 0, fdCount = rFds.size(); j < fdCount; ++j) { + rFds.valueAt(j).revoke(); } } + mRevocableFds.clear(); } } @@ -547,10 +662,10 @@ class BlobMetadata { fout.println("<empty>"); } else { for (int i = 0, count = mRevocableFds.size(); i < count; ++i) { - final String packageName = mRevocableFds.keyAt(i); - final ArraySet<RevocableFileDescriptor> packageFds = + final Accessor accessor = mRevocableFds.keyAt(i); + final ArraySet<RevocableFileDescriptor> rFds = mRevocableFds.valueAt(i); - fout.println(packageName + "#" + packageFds.size()); + fout.println(accessor + ": #" + rFds.size()); } } fout.decreaseIndent(); @@ -560,7 +675,6 @@ class BlobMetadata { void writeToXml(XmlSerializer out) throws IOException { synchronized (mMetadataLock) { XmlUtils.writeLongAttribute(out, ATTR_ID, mBlobId); - XmlUtils.writeIntAttribute(out, ATTR_USER_ID, mUserId); out.startTag(null, TAG_BLOB_HANDLE); mBlobHandle.writeToXml(out); @@ -584,7 +698,9 @@ class BlobMetadata { static BlobMetadata createFromXml(XmlPullParser in, int version, Context context) throws XmlPullParserException, IOException { final long blobId = XmlUtils.readLongAttribute(in, ATTR_ID); - final int userId = XmlUtils.readIntAttribute(in, ATTR_USER_ID); + if (version < XML_VERSION_ALLOW_ACCESS_ACROSS_USERS) { + XmlUtils.readIntAttribute(in, ATTR_USER_ID); + } BlobHandle blobHandle = null; final ArraySet<Committer> committers = new ArraySet<>(); @@ -608,7 +724,7 @@ class BlobMetadata { return null; } - final BlobMetadata blobMetadata = new BlobMetadata(context, blobId, blobHandle, userId); + final BlobMetadata blobMetadata = new BlobMetadata(context, blobId, blobHandle); blobMetadata.setCommitters(committers); blobMetadata.setLeasees(leasees); return blobMetadata; diff --git a/apex/blobstore/service/java/com/android/server/blob/BlobStoreConfig.java b/apex/blobstore/service/java/com/android/server/blob/BlobStoreConfig.java index 5cebf8d91cfc..502b29eb1a1f 100644 --- a/apex/blobstore/service/java/com/android/server/blob/BlobStoreConfig.java +++ b/apex/blobstore/service/java/com/android/server/blob/BlobStoreConfig.java @@ -27,12 +27,11 @@ import android.provider.DeviceConfig; import android.provider.DeviceConfig.Properties; import android.text.TextUtils; import android.util.DataUnit; +import android.util.IndentingPrintWriter; import android.util.Log; import android.util.Slog; import android.util.TimeUtils; -import com.android.internal.util.IndentingPrintWriter; - import java.io.File; import java.util.concurrent.TimeUnit; @@ -47,8 +46,9 @@ class BlobStoreConfig { public static final int XML_VERSION_ADD_DESC_RES_NAME = 3; public static final int XML_VERSION_ADD_COMMIT_TIME = 4; public static final int XML_VERSION_ADD_SESSION_CREATION_TIME = 5; + public static final int XML_VERSION_ALLOW_ACCESS_ACROSS_USERS = 6; - public static final int XML_VERSION_CURRENT = XML_VERSION_ADD_SESSION_CREATION_TIME; + public static final int XML_VERSION_CURRENT = XML_VERSION_ALLOW_ACCESS_ACROSS_USERS; public static final long INVALID_BLOB_ID = 0; public static final long INVALID_BLOB_SIZE = 0; diff --git a/apex/blobstore/service/java/com/android/server/blob/BlobStoreManagerService.java b/apex/blobstore/service/java/com/android/server/blob/BlobStoreManagerService.java index 0e7354726123..cc5e31a91123 100644 --- a/apex/blobstore/service/java/com/android/server/blob/BlobStoreManagerService.java +++ b/apex/blobstore/service/java/com/android/server/blob/BlobStoreManagerService.java @@ -32,6 +32,7 @@ import static com.android.server.blob.BlobStoreConfig.INVALID_BLOB_ID; import static com.android.server.blob.BlobStoreConfig.INVALID_BLOB_SIZE; import static com.android.server.blob.BlobStoreConfig.LOGV; import static com.android.server.blob.BlobStoreConfig.TAG; +import static com.android.server.blob.BlobStoreConfig.XML_VERSION_ALLOW_ACCESS_ACROSS_USERS; import static com.android.server.blob.BlobStoreConfig.XML_VERSION_CURRENT; import static com.android.server.blob.BlobStoreConfig.getAdjustedCommitTimeMs; import static com.android.server.blob.BlobStoreConfig.getDeletionOnLastLeaseDelayMs; @@ -83,6 +84,7 @@ import android.util.ArrayMap; import android.util.ArraySet; import android.util.AtomicFile; import android.util.ExceptionUtils; +import android.util.IndentingPrintWriter; import android.util.LongSparseArray; import android.util.Slog; import android.util.SparseArray; @@ -96,7 +98,6 @@ import com.android.internal.util.CollectionUtils; import com.android.internal.util.DumpUtils; import com.android.internal.util.FastXmlSerializer; import com.android.internal.util.FrameworkStatsLog; -import com.android.internal.util.IndentingPrintWriter; import com.android.internal.util.Preconditions; import com.android.internal.util.XmlUtils; import com.android.internal.util.function.pooled.PooledLambda; @@ -129,6 +130,7 @@ import java.util.Random; import java.util.Set; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicLong; +import java.util.function.BiConsumer; import java.util.function.Consumer; import java.util.function.Function; @@ -146,9 +148,9 @@ public class BlobStoreManagerService extends SystemService { @GuardedBy("mBlobsLock") private long mCurrentMaxSessionId; - // Contains data of userId -> {BlobHandle -> {BlobMetadata}} + // Contains data of BlobHandle -> BlobMetadata. @GuardedBy("mBlobsLock") - private final SparseArray<ArrayMap<BlobHandle, BlobMetadata>> mBlobsMap = new SparseArray<>(); + private final ArrayMap<BlobHandle, BlobMetadata> mBlobsMap = new ArrayMap<>(); // Contains all ids that are currently in use. @GuardedBy("mBlobsLock") @@ -265,27 +267,24 @@ public class BlobStoreManagerService extends SystemService { return userSessions; } - @GuardedBy("mBlobsLock") - private ArrayMap<BlobHandle, BlobMetadata> getUserBlobsLocked(int userId) { - ArrayMap<BlobHandle, BlobMetadata> userBlobs = mBlobsMap.get(userId); - if (userBlobs == null) { - userBlobs = new ArrayMap<>(); - mBlobsMap.put(userId, userBlobs); + @VisibleForTesting + void addUserSessionsForTest(LongSparseArray<BlobStoreSession> userSessions, int userId) { + synchronized (mBlobsLock) { + mSessions.put(userId, userSessions); } - return userBlobs; } @VisibleForTesting - void addUserSessionsForTest(LongSparseArray<BlobStoreSession> userSessions, int userId) { + BlobMetadata getBlobForTest(BlobHandle blobHandle) { synchronized (mBlobsLock) { - mSessions.put(userId, userSessions); + return mBlobsMap.get(blobHandle); } } @VisibleForTesting - void addUserBlobsForTest(ArrayMap<BlobHandle, BlobMetadata> userBlobs, int userId) { + int getBlobsCountForTest() { synchronized (mBlobsLock) { - mBlobsMap.put(userId, userBlobs); + return mBlobsMap.size(); } } @@ -319,14 +318,9 @@ public class BlobStoreManagerService extends SystemService { } @GuardedBy("mBlobsLock") - private void addBlobForUserLocked(BlobMetadata blobMetadata, int userId) { - addBlobForUserLocked(blobMetadata, getUserBlobsLocked(userId)); - } - - @GuardedBy("mBlobsLock") - private void addBlobForUserLocked(BlobMetadata blobMetadata, - ArrayMap<BlobHandle, BlobMetadata> userBlobs) { - userBlobs.put(blobMetadata.getBlobHandle(), blobMetadata); + @VisibleForTesting + void addBlobLocked(BlobMetadata blobMetadata) { + mBlobsMap.put(blobMetadata.getBlobHandle(), blobMetadata); addActiveBlobIdLocked(blobMetadata.getBlobId()); } @@ -404,8 +398,7 @@ public class BlobStoreManagerService extends SystemService { private ParcelFileDescriptor openBlobInternal(BlobHandle blobHandle, int callingUid, String callingPackage, String attributionTag) throws IOException { synchronized (mBlobsLock) { - final BlobMetadata blobMetadata = getUserBlobsLocked(UserHandle.getUserId(callingUid)) - .get(blobHandle); + final BlobMetadata blobMetadata = mBlobsMap.get(blobHandle); if (blobMetadata == null || !blobMetadata.isAccessAllowedForCaller( callingPackage, callingUid, attributionTag)) { if (blobMetadata == null) { @@ -415,7 +408,7 @@ public class BlobStoreManagerService extends SystemService { } else { FrameworkStatsLog.write(FrameworkStatsLog.BLOB_OPENED, callingUid, blobMetadata.getBlobId(), blobMetadata.getSize(), - FrameworkStatsLog.BLOB_LEASED__RESULT__ACCESS_NOT_ALLOWED); + FrameworkStatsLog.BLOB_OPENED__RESULT__ACCESS_NOT_ALLOWED); } throw new SecurityException("Caller not allowed to access " + blobHandle + "; callingUid=" + callingUid + ", callingPackage=" + callingPackage); @@ -425,7 +418,7 @@ public class BlobStoreManagerService extends SystemService { blobMetadata.getBlobId(), blobMetadata.getSize(), FrameworkStatsLog.BLOB_OPENED__RESULT__SUCCESS); - return blobMetadata.openForRead(callingPackage); + return blobMetadata.openForRead(callingPackage, callingUid); } } @@ -433,11 +426,11 @@ public class BlobStoreManagerService extends SystemService { private int getCommittedBlobsCountLocked(int uid, String packageName) { // TODO: Maintain a counter instead of traversing all the blobs final AtomicInteger blobsCount = new AtomicInteger(0); - forEachBlobInUser((blobMetadata) -> { + forEachBlobLocked(blobMetadata -> { if (blobMetadata.isACommitter(packageName, uid)) { blobsCount.getAndIncrement(); } - }, UserHandle.getUserId(uid)); + }); return blobsCount.get(); } @@ -445,11 +438,11 @@ public class BlobStoreManagerService extends SystemService { private int getLeasedBlobsCountLocked(int uid, String packageName) { // TODO: Maintain a counter instead of traversing all the blobs final AtomicInteger blobsCount = new AtomicInteger(0); - forEachBlobInUser((blobMetadata) -> { + forEachBlobLocked(blobMetadata -> { if (blobMetadata.isALeasee(packageName, uid)) { blobsCount.getAndIncrement(); } - }, UserHandle.getUserId(uid)); + }); return blobsCount.get(); } @@ -465,8 +458,16 @@ public class BlobStoreManagerService extends SystemService { throw new LimitExceededException("Too many leased blobs for the caller: " + leasesCount); } - final BlobMetadata blobMetadata = getUserBlobsLocked(UserHandle.getUserId(callingUid)) - .get(blobHandle); + if (leaseExpiryTimeMillis != 0 && blobHandle.expiryTimeMillis != 0 + && leaseExpiryTimeMillis > blobHandle.expiryTimeMillis) { + FrameworkStatsLog.write(FrameworkStatsLog.BLOB_LEASED, callingUid, + INVALID_BLOB_ID, INVALID_BLOB_SIZE, + FrameworkStatsLog.BLOB_LEASED__RESULT__LEASE_EXPIRY_INVALID); + throw new IllegalArgumentException( + "Lease expiry cannot be later than blobs expiry time"); + } + + final BlobMetadata blobMetadata = mBlobsMap.get(blobHandle); if (blobMetadata == null || !blobMetadata.isAccessAllowedForCaller( callingPackage, callingUid, attributionTag)) { if (blobMetadata == null) { @@ -481,15 +482,7 @@ public class BlobStoreManagerService extends SystemService { throw new SecurityException("Caller not allowed to access " + blobHandle + "; callingUid=" + callingUid + ", callingPackage=" + callingPackage); } - if (leaseExpiryTimeMillis != 0 && blobHandle.expiryTimeMillis != 0 - && leaseExpiryTimeMillis > blobHandle.expiryTimeMillis) { - FrameworkStatsLog.write(FrameworkStatsLog.BLOB_LEASED, callingUid, - blobMetadata.getBlobId(), blobMetadata.getSize(), - FrameworkStatsLog.BLOB_LEASED__RESULT__LEASE_EXPIRY_INVALID); - throw new IllegalArgumentException( - "Lease expiry cannot be later than blobs expiry time"); - } if (blobMetadata.getSize() > getRemainingLeaseQuotaBytesInternal(callingUid, callingPackage)) { @@ -518,20 +511,18 @@ public class BlobStoreManagerService extends SystemService { @GuardedBy("mBlobsLock") long getTotalUsageBytesLocked(int callingUid, String callingPackage) { final AtomicLong totalBytes = new AtomicLong(0); - forEachBlobInUser((blobMetadata) -> { + forEachBlobLocked((blobMetadata) -> { if (blobMetadata.isALeasee(callingPackage, callingUid)) { totalBytes.getAndAdd(blobMetadata.getSize()); } - }, UserHandle.getUserId(callingUid)); + }); return totalBytes.get(); } private void releaseLeaseInternal(BlobHandle blobHandle, int callingUid, String callingPackage, String attributionTag) { synchronized (mBlobsLock) { - final ArrayMap<BlobHandle, BlobMetadata> userBlobs = - getUserBlobsLocked(UserHandle.getUserId(callingUid)); - final BlobMetadata blobMetadata = userBlobs.get(blobHandle); + final BlobMetadata blobMetadata = mBlobsMap.get(blobHandle); if (blobMetadata == null || !blobMetadata.isAccessAllowedForCaller( callingPackage, callingUid, attributionTag)) { throw new SecurityException("Caller not allowed to access " + blobHandle @@ -547,12 +538,12 @@ public class BlobStoreManagerService extends SystemService { synchronized (mBlobsLock) { // Check if blobMetadata object is still valid. If it is not, then // it means that it was already deleted and nothing else to do here. - if (!Objects.equals(userBlobs.get(blobHandle), blobMetadata)) { + if (!Objects.equals(mBlobsMap.get(blobHandle), blobMetadata)) { return; } if (blobMetadata.shouldBeDeleted(true /* respectLeaseWaitTime */)) { deleteBlobLocked(blobMetadata); - userBlobs.remove(blobHandle); + mBlobsMap.remove(blobHandle); } writeBlobsInfoAsync(); } @@ -583,12 +574,18 @@ public class BlobStoreManagerService extends SystemService { } return packageResources; }; - getUserBlobsLocked(userId).forEach((blobHandle, blobMetadata) -> { + forEachBlobLocked((blobHandle, blobMetadata) -> { + if (!blobMetadata.hasACommitterOrLeaseeInUser(userId)) { + return; + } final ArrayList<LeaseInfo> leaseInfos = new ArrayList<>(); blobMetadata.forEachLeasee(leasee -> { if (!leasee.isStillValid()) { return; } + if (userId != UserHandle.getUserId(leasee.uid)) { + return; + } final int descriptionResId = leasee.descriptionResEntryName == null ? Resources.ID_NULL : getDescriptionResourceId(resourcesGetter.apply(leasee.packageName), @@ -608,9 +605,7 @@ public class BlobStoreManagerService extends SystemService { private void deleteBlobInternal(long blobId, int callingUid) { synchronized (mBlobsLock) { - final ArrayMap<BlobHandle, BlobMetadata> userBlobs = getUserBlobsLocked( - UserHandle.getUserId(callingUid)); - userBlobs.entrySet().removeIf(entry -> { + mBlobsMap.entrySet().removeIf(entry -> { final BlobMetadata blobMetadata = entry.getValue(); if (blobMetadata.getBlobId() == blobId) { deleteBlobLocked(blobMetadata); @@ -625,19 +620,20 @@ public class BlobStoreManagerService extends SystemService { private List<BlobHandle> getLeasedBlobsInternal(int callingUid, @NonNull String callingPackage) { final ArrayList<BlobHandle> leasedBlobs = new ArrayList<>(); - forEachBlobInUser(blobMetadata -> { - if (blobMetadata.isALeasee(callingPackage, callingUid)) { - leasedBlobs.add(blobMetadata.getBlobHandle()); - } - }, UserHandle.getUserId(callingUid)); + synchronized (mBlobsLock) { + forEachBlobLocked(blobMetadata -> { + if (blobMetadata.isALeasee(callingPackage, callingUid)) { + leasedBlobs.add(blobMetadata.getBlobHandle()); + } + }); + } return leasedBlobs; } private LeaseInfo getLeaseInfoInternal(BlobHandle blobHandle, int callingUid, @NonNull String callingPackage, String attributionTag) { synchronized (mBlobsLock) { - final BlobMetadata blobMetadata = getUserBlobsLocked(UserHandle.getUserId(callingUid)) - .get(blobHandle); + final BlobMetadata blobMetadata = mBlobsMap.get(blobHandle); if (blobMetadata == null || !blobMetadata.isAccessAllowedForCaller( callingPackage, callingUid, attributionTag)) { throw new SecurityException("Caller not allowed to access " + blobHandle @@ -699,14 +695,14 @@ public class BlobStoreManagerService extends SystemService { FrameworkStatsLog.BLOB_COMMITTED__RESULT__COUNT_LIMIT_EXCEEDED); break; } - final int userId = UserHandle.getUserId(session.getOwnerUid()); - final ArrayMap<BlobHandle, BlobMetadata> userBlobs = getUserBlobsLocked( - userId); - BlobMetadata blob = userBlobs.get(session.getBlobHandle()); - if (blob == null) { + final BlobMetadata blob; + final int blobIndex = mBlobsMap.indexOfKey(session.getBlobHandle()); + if (blobIndex >= 0) { + blob = mBlobsMap.valueAt(blobIndex); + } else { blob = new BlobMetadata(mContext, session.getSessionId(), - session.getBlobHandle(), userId); - addBlobForUserLocked(blob, userBlobs); + session.getBlobHandle()); + addBlobLocked(blob); } final Committer existingCommitter = blob.getExistingCommitter( session.getOwnerPackageName(), session.getOwnerUid()); @@ -738,7 +734,7 @@ public class BlobStoreManagerService extends SystemService { // But if it is a recommit, just leave it as is. if (session.getSessionId() == blob.getBlobId()) { deleteBlobLocked(blob); - userBlobs.remove(blob.getBlobHandle()); + mBlobsMap.remove(blob.getBlobHandle()); } } // Delete redundant data from recommits. @@ -874,13 +870,10 @@ public class BlobStoreManagerService extends SystemService { out.startTag(null, TAG_BLOBS); XmlUtils.writeIntAttribute(out, ATTR_VERSION, XML_VERSION_CURRENT); - for (int i = 0, userCount = mBlobsMap.size(); i < userCount; ++i) { - final ArrayMap<BlobHandle, BlobMetadata> userBlobs = mBlobsMap.valueAt(i); - for (int j = 0, blobsCount = userBlobs.size(); j < blobsCount; ++j) { - out.startTag(null, TAG_BLOB); - userBlobs.valueAt(j).writeToXml(out); - out.endTag(null, TAG_BLOB); - } + for (int i = 0, count = mBlobsMap.size(); i < count; ++i) { + out.startTag(null, TAG_BLOB); + mBlobsMap.valueAt(i).writeToXml(out); + out.endTag(null, TAG_BLOB); } out.endTag(null, TAG_BLOBS); @@ -925,16 +918,21 @@ public class BlobStoreManagerService extends SystemService { if (TAG_BLOB.equals(in.getName())) { final BlobMetadata blobMetadata = BlobMetadata.createFromXml( in, version, mContext); - final SparseArray<String> userPackages = allPackages.get( - blobMetadata.getUserId()); - if (userPackages == null) { - blobMetadata.getBlobFile().delete(); + blobMetadata.removeCommittersFromUnknownPkgs(allPackages); + blobMetadata.removeLeaseesFromUnknownPkgs(allPackages); + mCurrentMaxSessionId = Math.max(mCurrentMaxSessionId, blobMetadata.getBlobId()); + if (version >= XML_VERSION_ALLOW_ACCESS_ACROSS_USERS) { + addBlobLocked(blobMetadata); } else { - addBlobForUserLocked(blobMetadata, blobMetadata.getUserId()); - blobMetadata.removeCommittersFromUnknownPkgs(userPackages); - blobMetadata.removeLeaseesFromUnknownPkgs(userPackages); + final BlobMetadata existingBlobMetadata = mBlobsMap.get( + blobMetadata.getBlobHandle()); + if (existingBlobMetadata == null) { + addBlobLocked(blobMetadata); + } else { + existingBlobMetadata.addCommittersAndLeasees(blobMetadata); + blobMetadata.getBlobFile().delete(); + } } - mCurrentMaxSessionId = Math.max(mCurrentMaxSessionId, blobMetadata.getBlobId()); } } if (LOGV) { @@ -977,14 +975,6 @@ public class BlobStoreManagerService extends SystemService { } } - private int getPackageUid(String packageName, int userId) { - final int uid = mPackageManagerInternal.getPackageUid( - packageName, - MATCH_DIRECT_BOOT_AWARE | MATCH_DIRECT_BOOT_UNAWARE | MATCH_UNINSTALLED_PACKAGES, - userId); - return uid; - } - private SparseArray<SparseArray<String>> getAllPackages() { final SparseArray<SparseArray<String>> allPackages = new SparseArray<>(); final int[] allUsers = LocalServices.getService(UserManagerInternal.class).getUserIds(); @@ -1004,7 +994,7 @@ public class BlobStoreManagerService extends SystemService { return allPackages; } - AtomicFile prepareSessionsIndexFile() { + private AtomicFile prepareSessionsIndexFile() { final File file = BlobStoreConfig.prepareSessionIndexFile(); if (file == null) { return null; @@ -1012,7 +1002,7 @@ public class BlobStoreManagerService extends SystemService { return new AtomicFile(file, "session_index" /* commitLogTag */); } - AtomicFile prepareBlobsIndexFile() { + private AtomicFile prepareBlobsIndexFile() { final File file = BlobStoreConfig.prepareBlobsIndexFile(); if (file == null) { return null; @@ -1037,9 +1027,7 @@ public class BlobStoreManagerService extends SystemService { writeBlobSessionsAsync(); // Remove the package from the committer and leasee list - final ArrayMap<BlobHandle, BlobMetadata> userBlobs = - getUserBlobsLocked(UserHandle.getUserId(uid)); - userBlobs.entrySet().removeIf(entry -> { + mBlobsMap.entrySet().removeIf(entry -> { final BlobMetadata blobMetadata = entry.getValue(); final boolean isACommitter = blobMetadata.isACommitter(packageName, uid); if (isACommitter) { @@ -1074,14 +1062,15 @@ public class BlobStoreManagerService extends SystemService { } } - final ArrayMap<BlobHandle, BlobMetadata> userBlobs = - mBlobsMap.removeReturnOld(userId); - if (userBlobs != null) { - for (int i = 0, count = userBlobs.size(); i < count; ++i) { - final BlobMetadata blobMetadata = userBlobs.valueAt(i); + mBlobsMap.entrySet().removeIf(entry -> { + final BlobMetadata blobMetadata = entry.getValue(); + blobMetadata.removeDataForUser(userId); + if (blobMetadata.shouldBeDeleted(true /* respectLeaseWaitTime */)) { deleteBlobLocked(blobMetadata); + return true; } - } + return false; + }); if (LOGV) { Slog.v(TAG, "Removed blobs data in user " + userId); } @@ -1114,22 +1103,19 @@ public class BlobStoreManagerService extends SystemService { } // Cleanup any stale blobs. - for (int i = 0, userCount = mBlobsMap.size(); i < userCount; ++i) { - final ArrayMap<BlobHandle, BlobMetadata> userBlobs = mBlobsMap.valueAt(i); - userBlobs.entrySet().removeIf(entry -> { - final BlobMetadata blobMetadata = entry.getValue(); + mBlobsMap.entrySet().removeIf(entry -> { + final BlobMetadata blobMetadata = entry.getValue(); - // Remove expired leases - blobMetadata.removeExpiredLeases(); + // Remove expired leases + blobMetadata.removeExpiredLeases(); - if (blobMetadata.shouldBeDeleted(true /* respectLeaseWaitTime */)) { - deleteBlobLocked(blobMetadata); - deletedBlobIds.add(blobMetadata.getBlobId()); - return true; - } - return false; - }); - } + if (blobMetadata.shouldBeDeleted(true /* respectLeaseWaitTime */)) { + deleteBlobLocked(blobMetadata); + deletedBlobIds.add(blobMetadata.getBlobId()); + return true; + } + return false; + }); writeBlobsInfoAsync(); // Cleanup any stale sessions. @@ -1195,34 +1181,34 @@ public class BlobStoreManagerService extends SystemService { void runClearAllBlobs(@UserIdInt int userId) { synchronized (mBlobsLock) { - for (int i = 0, userCount = mBlobsMap.size(); i < userCount; ++i) { - final int blobUserId = mBlobsMap.keyAt(i); - if (userId != UserHandle.USER_ALL && userId != blobUserId) { - continue; + mBlobsMap.entrySet().removeIf(entry -> { + final BlobMetadata blobMetadata = entry.getValue(); + if (userId == UserHandle.USER_ALL) { + mActiveBlobIds.remove(blobMetadata.getBlobId()); + return true; } - final ArrayMap<BlobHandle, BlobMetadata> userBlobs = mBlobsMap.valueAt(i); - for (int j = 0, blobsCount = userBlobs.size(); j < blobsCount; ++j) { - mActiveBlobIds.remove(userBlobs.valueAt(j).getBlobId()); + blobMetadata.removeDataForUser(userId); + if (blobMetadata.shouldBeDeleted(false /* respectLeaseWaitTime */)) { + mActiveBlobIds.remove(blobMetadata.getBlobId()); + return true; } - } - if (userId == UserHandle.USER_ALL) { - mBlobsMap.clear(); - } else { - mBlobsMap.remove(userId); - } + return false; + }); writeBlobsInfoAsync(); } } void deleteBlob(@NonNull BlobHandle blobHandle, @UserIdInt int userId) { synchronized (mBlobsLock) { - final ArrayMap<BlobHandle, BlobMetadata> userBlobs = getUserBlobsLocked(userId); - final BlobMetadata blobMetadata = userBlobs.get(blobHandle); + final BlobMetadata blobMetadata = mBlobsMap.get(blobHandle); if (blobMetadata == null) { return; } - deleteBlobLocked(blobMetadata); - userBlobs.remove(blobHandle); + blobMetadata.removeDataForUser(userId); + if (blobMetadata.shouldBeDeleted(false /* respectLeaseWaitTime */)) { + deleteBlobLocked(blobMetadata); + mBlobsMap.remove(blobHandle); + } writeBlobsInfoAsync(); } } @@ -1235,11 +1221,12 @@ public class BlobStoreManagerService extends SystemService { boolean isBlobAvailable(long blobId, int userId) { synchronized (mBlobsLock) { - final ArrayMap<BlobHandle, BlobMetadata> userBlobs = getUserBlobsLocked(userId); - for (BlobMetadata blobMetadata : userBlobs.values()) { - if (blobMetadata.getBlobId() == blobId) { - return true; + for (int i = 0, blobCount = mBlobsMap.size(); i < blobCount; ++i) { + final BlobMetadata blobMetadata = mBlobsMap.valueAt(i); + if (blobMetadata.getBlobId() != blobId) { + continue; } + return blobMetadata.hasACommitterInUser(userId); } return false; } @@ -1274,27 +1261,22 @@ public class BlobStoreManagerService extends SystemService { @GuardedBy("mBlobsLock") private void dumpBlobsLocked(IndentingPrintWriter fout, DumpArgs dumpArgs) { - for (int i = 0, userCount = mBlobsMap.size(); i < userCount; ++i) { - final int userId = mBlobsMap.keyAt(i); - if (!dumpArgs.shouldDumpUser(userId)) { + fout.println("List of blobs (" + mBlobsMap.size() + "):"); + fout.increaseIndent(); + for (int i = 0, blobCount = mBlobsMap.size(); i < blobCount; ++i) { + final BlobMetadata blobMetadata = mBlobsMap.valueAt(i); + if (!dumpArgs.shouldDumpBlob(blobMetadata.getBlobId())) { continue; } - final ArrayMap<BlobHandle, BlobMetadata> userBlobs = mBlobsMap.valueAt(i); - fout.println("List of blobs in user #" - + userId + " (" + userBlobs.size() + "):"); + fout.println("Blob #" + blobMetadata.getBlobId()); fout.increaseIndent(); - for (int j = 0, blobsCount = userBlobs.size(); j < blobsCount; ++j) { - final BlobMetadata blobMetadata = userBlobs.valueAt(j); - if (!dumpArgs.shouldDumpBlob(blobMetadata.getBlobId())) { - continue; - } - fout.println("Blob #" + blobMetadata.getBlobId()); - fout.increaseIndent(); - blobMetadata.dump(fout, dumpArgs); - fout.decreaseIndent(); - } + blobMetadata.dump(fout, dumpArgs); fout.decreaseIndent(); } + if (mBlobsMap.isEmpty()) { + fout.println("<empty>"); + } + fout.decreaseIndent(); } private class BlobStorageStatsAugmenter implements StorageStatsAugmenter { @@ -1308,13 +1290,12 @@ public class BlobStoreManagerService extends SystemService { } }, userId); - forEachBlobInUser(blobMetadata -> { - if (blobMetadata.isALeasee(packageName)) { - if (!blobMetadata.hasOtherLeasees(packageName) || !callerHasStatsPermission) { - blobsDataSize.getAndAdd(blobMetadata.getSize()); - } + forEachBlob(blobMetadata -> { + if (blobMetadata.shouldAttributeToLeasee(packageName, userId, + callerHasStatsPermission)) { + blobsDataSize.getAndAdd(blobMetadata.getSize()); } - }, userId); + }); stats.dataSize += blobsDataSize.get(); } @@ -1330,13 +1311,12 @@ public class BlobStoreManagerService extends SystemService { } }, userId); - forEachBlobInUser(blobMetadata -> { - if (blobMetadata.isALeasee(uid)) { - if (!blobMetadata.hasOtherLeasees(uid) || !callerHasStatsPermission) { - blobsDataSize.getAndAdd(blobMetadata.getSize()); - } + forEachBlob(blobMetadata -> { + if (blobMetadata.shouldAttributeToLeasee(uid, + callerHasStatsPermission)) { + blobsDataSize.getAndAdd(blobMetadata.getSize()); } - }, userId); + }); stats.dataSize += blobsDataSize.get(); } @@ -1352,13 +1332,26 @@ public class BlobStoreManagerService extends SystemService { } } - private void forEachBlobInUser(Consumer<BlobMetadata> consumer, int userId) { - synchronized (mBlobsLock) { - final ArrayMap<BlobHandle, BlobMetadata> userBlobs = getUserBlobsLocked(userId); - for (int i = 0, count = userBlobs.size(); i < count; ++i) { - final BlobMetadata blobMetadata = userBlobs.valueAt(i); - consumer.accept(blobMetadata); - } + private void forEachBlob(Consumer<BlobMetadata> consumer) { + synchronized (mBlobsMap) { + forEachBlobLocked(consumer); + } + } + + @GuardedBy("mBlobsMap") + private void forEachBlobLocked(Consumer<BlobMetadata> consumer) { + for (int blobIdx = 0, count = mBlobsMap.size(); blobIdx < count; ++blobIdx) { + final BlobMetadata blobMetadata = mBlobsMap.valueAt(blobIdx); + consumer.accept(blobMetadata); + } + } + + @GuardedBy("mBlobsMap") + private void forEachBlobLocked(BiConsumer<BlobHandle, BlobMetadata> consumer) { + for (int blobIdx = 0, count = mBlobsMap.size(); blobIdx < count; ++blobIdx) { + final BlobHandle blobHandle = mBlobsMap.keyAt(blobIdx); + final BlobMetadata blobMetadata = mBlobsMap.valueAt(blobIdx); + consumer.accept(blobHandle, blobMetadata); } } @@ -1886,15 +1879,7 @@ public class BlobStoreManagerService extends SystemService { } private int pullBlobData(int atomTag, List<StatsEvent> data) { - synchronized (mBlobsLock) { - for (int i = 0, userCount = mBlobsMap.size(); i < userCount; ++i) { - final ArrayMap<BlobHandle, BlobMetadata> userBlobs = mBlobsMap.valueAt(i); - for (int j = 0, blobsCount = userBlobs.size(); j < blobsCount; ++j) { - final BlobMetadata blob = userBlobs.valueAt(j); - data.add(blob.dumpAsStatsEvent(atomTag)); - } - } - } + forEachBlob(blobMetadata -> data.add(blobMetadata.dumpAsStatsEvent(atomTag))); return StatsManager.PULL_SUCCESS; } diff --git a/apex/blobstore/service/java/com/android/server/blob/BlobStoreSession.java b/apex/blobstore/service/java/com/android/server/blob/BlobStoreSession.java index 2c3f682a46e0..3f0032fe537e 100644 --- a/apex/blobstore/service/java/com/android/server/blob/BlobStoreSession.java +++ b/apex/blobstore/service/java/com/android/server/blob/BlobStoreSession.java @@ -56,12 +56,12 @@ import android.os.storage.StorageManager; import android.system.ErrnoException; import android.system.Os; import android.util.ExceptionUtils; +import android.util.IndentingPrintWriter; import android.util.Slog; import com.android.internal.annotations.GuardedBy; import com.android.internal.annotations.VisibleForTesting; import com.android.internal.util.FrameworkStatsLog; -import com.android.internal.util.IndentingPrintWriter; import com.android.internal.util.Preconditions; import com.android.internal.util.XmlUtils; import com.android.server.blob.BlobStoreManagerService.DumpArgs; diff --git a/services/tests/mockingservicestests/src/com/android/server/blob/BlobStoreManagerServiceTest.java b/services/tests/mockingservicestests/src/com/android/server/blob/BlobStoreManagerServiceTest.java index 924ad7f3f5a1..76f7e80a3412 100644 --- a/services/tests/mockingservicestests/src/com/android/server/blob/BlobStoreManagerServiceTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/blob/BlobStoreManagerServiceTest.java @@ -35,7 +35,6 @@ import android.os.Looper; import android.os.Message; import android.os.UserHandle; import android.platform.test.annotations.Presubmit; -import android.util.ArrayMap; import android.util.LongSparseArray; import androidx.test.InstrumentationRegistry; @@ -68,7 +67,6 @@ public class BlobStoreManagerServiceTest { private File mBlobsDir; private LongSparseArray<BlobStoreSession> mUserSessions; - private ArrayMap<BlobHandle, BlobMetadata> mUserBlobs; private static final String TEST_PKG1 = "com.example1"; private static final String TEST_PKG2 = "com.example2"; @@ -99,10 +97,8 @@ public class BlobStoreManagerServiceTest { mHandler = new TestHandler(Looper.getMainLooper()); mService = new BlobStoreManagerService(mContext, new TestInjector()); mUserSessions = new LongSparseArray<>(); - mUserBlobs = new ArrayMap<>(); mService.addUserSessionsForTest(mUserSessions, UserHandle.myUserId()); - mService.addUserBlobsForTest(mUserBlobs, UserHandle.myUserId()); } @After @@ -147,7 +143,7 @@ public class BlobStoreManagerServiceTest { final BlobMetadata blobMetadata1 = createBlobMetadataMock(blobId1, blobFile1, blobHandle1, true /* hasLeases */); doReturn(true).when(blobMetadata1).isACommitter(TEST_PKG1, TEST_UID1); - mUserBlobs.put(blobHandle1, blobMetadata1); + addBlob(blobHandle1, blobMetadata1); final long blobId2 = 347; final File blobFile2 = mock(File.class); @@ -156,7 +152,7 @@ public class BlobStoreManagerServiceTest { final BlobMetadata blobMetadata2 = createBlobMetadataMock(blobId2, blobFile2, blobHandle2, false /* hasLeases */); doReturn(false).when(blobMetadata2).isACommitter(TEST_PKG1, TEST_UID1); - mUserBlobs.put(blobHandle2, blobMetadata2); + addBlob(blobHandle2, blobMetadata2); final long blobId3 = 49875; final File blobFile3 = mock(File.class); @@ -165,7 +161,7 @@ public class BlobStoreManagerServiceTest { final BlobMetadata blobMetadata3 = createBlobMetadataMock(blobId3, blobFile3, blobHandle3, true /* hasLeases */); doReturn(true).when(blobMetadata3).isACommitter(TEST_PKG1, TEST_UID1); - mUserBlobs.put(blobHandle3, blobMetadata3); + addBlob(blobHandle3, blobMetadata3); mService.addActiveIdsForTest(sessionId1, sessionId2, sessionId3, sessionId4, blobId1, blobId2, blobId3); @@ -197,10 +193,10 @@ public class BlobStoreManagerServiceTest { verify(blobMetadata2).destroy(); verify(blobMetadata3).destroy(); - assertThat(mUserBlobs.size()).isEqualTo(1); - assertThat(mUserBlobs.get(blobHandle1)).isNotNull(); - assertThat(mUserBlobs.get(blobHandle2)).isNull(); - assertThat(mUserBlobs.get(blobHandle3)).isNull(); + assertThat(mService.getBlobsCountForTest()).isEqualTo(1); + assertThat(mService.getBlobForTest(blobHandle1)).isNotNull(); + assertThat(mService.getBlobForTest(blobHandle2)).isNull(); + assertThat(mService.getBlobForTest(blobHandle3)).isNull(); assertThat(mService.getActiveIdsForTest()).containsExactly( sessionId2, sessionId3, blobId1); @@ -293,7 +289,7 @@ public class BlobStoreManagerServiceTest { "label1", System.currentTimeMillis() - 2000, "tag1"); final BlobMetadata blobMetadata1 = createBlobMetadataMock(blobId1, blobFile1, blobHandle1, true /* hasLeases */); - mUserBlobs.put(blobHandle1, blobMetadata1); + addBlob(blobHandle1, blobMetadata1); final long blobId2 = 78974; final File blobFile2 = mock(File.class); @@ -301,7 +297,7 @@ public class BlobStoreManagerServiceTest { "label2", System.currentTimeMillis() + 30000, "tag2"); final BlobMetadata blobMetadata2 = createBlobMetadataMock(blobId2, blobFile2, blobHandle2, true /* hasLeases */); - mUserBlobs.put(blobHandle2, blobMetadata2); + addBlob(blobHandle2, blobMetadata2); final long blobId3 = 97; final File blobFile3 = mock(File.class); @@ -309,7 +305,7 @@ public class BlobStoreManagerServiceTest { "label3", System.currentTimeMillis() + 4400000, "tag3"); final BlobMetadata blobMetadata3 = createBlobMetadataMock(blobId3, blobFile3, blobHandle3, false /* hasLeases */); - mUserBlobs.put(blobHandle3, blobMetadata3); + addBlob(blobHandle3, blobMetadata3); mService.addActiveIdsForTest(blobId1, blobId2, blobId3); @@ -321,8 +317,8 @@ public class BlobStoreManagerServiceTest { verify(blobMetadata2, never()).destroy(); verify(blobMetadata3).destroy(); - assertThat(mUserBlobs.size()).isEqualTo(1); - assertThat(mUserBlobs.get(blobHandle2)).isNotNull(); + assertThat(mService.getBlobsCountForTest()).isEqualTo(1); + assertThat(mService.getBlobForTest(blobHandle2)).isNotNull(); assertThat(mService.getActiveIdsForTest()).containsExactly(blobId2); assertThat(mService.getKnownIdsForTest()).containsExactly(blobId1, blobId2, blobId3); @@ -336,21 +332,21 @@ public class BlobStoreManagerServiceTest { doReturn(size1).when(blobMetadata1).getSize(); doReturn(true).when(blobMetadata1).isALeasee(TEST_PKG1, TEST_UID1); doReturn(true).when(blobMetadata1).isALeasee(TEST_PKG2, TEST_UID2); - mUserBlobs.put(mock(BlobHandle.class), blobMetadata1); + addBlob(mock(BlobHandle.class), blobMetadata1); final BlobMetadata blobMetadata2 = mock(BlobMetadata.class); final long size2 = 89475; doReturn(size2).when(blobMetadata2).getSize(); doReturn(false).when(blobMetadata2).isALeasee(TEST_PKG1, TEST_UID1); doReturn(true).when(blobMetadata2).isALeasee(TEST_PKG2, TEST_UID2); - mUserBlobs.put(mock(BlobHandle.class), blobMetadata2); + addBlob(mock(BlobHandle.class), blobMetadata2); final BlobMetadata blobMetadata3 = mock(BlobMetadata.class); final long size3 = 328732; doReturn(size3).when(blobMetadata3).getSize(); doReturn(true).when(blobMetadata3).isALeasee(TEST_PKG1, TEST_UID1); doReturn(false).when(blobMetadata3).isALeasee(TEST_PKG2, TEST_UID2); - mUserBlobs.put(mock(BlobHandle.class), blobMetadata3); + addBlob(mock(BlobHandle.class), blobMetadata3); // Verify usage is calculated correctly assertThat(mService.getTotalUsageBytesLocked(TEST_UID1, TEST_PKG1)) @@ -388,6 +384,11 @@ public class BlobStoreManagerServiceTest { return blobMetadata; } + private void addBlob(BlobHandle blobHandle, BlobMetadata blobMetadata) { + doReturn(blobHandle).when(blobMetadata).getBlobHandle(); + mService.addBlobLocked(blobMetadata); + } + private class TestHandler extends Handler { TestHandler(Looper looper) { super(looper); diff --git a/tests/BlobStoreTestUtils/src/com/android/utils/blob/Utils.java b/tests/BlobStoreTestUtils/src/com/android/utils/blob/Utils.java index ec859955694c..2d230a74a477 100644 --- a/tests/BlobStoreTestUtils/src/com/android/utils/blob/Utils.java +++ b/tests/BlobStoreTestUtils/src/com/android/utils/blob/Utils.java @@ -23,6 +23,7 @@ import android.app.blob.BlobStoreManager; import android.app.blob.LeaseInfo; import android.content.Context; import android.content.res.Resources; +import android.os.FileUtils; import android.os.ParcelFileDescriptor; import android.util.Log; @@ -56,6 +57,16 @@ public class Utils { } } + public static void writeToSession(BlobStoreManager.Session session, ParcelFileDescriptor input) + throws IOException { + try (FileInputStream in = new ParcelFileDescriptor.AutoCloseInputStream(input)) { + try (FileOutputStream out = new ParcelFileDescriptor.AutoCloseOutputStream( + session.openWrite(0, -1))) { + FileUtils.copy(in, out); + } + } + } + public static void writeToSession(BlobStoreManager.Session session, ParcelFileDescriptor input, long lengthBytes) throws IOException { try (FileInputStream in = new ParcelFileDescriptor.AutoCloseInputStream(input)) { |