From fd1bb4d4fd25c4241777ef9d63f0f27faeb63f9c Mon Sep 17 00:00:00 2001 From: Alex Buynytskyy Date: Thu, 14 Nov 2019 08:25:05 -0800 Subject: [PATCH] Minor refactorings and cleanups. - unified access to staged file list, - moved common exception handling inside sealAndValidateLocked. Test: atest PackageManagerShellCommandTest Change-Id: Iad51cd85036c313a98ed22e9647f7be55fed6fe6 Merged-In: Iad51cd85036c313a98ed22e9647f7be55fed6fe6 --- .../server/pm/PackageInstallerSession.java | 214 ++++++++---------- 1 file changed, 93 insertions(+), 121 deletions(-) diff --git a/services/core/java/com/android/server/pm/PackageInstallerSession.java b/services/core/java/com/android/server/pm/PackageInstallerSession.java index 608cc09ef6a0a..d0167c58df9f4 100644 --- a/services/core/java/com/android/server/pm/PackageInstallerSession.java +++ b/services/core/java/com/android/server/pm/PackageInstallerSession.java @@ -18,7 +18,6 @@ package com.android.server.pm; import static android.content.pm.PackageManager.INSTALL_FAILED_ABORTED; import static android.content.pm.PackageManager.INSTALL_FAILED_BAD_SIGNATURE; -import static android.content.pm.PackageManager.INSTALL_FAILED_CONTAINER_ERROR; import static android.content.pm.PackageManager.INSTALL_FAILED_INSUFFICIENT_STORAGE; import static android.content.pm.PackageManager.INSTALL_FAILED_INTERNAL_ERROR; import static android.content.pm.PackageManager.INSTALL_FAILED_INVALID_APK; @@ -129,7 +128,7 @@ import java.util.concurrent.atomic.AtomicInteger; public class PackageInstallerSession extends IPackageInstallerSession.Stub { private static final String TAG = "PackageInstallerSession"; private static final boolean LOGD = true; - private static final String REMOVE_SPLIT_MARKER_EXTENSION = ".removed"; + private static final String REMOVE_MARKER_EXTENSION = ".removed"; private static final int MSG_COMMIT = 1; private static final int MSG_ON_PACKAGE_INSTALLED = 2; @@ -291,9 +290,6 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { @GuardedBy("mLock") private File mResolvedBaseFile; - @GuardedBy("mLock") - private File mResolvedStageDir; - @GuardedBy("mLock") private final List mResolvedStagedFiles = new ArrayList<>(); @GuardedBy("mLock") @@ -313,7 +309,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { // Installers can't stage directories, so it's fine to ignore // entries like "lost+found". if (file.isDirectory()) return false; - if (file.getName().endsWith(REMOVE_SPLIT_MARKER_EXTENSION)) return false; + if (file.getName().endsWith(REMOVE_MARKER_EXTENSION)) return false; if (DexMetadataHelper.isDexMetadataFile(file)) return false; if (VerityUtils.isFsveritySignatureFile(file)) return false; return true; @@ -323,7 +319,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { @Override public boolean accept(File file) { if (file.isDirectory()) return false; - if (!file.getName().endsWith(REMOVE_SPLIT_MARKER_EXTENSION)) return false; + if (!file.getName().endsWith(REMOVE_MARKER_EXTENSION)) return false; return true; } }; @@ -557,23 +553,6 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { } } - /** - * Resolve the actual location where staged data should be written. This - * might point at an ASEC mount point, which is why we delay path resolution - * until someone actively works with the session. - */ - @GuardedBy("mLock") - private File resolveStageDirLocked() throws IOException { - if (mResolvedStageDir == null) { - if (stageDir != null) { - mResolvedStageDir = stageDir; - } else { - throw new IOException("Missing stageDir"); - } - } - return mResolvedStageDir; - } - @Override public void setClientProgress(float progress) { synchronized (mLock) { @@ -613,14 +592,32 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { assertCallerIsOwnerOrRootLocked(); assertPreparedAndNotCommittedOrDestroyedLocked("getNames"); - try { - return resolveStageDirLocked().list(); - } catch (IOException e) { - throw ExceptionUtils.wrap(e); - } + return getNamesLocked(); } } + @GuardedBy("mLock") + private String[] getNamesLocked() { + return stageDir.list(); + } + + private static File[] filterFiles(File parent, String[] names, FileFilter filter) { + return Arrays.stream(names).map(name -> new File(parent, name)).filter( + file -> filter.accept(file)).toArray(File[]::new); + } + + @GuardedBy("mLock") + private File[] getAddedFilesLocked() { + String[] names = getNamesLocked(); + return filterFiles(stageDir, names, sAddedFilter); + } + + @GuardedBy("mLock") + private File[] getRemovedFilesLocked() { + String[] names = getNamesLocked(); + return filterFiles(stageDir, names, sRemovedFilter); + } + @Override public void removeSplit(String splitName) { if (TextUtils.isEmpty(params.appPackageName)) { @@ -639,13 +636,17 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { } } + private static String getRemoveMarkerName(String name) { + final String markerName = name + REMOVE_MARKER_EXTENSION; + if (!FileUtils.isValidExtFilename(markerName)) { + throw new IllegalArgumentException("Invalid marker: " + markerName); + } + return markerName; + } + private void createRemoveSplitMarkerLocked(String splitName) throws IOException { try { - final String markerName = splitName + REMOVE_SPLIT_MARKER_EXTENSION; - if (!FileUtils.isValidExtFilename(markerName)) { - throw new IllegalArgumentException("Invalid marker: " + markerName); - } - final File target = new File(resolveStageDirLocked(), markerName); + final File target = new File(stageDir, getRemoveMarkerName(splitName)); target.createNewFile(); Os.chmod(target.getAbsolutePath(), 0 /*mode*/); } catch (ErrnoException e) { @@ -679,7 +680,6 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { // will block any attempted install transitions. final RevocableFileDescriptor fd; final FileBridge bridge; - final File stageDir; synchronized (mLock) { assertCallerIsOwnerOrRootLocked(); assertPreparedAndNotSealedLocked("openWrite"); @@ -693,8 +693,6 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { bridge = new FileBridge(); mBridges.add(bridge); } - - stageDir = resolveStageDirLocked(); } try { @@ -800,7 +798,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { if (!FileUtils.isValidExtFilename(name)) { throw new IllegalArgumentException("Invalid name: " + name); } - final File target = new File(resolveStageDirLocked(), name); + final File target = new File(stageDir, name); final FileDescriptor targetFd = Os.open(target.getAbsolutePath(), O_RDONLY, 0); return new ParcelFileDescriptor(targetFd); } catch (ErrnoException e) { @@ -946,7 +944,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { * This method may be called multiple times to update the status receiver validate caller * permissions. */ - public boolean markAsCommitted( + private boolean markAsCommitted( @NonNull IntentSender statusReceiver, boolean forTransfer) { Preconditions.checkNotNull(statusReceiver); @@ -981,12 +979,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { if (!mSealed) { try { sealAndValidateLocked(childSessions); - } catch (IOException e) { - throw new IllegalArgumentException(e); } catch (PackageManagerException e) { - // Do now throw an exception here to stay compatible with O and older - destroyInternal(); - dispatchSessionFinished(e.error, ExceptionUtils.getCompleteMessage(e), null); return false; } } @@ -1086,52 +1079,59 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { */ @GuardedBy("mLock") private void sealAndValidateLocked(List childSessions) - throws PackageManagerException, IOException { - assertNoWriteFileTransfersOpenLocked(); - assertPreparedAndNotDestroyedLocked("sealing of session"); + throws PackageManagerException { + try { + assertNoWriteFileTransfersOpenLocked(); + assertPreparedAndNotDestroyedLocked("sealing of session"); - mSealed = true; + mSealed = true; - if (childSessions != null) { - assertMultiPackageConsistencyLocked(childSessions); - } - - if (params.isStaged) { - final PackageInstallerSession activeSession = mStagingManager.getActiveSession(); - final boolean anotherSessionAlreadyInProgress = - activeSession != null && sessionId != activeSession.sessionId - && mParentSessionId != activeSession.sessionId; - if (anotherSessionAlreadyInProgress) { - throw new PackageManagerException( - PackageManager.INSTALL_FAILED_OTHER_STAGED_SESSION_IN_PROGRESS, - "There is already in-progress committed staged session " - + activeSession.sessionId, null); + if (childSessions != null) { + assertMultiPackageConsistencyLocked(childSessions); } - } - // Read transfers from the original owner stay open, but as the session's data - // cannot be modified anymore, there is no leak of information. For staged sessions, - // further validation is performed by the staging manager. - if (!params.isMultiPackage) { - final PackageInfo pkgInfo = mPm.getPackageInfo( - params.appPackageName, PackageManager.GET_SIGNATURES - | PackageManager.MATCH_STATIC_SHARED_LIBRARIES /*flags*/, userId); - - resolveStageDirLocked(); - - try { - if ((params.installFlags & PackageManager.INSTALL_APEX) != 0) { - validateApexInstallLocked(); - } else { - validateApkInstallLocked(pkgInfo); + if (params.isStaged) { + final PackageInstallerSession activeSession = mStagingManager.getActiveSession(); + final boolean anotherSessionAlreadyInProgress = + activeSession != null && sessionId != activeSession.sessionId + && mParentSessionId != activeSession.sessionId; + if (anotherSessionAlreadyInProgress) { + throw new PackageManagerException( + PackageManager.INSTALL_FAILED_OTHER_STAGED_SESSION_IN_PROGRESS, + "There is already in-progress committed staged session " + + activeSession.sessionId, null); } - } catch (PackageManagerException e) { - throw e; - } catch (Throwable e) { - // Convert all exceptions into package manager exceptions as only those are handled - // in the code above - throw new PackageManagerException(e); } + + // Read transfers from the original owner stay open, but as the session's data + // cannot be modified anymore, there is no leak of information. For staged sessions, + // further validation is performed by the staging manager. + if (!params.isMultiPackage) { + final PackageInfo pkgInfo = mPm.getPackageInfo( + params.appPackageName, PackageManager.GET_SIGNATURES + | PackageManager.MATCH_STATIC_SHARED_LIBRARIES /*flags*/, userId); + + try { + if ((params.installFlags & PackageManager.INSTALL_APEX) != 0) { + validateApexInstallLocked(); + } else { + validateApkInstallLocked(pkgInfo); + } + } catch (PackageManagerException e) { + throw e; + } catch (Throwable e) { + // Convert all exceptions into package manager exceptions as only those are + // handled in the code above. + throw new PackageManagerException(e); + } + } + } catch (PackageManagerException e) { + // Session is sealed but could not be verified, we need to destroy it. + destroyInternal(); + // Dispatch message to remove session from PackageInstallerService + dispatchSessionFinished( + e.error, ExceptionUtils.getCompleteMessage(e), null); + throw e; } } @@ -1154,15 +1154,8 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { synchronized (mLock) { try { sealAndValidateLocked(childSessions); - } catch (IOException e) { - throw new IllegalStateException(e); } catch (PackageManagerException e) { Slog.e(TAG, "Package not valid", e); - // Session is sealed but could not be verified, we need to destroy it. - destroyInternal(); - // Dispatch message to remove session from PackageInstallerService - dispatchSessionFinished( - e.error, ExceptionUtils.getCompleteMessage(e), null); } } } @@ -1203,13 +1196,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { try { sealAndValidateLocked(childSessions); - } catch (IOException e) { - throw new IllegalStateException(e); } catch (PackageManagerException e) { - // Session is sealed but could not be verified, we need to destroy it - destroyInternal(); - dispatchSessionFinished(e.error, ExceptionUtils.getCompleteMessage(e), null); - throw new IllegalArgumentException("Package is not valid", e); } @@ -1356,7 +1343,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { if (params.mode == SessionParams.MODE_INHERIT_EXISTING) { try { final List fromFiles = mResolvedInheritedFiles; - final File toDir = resolveStageDirLocked(); + final File toDir = stageDir; if (LOGD) Slog.d(TAG, "Inherited files: " + mResolvedInheritedFiles); if (!mResolvedInheritedFiles.isEmpty() && mInheritedFilesBase == null) { @@ -1406,8 +1393,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { computeProgressLocked(true); // Unpack native libraries - extractNativeLibraries(mResolvedStageDir, params.abiOverride, - mayInheritNativeLibs()); + extractNativeLibraries(stageDir, params.abiOverride, mayInheritNativeLibs()); } // We've reached point of no return; call into PMS to install the stage. @@ -1468,7 +1454,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { @GuardedBy("mLock") private void validateApexInstallLocked() throws PackageManagerException { - final File[] addedFiles = mResolvedStageDir.listFiles(sAddedFilter); + final File[] addedFiles = getAddedFilesLocked(); if (ArrayUtils.isEmpty(addedFiles)) { throw new PackageManagerException(INSTALL_FAILED_INVALID_APK, "No packages staged"); } @@ -1478,13 +1464,6 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { "Too many files for apex install"); } - try { - resolveStageDirLocked(); - } catch (IOException e) { - throw new PackageManagerException(INSTALL_FAILED_CONTAINER_ERROR, - "Failed to resolve stage location", e); - } - File addedFile = addedFiles[0]; // there is only one file // Ensure file name has proper suffix @@ -1497,7 +1476,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { "Invalid filename: " + targetName); } - final File targetFile = new File(mResolvedStageDir, targetName); + final File targetFile = new File(stageDir, targetName); resolveAndStageFile(addedFile, targetFile); mResolvedBaseFile = targetFile; @@ -1538,25 +1517,18 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { && params.mode == SessionParams.MODE_INHERIT_EXISTING && VerityUtils.hasFsverity(pkgInfo.applicationInfo.getBaseCodePath()); - try { - resolveStageDirLocked(); - } catch (IOException e) { - throw new PackageManagerException(INSTALL_FAILED_CONTAINER_ERROR, - "Failed to resolve stage location", e); - } - - final File[] removedFiles = mResolvedStageDir.listFiles(sRemovedFilter); + final File[] removedFiles = getRemovedFilesLocked(); final List removeSplitList = new ArrayList<>(); if (!ArrayUtils.isEmpty(removedFiles)) { for (File removedFile : removedFiles) { final String fileName = removedFile.getName(); final String splitName = fileName.substring( - 0, fileName.length() - REMOVE_SPLIT_MARKER_EXTENSION.length()); + 0, fileName.length() - REMOVE_MARKER_EXTENSION.length()); removeSplitList.add(splitName); } } - final File[] addedFiles = mResolvedStageDir.listFiles(sAddedFilter); + final File[] addedFiles = getAddedFilesLocked(); if (ArrayUtils.isEmpty(addedFiles) && removeSplitList.size() == 0) { throw new PackageManagerException(INSTALL_FAILED_INVALID_APK, "No packages staged"); } @@ -1600,7 +1572,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { "Invalid filename: " + targetName); } - final File targetFile = new File(mResolvedStageDir, targetName); + final File targetFile = new File(stageDir, targetName); resolveAndStageFile(addedFile, targetFile); // Base is coming from session @@ -1615,7 +1587,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { throw new PackageManagerException(INSTALL_FAILED_INVALID_APK, "Invalid filename: " + dexMetadataFile); } - final File targetDexMetadataFile = new File(mResolvedStageDir, + final File targetDexMetadataFile = new File(stageDir, DexMetadataHelper.buildDexMetadataPathForApk(targetName)); resolveAndStageFile(dexMetadataFile, targetDexMetadataFile); }