Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

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

Browse Source
This commit is contained in:
Songchun Fan
2020-04-27 18:21:50 +00:00
committed by Android (Google) Code Review
parent 16ee6e725f e5c0afe110
commit 99a4daeaf1
5 changed files with 42 additions and 44 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
services/art-profile
View File

@@ -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

1
services/art-profile-boot
View File

@@ -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

18
services/core/java/com/android/server/pm/PackageManagerService.java
View File

@@ -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
sourcePackageSetting.signatures.mSigningDetails =
parsedPackage.getSigningDetails();
synchronized (mLock) {
sourcePackageSetting.signatures.mSigningDetails =
parsedPackage.getSigningDetails();
}
sigsOk = true;
} else {
sigsOk = false;

22
services/core/java/com/android/server/pm/permission/BasePermission.java
View File

@@ -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);

43
services/core/java/com/android/server/pm/permission/PermissionManagerService.java
View File

@@ -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(
bp.getSourcePackageSetting().getSigningDetails())
|| bp.getSourcePackageSetting().getSigningDetails().checkCapability(
pkg.getSigningDetails().hasAncestorOrSelf(sourceSigningDetails)
|| sourceSigningDetails.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
|| !packageName.equals(bp.getSourcePackageName())) {
if (!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
|| !packageName.equals(bp.getSourcePackageName())) {
if (!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()