diff --git a/services/core/java/com/android/server/pm/PackageInstallerSession.java b/services/core/java/com/android/server/pm/PackageInstallerSession.java index 7b71be98834c2..4eafe5143c8ff 100644 --- a/services/core/java/com/android/server/pm/PackageInstallerSession.java +++ b/services/core/java/com/android/server/pm/PackageInstallerSession.java @@ -309,23 +309,24 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { private final String mOriginalInstallerPackageName; /** Uid of the owner of the installer session */ - @GuardedBy("mLock") - private int mInstallerUid; + private volatile int mInstallerUid; /** Where this install request came from */ @GuardedBy("mLock") private InstallSource mInstallSource; - @GuardedBy("mLock") + private final Object mProgressLock = new Object(); + + @GuardedBy("mProgressLock") private float mClientProgress = 0; - @GuardedBy("mLock") + @GuardedBy("mProgressLock") private float mInternalProgress = 0; - @GuardedBy("mLock") + @GuardedBy("mProgressLock") private float mProgress = 0; - @GuardedBy("mLock") + @GuardedBy("mProgressLock") private float mReportedProgress = -1; - @GuardedBy("mLock") + @GuardedBy("mProgressLock") private float mIncrementalProgress = 0; /** State of the session. */ @@ -571,7 +572,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { */ @Override public void installSession(IntentSender statusReceiver) { - assertCallerIsOwnerOrRootOrSystemLocked(); + assertCallerIsOwnerOrRootOrSystem(); assertNotChildLocked("StagedSession#installSession"); Preconditions.checkArgument(isCommitted() && isSessionReady()); @@ -670,7 +671,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { final Runnable r; synchronized (mLock) { assertNotChildLocked("StagedSession#abandon"); - assertCallerIsOwnerOrRootLocked(); + assertCallerIsOwnerOrRoot(); if (isInTerminalState()) { // We keep the session in the database if it's in a finalized state. It will be // removed by PackageInstallerService when the last update time is old enough. @@ -744,7 +745,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { */ @Override public void verifySession() { - assertCallerIsOwnerOrRootOrSystemLocked(); + assertCallerIsOwnerOrRootOrSystem(); Preconditions.checkArgument(isCommitted()); Preconditions.checkArgument(!mSessionApplied && !mSessionFailed); verify(); @@ -1229,7 +1230,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { } } - @GuardedBy("mLock") + @GuardedBy("mProgressLock") private void setClientProgressLocked(float progress) { // Always publish first staging movement final boolean forcePublish = (mClientProgress == 0); @@ -1239,21 +1240,21 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { @Override public void setClientProgress(float progress) { - synchronized (mLock) { - assertCallerIsOwnerOrRootLocked(); + assertCallerIsOwnerOrRoot(); + synchronized (mProgressLock) { setClientProgressLocked(progress); } } @Override public void addClientProgress(float progress) { - synchronized (mLock) { - assertCallerIsOwnerOrRootLocked(); + assertCallerIsOwnerOrRoot(); + synchronized (mProgressLock) { setClientProgressLocked(mClientProgress + progress); } } - @GuardedBy("mLock") + @GuardedBy("mProgressLock") private void computeProgressLocked(boolean forcePublish) { if (!mCommitted) { mProgress = MathUtils.constrain(mClientProgress * 0.8f, 0f, 0.8f) @@ -1278,8 +1279,8 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { @Override public String[] getNames() { + assertCallerIsOwnerOrRoot(); synchronized (mLock) { - assertCallerIsOwnerOrRootLocked(); assertPreparedAndNotCommittedOrDestroyedLocked("getNames"); return getNamesLocked(); @@ -1362,8 +1363,8 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { } } + assertCallerIsOwnerOrRoot(); synchronized (mLock) { - assertCallerIsOwnerOrRootLocked(); assertPreparedAndNotCommittedOrDestroyedLocked("addChecksums"); if (mChecksums.containsKey(name)) { @@ -1384,8 +1385,8 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { throw new IllegalStateException("Must specify package name to remove a split"); } + assertCallerIsOwnerOrRoot(); synchronized (mLock) { - assertCallerIsOwnerOrRootLocked(); assertPreparedAndNotCommittedOrDestroyedLocked("removeSplit"); try { @@ -1431,8 +1432,8 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { throw new IllegalStateException( "Cannot write regular files in a data loader installation session."); } + assertCallerIsOwnerOrRoot(); synchronized (mLock) { - assertCallerIsOwnerOrRootLocked(); assertPreparedAndNotSealedLocked("assertCanWrite"); } if (reverseMode) { @@ -1605,8 +1606,8 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { throw new IllegalStateException( "Cannot read regular files in a data loader installation session."); } + assertCallerIsOwnerOrRoot(); synchronized (mLock) { - assertCallerIsOwnerOrRootLocked(); assertPreparedAndNotCommittedOrDestroyedLocked("openRead"); try { return openReadInternalLocked(name); @@ -1634,8 +1635,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { * Check if the caller is the owner of this session. Otherwise throw a * {@link SecurityException}. */ - @GuardedBy("mLock") - private void assertCallerIsOwnerOrRootLocked() { + private void assertCallerIsOwnerOrRoot() { final int callingUid = Binder.getCallingUid(); if (callingUid != Process.ROOT_UID && callingUid != mInstallerUid) { throw new SecurityException("Session does not belong to uid " + callingUid); @@ -1646,8 +1646,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { * Check if the caller is the owner of this session. Otherwise throw a * {@link SecurityException}. */ - @GuardedBy("mLock") - private void assertCallerIsOwnerOrRootOrSystemLocked() { + private void assertCallerIsOwnerOrRootOrSystem() { final int callingUid = Binder.getCallingUid(); if (callingUid != Process.ROOT_UID && callingUid != mInstallerUid && callingUid != Process.SYSTEM_UID) { @@ -1929,9 +1928,9 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { */ private boolean markAsSealed(@NonNull IntentSender statusReceiver, boolean forTransfer) { Objects.requireNonNull(statusReceiver); + assertCallerIsOwnerOrRoot(); synchronized (mLock) { - assertCallerIsOwnerOrRootLocked(); assertPreparedAndNotDestroyedLocked("commit of session " + sessionId); assertNoWriteFileTransfersOpenLocked(); @@ -2004,9 +2003,11 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { "Session destroyed"); } if (!isIncrementalInstallation()) { - // For non-incremental installs, client staging is fully done at this point - mClientProgress = 1f; - computeProgressLocked(true); + synchronized (mProgressLock) { + // For non-incremental installs, client staging is fully done at this point + mClientProgress = 1f; + computeProgressLocked(true); + } } // This ongoing commit should keep session active, even though client @@ -2191,7 +2192,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { } synchronized (mLock) { - assertCallerIsOwnerOrRootLocked(); + assertCallerIsOwnerOrRoot(); assertPreparedAndNotSealedLocked("transfer"); try { @@ -2376,8 +2377,10 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { PackageLite result = parseApkLite(); if (result != null) { mPackageLite = result; - mInternalProgress = 0.5f; - computeProgressLocked(true); + synchronized (mProgressLock) { + mInternalProgress = 0.5f; + computeProgressLocked(true); + } extractNativeLibraries( mPackageLite, stageDir, params.abiOverride, mayInheritNativeLibs()); @@ -3554,7 +3557,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { int activeCount; synchronized (mLock) { if (checkCaller) { - assertCallerIsOwnerOrRootLocked(); + assertCallerIsOwnerOrRoot(); } activeCount = mActiveCount.decrementAndGet(); @@ -3593,7 +3596,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { private void abandonNonStaged() { synchronized (mLock) { assertNotChildLocked("abandonNonStaged"); - assertCallerIsOwnerOrRootLocked(); + assertCallerIsOwnerOrRoot(); if (mRelinquished) { if (LOGD) Slog.d(TAG, "Ignoring abandon after commit relinquished control"); return; @@ -3659,7 +3662,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { } synchronized (mLock) { - assertCallerIsOwnerOrRootLocked(); + assertCallerIsOwnerOrRoot(); assertPreparedAndNotSealedLocked("addFile"); if (!mFiles.add(new FileEntry(mFiles.size(), @@ -3680,7 +3683,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { } synchronized (mLock) { - assertCallerIsOwnerOrRootLocked(); + assertCallerIsOwnerOrRoot(); assertPreparedAndNotSealedLocked("removeFile"); if (!mFiles.add(new FileEntry(mFiles.size(), @@ -3893,7 +3896,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { new IPackageLoadingProgressCallback.Stub() { @Override public void onPackageLoadingProgressChanged(float progress) { - synchronized (mLock) { + synchronized (mProgressLock) { mIncrementalProgress = progress; computeProgressLocked(true); } @@ -3993,7 +3996,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { + " as it is in an invalid state."); } synchronized (mLock) { - assertCallerIsOwnerOrRootLocked(); + assertCallerIsOwnerOrRoot(); assertPreparedAndNotSealedLocked("addChildSessionId"); final int indexOfSession = mChildSessions.indexOfKey(childSessionId); @@ -4012,7 +4015,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { @Override public void removeChildSessionId(int sessionId) { synchronized (mLock) { - assertCallerIsOwnerOrRootLocked(); + assertCallerIsOwnerOrRoot(); assertPreparedAndNotSealedLocked("removeChildSessionId"); final int indexOfSession = mChildSessions.indexOfKey(sessionId);