From d5d79a9f0d5fc62c6d2a110ec2b0d16513336b27 Mon Sep 17 00:00:00 2001 From: Christopher Tate Date: Wed, 4 Oct 2017 17:38:20 -0700 Subject: [PATCH 1/2] Do not apply app-link autoVerify policy to instant app installs Bug: 66698768 Test: manual (cherry picked from commit 05941b396a44bd81c143b727e5c4c630212228e0) Merged-In: Ib9bea22bf8096e708eef93934e0e972be3a2a4c5 Change-Id: Ifc372eef9bc2a140b9252c85f9be0c2ff9cdbb4c --- .../java/com/android/server/pm/PackageManagerService.java | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java index 8853aa0e89756..9541a686361f4 100644 --- a/services/core/java/com/android/server/pm/PackageManagerService.java +++ b/services/core/java/com/android/server/pm/PackageManagerService.java @@ -18315,7 +18315,13 @@ public class PackageManagerService extends IPackageManager.Stub // TODO: Layering violation BackgroundDexOptService.notifyPackageChanged(pkg.packageName); - startIntentFilterVerifications(args.user.getIdentifier(), replace, pkg); + if (!instantApp) { + startIntentFilterVerifications(args.user.getIdentifier(), replace, pkg); + } else { + if (DEBUG_DOMAIN_VERIFICATION) { + Slog.d(TAG, "Not verifying instant app install for app links: " + pkgName); + } + } try (PackageFreezer freezer = freezePackageForInstall(pkgName, installFlags, "installPackageLI")) { From b4236711af8950402a1947de807d4220eed0f2da Mon Sep 17 00:00:00 2001 From: Calin Juravle Date: Wed, 15 Nov 2017 16:28:01 -0800 Subject: [PATCH 2/2] Fix package install flow w.r.t. dexopt Calling dexopt before the applicationInfo gets the uid is wrong. Dexopt needs to be able to set the GID of the odex file to the UserHandle.getSharedAppGid(pkg.applicationInfo.uid) and that is possible only with a valid uid. Move the dexopt logic after installNewPackageLIF/replacePackageLIF to ensure that we get a valid uid. Bug: 69331247 Test: adb install & check the GID of the compiler artifacts (cherry picked from commit c6540daf8388ce1ebd2e4157ec70b9651ae14b4f) Merged-In: I2434a1a0b9015091a9af2009b3f785b7a16e1256 Change-Id: I80504f434e1507f30f366d1122224ff959bec4a1 --- .../server/pm/PackageDexOptimizer.java | 7 ++ .../server/pm/PackageManagerService.java | 93 ++++++++++--------- 2 files changed, 56 insertions(+), 44 deletions(-) diff --git a/services/core/java/com/android/server/pm/PackageDexOptimizer.java b/services/core/java/com/android/server/pm/PackageDexOptimizer.java index 401eb62f0b1bf..713fab983a708 100644 --- a/services/core/java/com/android/server/pm/PackageDexOptimizer.java +++ b/services/core/java/com/android/server/pm/PackageDexOptimizer.java @@ -128,6 +128,10 @@ public class PackageDexOptimizer { int performDexOpt(PackageParser.Package pkg, String[] sharedLibraries, String[] instructionSets, CompilerStats.PackageStats packageStats, PackageDexUsage.PackageUseInfo packageUseInfo, DexoptOptions options) { + if (pkg.applicationInfo.uid == -1) { + throw new IllegalArgumentException("Dexopt for " + pkg.packageName + + " has invalid uid."); + } if (!canOptimizePackage(pkg)) { return DEX_OPT_SKIPPED; } @@ -293,6 +297,9 @@ public class PackageDexOptimizer { */ public int dexOptSecondaryDexPath(ApplicationInfo info, String path, PackageDexUsage.DexUseInfo dexUseInfo, DexoptOptions options) { + if (info.uid == -1) { + throw new IllegalArgumentException("Dexopt for path " + path + " has invalid uid."); + } synchronized (mInstallLock) { final long acquireTime = acquireWakeLockLI(info.uid); try { diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java index 9541a686361f4..ee18892df417c 100644 --- a/services/core/java/com/android/server/pm/PackageManagerService.java +++ b/services/core/java/com/android/server/pm/PackageManagerService.java @@ -18271,50 +18271,6 @@ public class PackageManagerService extends IPackageManager.Stub return; } - // Verify if we need to dexopt the app. - // - // NOTE: it is *important* to call dexopt after doRename which will sync the - // package data from PackageParser.Package and its corresponding ApplicationInfo. - // - // We only need to dexopt if the package meets ALL of the following conditions: - // 1) it is not forward locked. - // 2) it is not on on an external ASEC container. - // 3) it is not an instant app or if it is then dexopt is enabled via gservices. - // - // Note that we do not dexopt instant apps by default. dexopt can take some time to - // complete, so we skip this step during installation. Instead, we'll take extra time - // the first time the instant app starts. It's preferred to do it this way to provide - // continuous progress to the useur instead of mysteriously blocking somewhere in the - // middle of running an instant app. The default behaviour can be overridden - // via gservices. - final boolean performDexopt = !forwardLocked - && !pkg.applicationInfo.isExternalAsec() - && (!instantApp || Global.getInt(mContext.getContentResolver(), - Global.INSTANT_APP_DEXOPT_ENABLED, 0) != 0); - - if (performDexopt) { - Trace.traceBegin(TRACE_TAG_PACKAGE_MANAGER, "dexopt"); - // Do not run PackageDexOptimizer through the local performDexOpt - // method because `pkg` may not be in `mPackages` yet. - // - // Also, don't fail application installs if the dexopt step fails. - DexoptOptions dexoptOptions = new DexoptOptions(pkg.packageName, - REASON_INSTALL, - DexoptOptions.DEXOPT_BOOT_COMPLETE); - mPackageDexOptimizer.performDexOpt(pkg, pkg.usesLibraryFiles, - null /* instructionSets */, - getOrCreateCompilerPackageStats(pkg), - mDexManager.getPackageUseInfoOrDefault(pkg.packageName), - dexoptOptions); - Trace.traceEnd(TRACE_TAG_PACKAGE_MANAGER); - } - - // Notify BackgroundDexOptService that the package has been changed. - // If this is an update of a package which used to fail to compile, - // BackgroundDexOptService will remove it from its blacklist. - // TODO: Layering violation - BackgroundDexOptService.notifyPackageChanged(pkg.packageName); - if (!instantApp) { startIntentFilterVerifications(args.user.getIdentifier(), replace, pkg); } else { @@ -18346,6 +18302,55 @@ public class PackageManagerService extends IPackageManager.Stub } } + // Check whether we need to dexopt the app. + // + // NOTE: it is IMPORTANT to call dexopt: + // - after doRename which will sync the package data from PackageParser.Package and its + // corresponding ApplicationInfo. + // - after installNewPackageLIF or replacePackageLIF which will update result with the + // uid of the application (pkg.applicationInfo.uid). + // This update happens in place! + // + // We only need to dexopt if the package meets ALL of the following conditions: + // 1) it is not forward locked. + // 2) it is not on on an external ASEC container. + // 3) it is not an instant app or if it is then dexopt is enabled via gservices. + // + // Note that we do not dexopt instant apps by default. dexopt can take some time to + // complete, so we skip this step during installation. Instead, we'll take extra time + // the first time the instant app starts. It's preferred to do it this way to provide + // continuous progress to the useur instead of mysteriously blocking somewhere in the + // middle of running an instant app. The default behaviour can be overridden + // via gservices. + final boolean performDexopt = (res.returnCode == PackageManager.INSTALL_SUCCEEDED) + && !forwardLocked + && !pkg.applicationInfo.isExternalAsec() + && (!instantApp || Global.getInt(mContext.getContentResolver(), + Global.INSTANT_APP_DEXOPT_ENABLED, 0) != 0); + + if (performDexopt) { + Trace.traceBegin(TRACE_TAG_PACKAGE_MANAGER, "dexopt"); + // Do not run PackageDexOptimizer through the local performDexOpt + // method because `pkg` may not be in `mPackages` yet. + // + // Also, don't fail application installs if the dexopt step fails. + DexoptOptions dexoptOptions = new DexoptOptions(pkg.packageName, + REASON_INSTALL, + DexoptOptions.DEXOPT_BOOT_COMPLETE); + mPackageDexOptimizer.performDexOpt(pkg, pkg.usesLibraryFiles, + null /* instructionSets */, + getOrCreateCompilerPackageStats(pkg), + mDexManager.getPackageUseInfoOrDefault(pkg.packageName), + dexoptOptions); + Trace.traceEnd(TRACE_TAG_PACKAGE_MANAGER); + } + + // Notify BackgroundDexOptService that the package has been changed. + // If this is an update of a package which used to fail to compile, + // BackgroundDexOptService will remove it from its blacklist. + // TODO: Layering violation + BackgroundDexOptService.notifyPackageChanged(pkg.packageName); + synchronized (mPackages) { final PackageSetting ps = mSettings.mPackages.get(pkgName); if (ps != null) {