Merge "[pm] fix potential NPE during permission check" into rvc-dev

services/art-profile

@@ -32290,7 +32290,6 @@ HPLcom/android/server/pm/permission/BasePermission;->generatePermissionInfo(Ljav
 HSPLcom/android/server/pm/permission/BasePermission;->getName()Ljava/lang/String;
 HSPLcom/android/server/pm/permission/BasePermission;->getProtectionLevel()I
 HSPLcom/android/server/pm/permission/BasePermission;->getSourcePackageName()Ljava/lang/String;
-HSPLcom/android/server/pm/permission/BasePermission;->getSourcePackageSetting()Lcom/android/server/pm/PackageSettingBase;
 PLcom/android/server/pm/permission/BasePermission;->getUid()I
 HSPLcom/android/server/pm/permission/BasePermission;->isAppOp()Z
 HSPLcom/android/server/pm/permission/BasePermission;->isAppPredictor()Z
@@ -32329,7 +32328,6 @@ HSPLcom/android/server/pm/permission/BasePermission;->readLPw(Ljava/util/Map;Lor
 HSPLcom/android/server/pm/permission/BasePermission;->setGids([IZ)V
 HSPLcom/android/server/pm/permission/BasePermission;->setPermission(Landroid/content/pm/parsing/ComponentParseUtils$ParsedPermission;)V
 PLcom/android/server/pm/permission/BasePermission;->setPermission(Landroid/content/pm/parsing/component/ParsedPermission;)V
-HSPLcom/android/server/pm/permission/BasePermission;->setSourcePackageSetting(Lcom/android/server/pm/PackageSettingBase;)V
 HSPLcom/android/server/pm/permission/BasePermission;->updateDynamicPermission(Ljava/util/Collection;)V
 HSPLcom/android/server/pm/permission/BasePermission;->writeLPr(Lorg/xmlpull/v1/XmlSerializer;)V
 HSPLcom/android/server/pm/permission/DefaultPermissionGrantPolicy$1;-><init>(Lcom/android/server/pm/permission/DefaultPermissionGrantPolicy;Landroid/os/Looper;)V

services/art-profile-boot

@@ -171,7 +171,6 @@ Lcom/android/server/pm/PackageUsage;->readToken(Ljava/io/InputStream;Ljava/lang/
 Lcom/android/server/pm/PackageUsage;->readVersion1LP(Ljava/util/Map;Ljava/io/InputStream;Ljava/lang/StringBuffer;)V
 Lcom/android/server/pm/PackageUsage;->parseAsLong(Ljava/lang/String;)J
 Lcom/android/server/pm/CompilerStats;->read(Ljava/io/Reader;)Z
-Lcom/android/server/pm/permission/BasePermission;->getSourcePackageSetting()Lcom/android/server/pm/PackageSettingBase;
 Lcom/android/server/pm/permission/PermissionManagerService;->updatePermissionSourcePackage(Ljava/lang/String;Landroid/content/pm/PackageParser$Package;)Z
 Lcom/android/server/pm/permission/BasePermission;->isDynamic()Z
 Lcom/android/server/pm/permission/PermissionManagerService;->cacheBackgroundToForegoundPermissionMapping()V

services/core/java/com/android/server/pm/PackageManagerService.java

@@ -17122,7 +17122,12 @@ public class PackageManagerService extends IPackageManager.Stub
                     // "updating same package" could also involve key-rotation.
                     final boolean sigsOk;
                     final String sourcePackageName = bp.getSourcePackageName();
-                    final PackageSettingBase sourcePackageSetting = bp.getSourcePackageSetting();
+                    final PackageSetting sourcePackageSetting;
+                    synchronized (mLock) {
+                        sourcePackageSetting = mSettings.getPackageLPr(sourcePackageName);
+                    }
+                    final SigningDetails sourceSigningDetails = (sourcePackageSetting == null
+                            ? SigningDetails.UNKNOWN : sourcePackageSetting.getSigningDetails());
                     final KeySetManagerService ksms = mSettings.mKeySetManagerService;
                     if (sourcePackageName.equals(parsedPackage.getPackageName())
                             && (ksms.shouldCheckUpgradeKeySetLocked(
@@ -17133,18 +17138,19 @@ public class PackageManagerService extends IPackageManager.Stub
                         // in the event of signing certificate rotation, we need to see if the
                         // package's certificate has rotated from the current one, or if it is an
                         // older certificate with which the current is ok with sharing permissions
-                        if (sourcePackageSetting.signatures.mSigningDetails.checkCapability(
+                        if (sourceSigningDetails.checkCapability(
                                 parsedPackage.getSigningDetails(),
                                 PackageParser.SigningDetails.CertCapabilities.PERMISSION)) {
                             sigsOk = true;
                         } else if (parsedPackage.getSigningDetails().checkCapability(
-                                sourcePackageSetting.signatures.mSigningDetails,
+                                sourceSigningDetails,
                                 PackageParser.SigningDetails.CertCapabilities.PERMISSION)) {
-
                             // the scanned package checks out, has signing certificate rotation
                             // history, and is newer; bring it over
+                            synchronized (mLock) {
                                 sourcePackageSetting.signatures.mSigningDetails =
                                         parsedPackage.getSigningDetails();
+                            }
                             sigsOk = true;
                         } else {
                             sigsOk = false;

services/core/java/com/android/server/pm/permission/BasePermission.java

@@ -31,7 +31,6 @@ import android.annotation.NonNull;
 import android.annotation.Nullable;
 import android.content.pm.PackageManagerInternal;
 import android.content.pm.PermissionInfo;
-import android.content.pm.Signature;
 import android.content.pm.parsing.component.ParsedPermission;
 import android.os.UserHandle;
 import android.util.Log;
@@ -86,9 +85,6 @@ public final class BasePermission {
 
     String sourcePackageName;
 
-    // TODO: Can we get rid of this? Seems we only use some signature info from the setting
-    PackageSettingBase sourcePackageSetting;
-
     int protectionLevel;
 
     ParsedPermission perm;
@@ -130,12 +126,6 @@ public final class BasePermission {
     public String getSourcePackageName() {
         return sourcePackageName;
     }
-    public PackageSettingBase getSourcePackageSetting() {
-        return sourcePackageSetting;
-    }
-    public Signature[] getSourceSignatures() {
-        return sourcePackageSetting.getSignatures();
-    }
     public int getType() {
         return type;
     }
@@ -149,9 +139,6 @@ public final class BasePermission {
     public void setPermission(@Nullable ParsedPermission perm) {
         this.perm = perm;
     }
-    public void setSourcePackageSetting(PackageSettingBase sourcePackageSetting) {
-        this.sourcePackageSetting = sourcePackageSetting;
-    }
 
     public int[] computeGids(int userId) {
         if (perUser) {
@@ -290,7 +277,6 @@ public final class BasePermission {
             return;
         }
         sourcePackageName = newPackageName;
-        sourcePackageSetting = null;
         perm = null;
         if (pendingPermissionInfo != null) {
             pendingPermissionInfo.packageName = newPackageName;
@@ -319,10 +305,9 @@ public final class BasePermission {
         if (PackageManagerService.DEBUG_SETTINGS) Log.v(TAG, "Dynamic permission: name="
                 + getName() + " pkg=" + getSourcePackageName()
                 + " info=" + pendingPermissionInfo);
-        if (sourcePackageSetting == null && pendingPermissionInfo != null) {
+        if (pendingPermissionInfo != null) {
             final BasePermission tree = findPermissionTree(permissionTrees, name);
             if (tree != null && tree.perm != null) {
-                sourcePackageSetting = tree.sourcePackageSetting;
                 perm = new ParsedPermission(tree.perm, pendingPermissionInfo,
                         tree.perm.getPackageName(), name);
                 uid = tree.uid;
@@ -355,7 +340,6 @@ public final class BasePermission {
                 if (bp.type == BasePermission.TYPE_BUILTIN && bp.perm == null) {
                     // It's a built-in permission and no owner, take ownership now
                     p.setFlags(p.getFlags() | PermissionInfo.FLAG_INSTALLED);
-                    bp.sourcePackageSetting = pkgSetting;
                     bp.perm = p;
                     bp.uid = pkg.getUid();
                     bp.sourcePackageName = p.getPackageName();
@@ -378,7 +362,6 @@ public final class BasePermission {
                 if (tree == null
                         || tree.sourcePackageName.equals(p.getPackageName())) {
                     p.setFlags(p.getFlags() | PermissionInfo.FLAG_INSTALLED);
-                    bp.sourcePackageSetting = pkgSetting;
                     bp.perm = p;
                     bp.uid = pkg.getUid();
                     bp.sourcePackageName = p.getPackageName();
@@ -639,9 +622,6 @@ public final class BasePermission {
                 pw.print("    flags=0x"); pw.println(Integer.toHexString(perm.getFlags()));
             }
         }
-        if (sourcePackageSetting != null) {
-            pw.print("    packageSetting="); pw.println(sourcePackageSetting);
-        }
         if (READ_EXTERNAL_STORAGE.equals(name)) {
             pw.print("    enforced=");
             pw.println(readEnforced);

services/core/java/com/android/server/pm/permission/PermissionManagerService.java

@@ -2526,7 +2526,7 @@ public class PermissionManagerService extends IPermissionManager.Stub {
                             + " checking " + permName + ": " + bp);
                 }
 
-                if (bp == null || bp.getSourcePackageSetting() == null) {
+                if (bp == null || getSourcePackageSetting(bp) == null) {
                     if (packageOfInterest == null || packageOfInterest.equals(
                             pkg.getPackageName())) {
                         if (DEBUG_PERMISSIONS) {
@@ -3403,10 +3403,11 @@ public class PermissionManagerService extends IPermissionManager.Stub {
         //     - or its signing certificate is a previous signing certificate of the defining
         //       package, and the defining package still trusts the old certificate for permissions
         //     - or it shares the above relationships with the system package
+        final PackageParser.SigningDetails sourceSigningDetails =
+                getSourcePackageSigningDetails(bp);
         boolean allowed =
-                pkg.getSigningDetails().hasAncestorOrSelf(
+                pkg.getSigningDetails().hasAncestorOrSelf(sourceSigningDetails)
-                        bp.getSourcePackageSetting().getSigningDetails())
+                || sourceSigningDetails.checkCapability(
-                || bp.getSourcePackageSetting().getSigningDetails().checkCapability(
                         pkg.getSigningDetails(),
                         PackageParser.SigningDetails.CertCapabilities.PERMISSION)
                 || pkg.getSigningDetails().hasAncestorOrSelf(systemPackage.getSigningDetails())
@@ -3576,6 +3577,22 @@ public class PermissionManagerService extends IPermissionManager.Stub {
         return allowed;
     }
 
+    @NonNull
+    private PackageParser.SigningDetails getSourcePackageSigningDetails(
+            @NonNull BasePermission bp) {
+        final PackageSetting ps = getSourcePackageSetting(bp);
+        if (ps == null) {
+            return PackageParser.SigningDetails.UNKNOWN;
+        }
+        return ps.getSigningDetails();
+    }
+
+    @Nullable
+    private PackageSetting getSourcePackageSetting(@NonNull BasePermission bp) {
+        final String sourcePackageName = bp.getSourcePackageName();
+        return mPackageManagerInt.getPackageSetting(sourcePackageName);
+    }
+
     private static boolean isProfileOwner(int uid) {
         DevicePolicyManagerInternal dpmInternal =
                 LocalServices.getService(DevicePolicyManagerInternal.class);
@@ -4084,8 +4101,10 @@ public class PermissionManagerService extends IPermissionManager.Stub {
                 if (bp.isDynamic()) {
                     bp.updateDynamicPermission(mSettings.mPermissionTrees.values());
                 }
-                if (bp.getSourcePackageSetting() == null
+                if (!packageName.equals(bp.getSourcePackageName())) {
-                        || !packageName.equals(bp.getSourcePackageName())) {
+                    // Not checking sourcePackageSetting because it can be null when
+                    // the permission source package is the target package and the target package is
+                    // being uninstalled,
                     continue;
                 }
                 // The target package is the source of the current permission
@@ -4123,9 +4142,6 @@ public class PermissionManagerService extends IPermissionManager.Stub {
                                 bp.getSourcePackageName());
                 synchronized (mLock) {
                     if (sourcePkg != null && sourcePs != null) {
-                        if (bp.getSourcePackageSetting() == null) {
-                            bp.setSourcePackageSetting(sourcePs);
-                        }
                         continue;
                     }
                     Slog.w(TAG, "Removing dangling permission: " + bp.getName()
@@ -4200,8 +4216,10 @@ public class PermissionManagerService extends IPermissionManager.Stub {
             final Iterator<BasePermission> it = mSettings.mPermissionTrees.values().iterator();
             while (it.hasNext()) {
                 final BasePermission bp = it.next();
-                if (bp.getSourcePackageSetting() == null
+                if (!packageName.equals(bp.getSourcePackageName())) {
-                        || !packageName.equals(bp.getSourcePackageName())) {
+                    // Not checking sourcePackageSetting because it can be null when
+                    // the permission source package is the target package and the target package is
+                    // being uninstalled,
                     continue;
                 }
                 // The target package is the source of the current permission tree
@@ -4227,9 +4245,6 @@ public class PermissionManagerService extends IPermissionManager.Stub {
                                 bp.getSourcePackageName());
                 synchronized (mLock) {
                     if (sourcePkg != null && sourcePs != null) {
-                        if (bp.getSourcePackageSetting() == null) {
-                            bp.setSourcePackageSetting(sourcePs);
-                        }
                         continue;
                     }
                     Slog.w(TAG, "Removing dangling permission tree: " + bp.getName()