From 10aa9fd06f491c3d3dbeabfd5040b08cf9f0767c Mon Sep 17 00:00:00 2001 From: Martijn Coenen Date: Thu, 3 Feb 2022 15:49:40 +0100 Subject: [PATCH 1/3] Make isCallerSameApp() correct for supplemental processes. UIDs from the new supplemental process range must always map back to the supplemental package. Bug: 215012578 Test: atest SupplementalProcessTests, TEST_MAPPING Change-Id: I867d163de74dd83a39936c6304a58888b921e992 --- services/core/java/com/android/server/pm/ComputerEngine.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/services/core/java/com/android/server/pm/ComputerEngine.java b/services/core/java/com/android/server/pm/ComputerEngine.java index c942a4357900e..2011528eabcad 100644 --- a/services/core/java/com/android/server/pm/ComputerEngine.java +++ b/services/core/java/com/android/server/pm/ComputerEngine.java @@ -2369,6 +2369,10 @@ public class ComputerEngine implements Computer { } public final boolean isCallerSameApp(String packageName, int uid) { + if (Process.isSupplemental(uid)) { + return (packageName != null + && packageName.equals(mService.getSupplementalProcessPackageName())); + } AndroidPackage pkg = mPackages.get(packageName); return pkg != null && UserHandle.getAppId(uid) == pkg.getUid(); From ccfec2282243e041d8f576648edceb28cc2b1dbc Mon Sep 17 00:00:00 2001 From: Martijn Coenen Date: Thu, 3 Feb 2022 15:43:32 +0100 Subject: [PATCH 2/3] Start supplemental processes in new UID range. When we start a supplemental process on behalf of an app, we want to start it in its designated UID range. To do that, modify ServiceRecord to keep track of which app we started the supplemental process for; then, modify the various startProcess calls to recognize supplemental processes and assign the correct UID. Bug: 215012578 Test: atest SupplementalProcessTests Change-Id: I6338666eaeb39f8775f38878e1db4221c1a0def0 --- .../com/android/server/am/ActiveServices.java | 27 ++++++++++------ .../server/am/ActivityManagerService.java | 31 +++++++++++++++---- .../com/android/server/am/ProcessList.java | 10 ++++-- .../com/android/server/am/ServiceRecord.java | 8 +++-- 4 files changed, 57 insertions(+), 19 deletions(-) diff --git a/services/core/java/com/android/server/am/ActiveServices.java b/services/core/java/com/android/server/am/ActiveServices.java index dc64d800c8c03..092172a158615 100644 --- a/services/core/java/com/android/server/am/ActiveServices.java +++ b/services/core/java/com/android/server/am/ActiveServices.java @@ -2721,8 +2721,8 @@ public final class ActiveServices { int bindServiceLocked(IApplicationThread caller, IBinder token, Intent service, String resolvedType, final IServiceConnection connection, int flags, - String instanceName, boolean isSupplementalProcessService, String callingPackage, - final int userId) + String instanceName, boolean isSupplementalProcessService, int supplementedAppUid, + String callingPackage, final int userId) throws TransactionTooLargeException { if (DEBUG_SERVICE) Slog.v(TAG_SERVICE, "bindService: " + service + " type=" + resolvedType + " conn=" + connection.asBinder() @@ -2807,8 +2807,8 @@ public final class ActiveServices { final boolean allowInstant = (flags & Context.BIND_ALLOW_INSTANT) != 0; ServiceLookupResult res = retrieveServiceLocked(service, instanceName, - isSupplementalProcessService, resolvedType, callingPackage, callingPid, callingUid, - userId, true, callerFg, isBindExternal, allowInstant); + isSupplementalProcessService, supplementedAppUid, resolvedType, callingPackage, + callingPid, callingUid, userId, true, callerFg, isBindExternal, allowInstant); if (res == null) { return 0; } @@ -3228,13 +3228,14 @@ public final class ActiveServices { int callingPid, int callingUid, int userId, boolean createIfNeeded, boolean callingFromFg, boolean isBindExternal, boolean allowInstant) { - return retrieveServiceLocked(service, instanceName, false, resolvedType, callingPackage, + return retrieveServiceLocked(service, instanceName, false, 0, resolvedType, callingPackage, callingPid, callingUid, userId, createIfNeeded, callingFromFg, isBindExternal, allowInstant); } private ServiceLookupResult retrieveServiceLocked(Intent service, - String instanceName, boolean isSupplementalProcessService, String resolvedType, + String instanceName, boolean isSupplementalProcessService, int supplementedAppUid, + String resolvedType, String callingPackage, int callingPid, int callingUid, int userId, boolean createIfNeeded, boolean callingFromFg, boolean isBindExternal, boolean allowInstant) { @@ -3415,7 +3416,7 @@ public final class ActiveServices { : null; r = new ServiceRecord(mAm, className, name, definingPackageName, definingUid, filter, sInfo, callingFromFg, res, - supplementalProcessName); + supplementalProcessName, supplementedAppUid); res.setService(r); smap.mServicesByInstanceName.put(name, r); smap.mServicesByIntent.put(filter, r); @@ -4189,8 +4190,16 @@ public final class ActiveServices { if (app == null && !permissionsReviewRequired && !packageFrozen) { // TODO (chriswailes): Change the Zygote policy flags based on if the launch-for-service // was initiated from a notification tap or not. - if ((app = mAm.startProcessLocked(procName, r.appInfo, true, intentFlags, - hostingRecord, ZYGOTE_POLICY_FLAG_EMPTY, false, isolated)) == null) { + if (r.supplemental) { + final int uid = Process.toSupplementalUid(r.supplementedAppUid); + app = mAm.startSupplementalProcessLocked(procName, r.appInfo, true, intentFlags, + hostingRecord, ZYGOTE_POLICY_FLAG_EMPTY, uid); + r.isolationHostProc = app; + } else { + app = mAm.startProcessLocked(procName, r.appInfo, true, intentFlags, + hostingRecord, ZYGOTE_POLICY_FLAG_EMPTY, false, isolated); + } + if (app == null) { String msg = "Unable to launch app " + r.appInfo.packageName + "/" + r.appInfo.uid + " for service " diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java index 54f462aa8ae05..a478c3115139d 100644 --- a/services/core/java/com/android/server/am/ActivityManagerService.java +++ b/services/core/java/com/android/server/am/ActivityManagerService.java @@ -1890,6 +1890,8 @@ public class ActivityManagerService extends IActivityManager.Stub synchronized (this) { ProcessRecord app = mProcessList.newProcessRecordLocked(info, info.processName, + false, + 0, false, 0, new HostingRecord("system")); @@ -2780,11 +2782,24 @@ public class ActivityManagerService extends IActivityManager.Stub false /* knownToBeDead */, 0 /* intentFlags */, sNullHostingRecord /* hostingRecord */, ZYGOTE_POLICY_FLAG_EMPTY, true /* allowWhileBooting */, true /* isolated */, - uid, abiOverride, entryPoint, entryPointArgs, crashHandler); + uid, false /* supplemental */, 0 /* supplementalUid */, + abiOverride, entryPoint, entryPointArgs, crashHandler); return proc != null; } } + @GuardedBy("this") + final ProcessRecord startSupplementalProcessLocked(String processName, + ApplicationInfo info, boolean knownToBeDead, int intentFlags, + HostingRecord hostingRecord, int zygotePolicyFlags, int supplementalUid) { + return mProcessList.startProcessLocked(processName, info, knownToBeDead, intentFlags, + hostingRecord, zygotePolicyFlags, false /* allowWhileBooting */, + false /* isolated */, 0 /* isolatedUid */, + true /* supplemental */, supplementalUid, + null /* ABI override */, null /* entryPoint */, + null /* entryPointArgs */, null /* crashHandler */); + } + @GuardedBy("this") final ProcessRecord startProcessLocked(String processName, ApplicationInfo info, boolean knownToBeDead, int intentFlags, @@ -2792,6 +2807,7 @@ public class ActivityManagerService extends IActivityManager.Stub boolean isolated) { return mProcessList.startProcessLocked(processName, info, knownToBeDead, intentFlags, hostingRecord, zygotePolicyFlags, allowWhileBooting, isolated, 0 /* isolatedUid */, + false /* supplemental */, 0 /* supplementalUid */, null /* ABI override */, null /* entryPoint */, null /* entryPointArgs */, null /* crashHandler */); } @@ -6521,6 +6537,7 @@ public class ActivityManagerService extends IActivityManager.Stub if (app == null) { app = mProcessList.newProcessRecordLocked(info, customProcess, isolated, 0, + false, 0, new HostingRecord("added application", customProcess != null ? customProcess : info.processName)); updateLruProcessLocked(app, false, null); @@ -12346,12 +12363,13 @@ public class ActivityManagerService extends IActivityManager.Stub String resolvedType, IServiceConnection connection, int flags, String instanceName, String callingPackage, int userId) throws TransactionTooLargeException { return bindServiceInstance(caller, token, service, resolvedType, connection, flags, - instanceName, false, callingPackage, userId); + instanceName, false, 0, callingPackage, userId); } private int bindServiceInstance(IApplicationThread caller, IBinder token, Intent service, String resolvedType, IServiceConnection connection, int flags, String instanceName, - boolean isSupplementalProcessService, String callingPackage, int userId) + boolean isSupplementalProcessService, int supplementedAppUid, String callingPackage, + int userId) throws TransactionTooLargeException { enforceNotIsolatedCaller("bindService"); @@ -12382,7 +12400,8 @@ public class ActivityManagerService extends IActivityManager.Stub synchronized(this) { return mServices.bindServiceLocked(caller, token, service, resolvedType, connection, - flags, instanceName, isSupplementalProcessService, callingPackage, userId); + flags, instanceName, isSupplementalProcessService, supplementedAppUid, + callingPackage, userId); } } @@ -15976,8 +15995,8 @@ public class ActivityManagerService extends IActivityManager.Stub return ActivityManagerService.this.bindServiceInstance( mContext.getIApplicationThread(), mContext.getActivityToken(), service, service.resolveTypeIfNeeded(mContext.getContentResolver()), sd, flags, - processName, /*isSupplementalProcessService*/ true, mContext.getOpPackageName(), - UserHandle.getUserId(userAppUid)) != 0; + processName, /*isSupplementalProcessService*/ true, userAppUid, + mContext.getOpPackageName(), UserHandle.getUserId(userAppUid)) != 0; } @Override diff --git a/services/core/java/com/android/server/am/ProcessList.java b/services/core/java/com/android/server/am/ProcessList.java index 1ad0bcea711c2..4539cc8e05a24 100644 --- a/services/core/java/com/android/server/am/ProcessList.java +++ b/services/core/java/com/android/server/am/ProcessList.java @@ -2525,6 +2525,7 @@ public final class ProcessList { ProcessRecord startProcessLocked(String processName, ApplicationInfo info, boolean knownToBeDead, int intentFlags, HostingRecord hostingRecord, int zygotePolicyFlags, boolean allowWhileBooting, boolean isolated, int isolatedUid, + boolean supplemental, int supplementalUid, String abiOverride, String entryPoint, String[] entryPointArgs, Runnable crashHandler) { long startTime = SystemClock.uptimeMillis(); ProcessRecord app; @@ -2618,7 +2619,8 @@ public final class ProcessList { if (app == null) { checkSlow(startTime, "startProcess: creating new process record"); - app = newProcessRecordLocked(info, processName, isolated, isolatedUid, hostingRecord); + app = newProcessRecordLocked(info, processName, isolated, isolatedUid, supplemental, + supplementalUid, hostingRecord); if (app == null) { Slog.w(TAG, "Failed making new process record for " + processName + "/" + info.uid + " isolated=" + isolated); @@ -3113,10 +3115,14 @@ public final class ProcessList { @GuardedBy("mService") ProcessRecord newProcessRecordLocked(ApplicationInfo info, String customProcess, - boolean isolated, int isolatedUid, HostingRecord hostingRecord) { + boolean isolated, int isolatedUid, boolean supplemental, int supplementalUid, + HostingRecord hostingRecord) { String proc = customProcess != null ? customProcess : info.processName; final int userId = UserHandle.getUserId(info.uid); int uid = info.uid; + if (supplemental) { + uid = supplementalUid; + } if (isolated) { if (isolatedUid == 0) { IsolatedUidRange uidRange = getOrCreateIsolatedUidRangeLocked(info, hostingRecord); diff --git a/services/core/java/com/android/server/am/ServiceRecord.java b/services/core/java/com/android/server/am/ServiceRecord.java index d3b57529834a0..711c57669fd6e 100644 --- a/services/core/java/com/android/server/am/ServiceRecord.java +++ b/services/core/java/com/android/server/am/ServiceRecord.java @@ -94,6 +94,8 @@ final class ServiceRecord extends Binder implements ComponentName.WithComponentN final boolean exported; // from ServiceInfo.exported final Runnable restarter; // used to schedule retries of starting the service final long createRealTime; // when this service was created + final boolean supplemental; // whether this is a supplemental service + final int supplementedAppUid; // the app uid for which this supplemental service is running final ArrayMap bindings = new ArrayMap(); // All active bindings to the service. @@ -571,13 +573,13 @@ final class ServiceRecord extends Binder implements ComponentName.WithComponentN Intent.FilterComparison intent, ServiceInfo sInfo, boolean callerIsFg, Runnable restarter) { this(ams, name, instanceName, definingPackageName, definingUid, intent, sInfo, callerIsFg, - restarter, null); + restarter, null, 0); } ServiceRecord(ActivityManagerService ams, ComponentName name, ComponentName instanceName, String definingPackageName, int definingUid, Intent.FilterComparison intent, ServiceInfo sInfo, boolean callerIsFg, - Runnable restarter, String supplementalProcessName) { + Runnable restarter, String supplementalProcessName, int supplementedAppUid) { this.ams = ams; this.name = name; this.instanceName = instanceName; @@ -588,6 +590,8 @@ final class ServiceRecord extends Binder implements ComponentName.WithComponentN serviceInfo = sInfo; appInfo = sInfo.applicationInfo; packageName = sInfo.applicationInfo.packageName; + supplemental = supplementalProcessName != null; + this.supplementedAppUid = supplementedAppUid; if ((sInfo.flags & ServiceInfo.FLAG_ISOLATED_PROCESS) != 0) { processName = sInfo.processName + ":" + instanceName.getClassName(); } else if (supplementalProcessName != null) { From 130738f15ca6ee54909971df01340bc5b31cc06c Mon Sep 17 00:00:00 2001 From: Martijn Coenen Date: Thu, 3 Feb 2022 19:40:40 +0100 Subject: [PATCH 3/3] Handle supplemental UIDs in package/UID verification. Supplemental UIDs are processes that are spawned alongside regular app processes. These supplemental processes all share the same package name; allow this package name when verifying packageName / UID combinations for the supplemental UID range. Bug: 215012578 Test: atest AppOpsTests CtsAppOpsTestCases Change-Id: I10df1eff13b789caca91826d997c8c6e1cf241ed --- .../android/server/appop/AppOpsService.java | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/services/core/java/com/android/server/appop/AppOpsService.java b/services/core/java/com/android/server/appop/AppOpsService.java index 40fda4cbec5e2..cebcc64f7d5e1 100644 --- a/services/core/java/com/android/server/appop/AppOpsService.java +++ b/services/core/java/com/android/server/appop/AppOpsService.java @@ -4549,6 +4549,26 @@ public class AppOpsService extends IAppOpsService.Stub { return new PackageVerificationResult(null, /* isAttributionTagValid */ true); } + if (Process.isSupplemental(uid)) { + // Supplemental processes run in their own UID range, but their associated + // UID for checks should always be the UID of the supplemental package. + // TODO: We will need to modify the callers of this function instead, so + // modifications and checks against the app ops state are done with the + // correct UID. + try { + final PackageManager pm = mContext.getPackageManager(); + final String supplementalPackageName = pm.getSupplementalProcessPackageName(); + if (Objects.equals(packageName, supplementalPackageName)) { + int supplementalAppId = pm.getPackageUid(supplementalPackageName, + PackageManager.PackageInfoFlags.of(0)); + uid = UserHandle.getUid(UserHandle.getUserId(uid), supplementalAppId); + } + } catch (PackageManager.NameNotFoundException e) { + // Shouldn't happen for the supplemental package + e.printStackTrace(); + } + } + // Do not check if uid/packageName/attributionTag is already known. synchronized (this) {