From 480275a1977bcace74963d83c70c4c9e53d3a029 Mon Sep 17 00:00:00 2001 From: Galia Peycheva Date: Wed, 18 Mar 2020 17:33:42 +0100 Subject: Add verification for setting ACTIVITY_TYPE_DREAM Bug: 152281628 Test: m && flash && check that dream starts normally Change-Id: Ie0d8849aeb0c2de46a0e36bb0f9958091e0d28f4 --- .../service/dreams/DreamManagerInternal.java | 6 +++++ .../android/server/dreams/DreamManagerService.java | 10 ++++++-- .../java/com/android/server/wm/ActivityRecord.java | 27 ++++++++++++++++++++++ .../server/wm/ActivityTaskManagerInternal.java | 6 +++++ .../server/wm/ActivityTaskManagerService.java | 21 +++++++++++++++++ 5 files changed, 68 insertions(+), 2 deletions(-) diff --git a/core/java/android/service/dreams/DreamManagerInternal.java b/core/java/android/service/dreams/DreamManagerInternal.java index 41fdd0bfe477..7bf5c38b7fd4 100644 --- a/core/java/android/service/dreams/DreamManagerInternal.java +++ b/core/java/android/service/dreams/DreamManagerInternal.java @@ -49,6 +49,12 @@ public abstract class DreamManagerInternal { * Called by the ActivityTaskManagerService to verify that the startDreamActivity * request comes from the current active dream component. * + * This function and its call path should not acquire the DreamManagerService lock + * to avoid deadlock with the ActivityTaskManager lock. + * + * TODO: Make this interaction push-based - the DreamManager should inform the + * ActivityTaskManager whenever the active dream component changes. + * * @param doze If true returns the current active doze component. Otherwise, returns the * active dream component. */ diff --git a/services/core/java/com/android/server/dreams/DreamManagerService.java b/services/core/java/com/android/server/dreams/DreamManagerService.java index eb0257e95b6c..bcf262d97235 100644 --- a/services/core/java/com/android/server/dreams/DreamManagerService.java +++ b/services/core/java/com/android/server/dreams/DreamManagerService.java @@ -51,6 +51,7 @@ import com.android.internal.util.DumpUtils; import com.android.server.FgThread; import com.android.server.LocalServices; import com.android.server.SystemService; +import com.android.server.wm.ActivityTaskManagerInternal; import java.io.FileDescriptor; import java.io.PrintWriter; @@ -75,6 +76,7 @@ public final class DreamManagerService extends SystemService { private final PowerManager mPowerManager; private final PowerManagerInternal mPowerManagerInternal; private final PowerManager.WakeLock mDozeWakeLock; + private final ActivityTaskManagerInternal mAtmInternal; private Binder mCurrentDreamToken; private ComponentName mCurrentDreamName; @@ -97,6 +99,7 @@ public final class DreamManagerService extends SystemService { mPowerManager = (PowerManager)context.getSystemService(Context.POWER_SERVICE); mPowerManagerInternal = getLocalService(PowerManagerInternal.class); + mAtmInternal = getLocalService(ActivityTaskManagerInternal.class); mDozeWakeLock = mPowerManager.newWakeLock(PowerManager.DOZE_WAKE_LOCK, TAG); mDozeConfig = new AmbientDisplayConfiguration(mContext); } @@ -383,8 +386,10 @@ public final class DreamManagerService extends SystemService { PowerManager.WakeLock wakeLock = mPowerManager .newWakeLock(PowerManager.PARTIAL_WAKE_LOCK, "startDream"); - mHandler.post(wakeLock.wrap( - () -> mController.startDream(newToken, name, isTest, canDoze, userId, wakeLock))); + mHandler.post(wakeLock.wrap(() -> { + mAtmInternal.notifyDreamStateChanged(true); + mController.startDream(newToken, name, isTest, canDoze, userId, wakeLock); + })); } private void stopDreamLocked(final boolean immediate) { @@ -422,6 +427,7 @@ public final class DreamManagerService extends SystemService { } mCurrentDreamDozeScreenState = Display.STATE_UNKNOWN; mCurrentDreamDozeScreenBrightness = PowerManager.BRIGHTNESS_DEFAULT; + mAtmInternal.notifyDreamStateChanged(false); } private void checkPermission(String permission) { diff --git a/services/core/java/com/android/server/wm/ActivityRecord.java b/services/core/java/com/android/server/wm/ActivityRecord.java index 78d6e27d067e..a0377516762c 100644 --- a/services/core/java/com/android/server/wm/ActivityRecord.java +++ b/services/core/java/com/android/server/wm/ActivityRecord.java @@ -36,6 +36,7 @@ import static android.app.AppOpsManager.MODE_ALLOWED; import static android.app.AppOpsManager.OP_PICTURE_IN_PICTURE; import static android.app.WaitResult.INVALID_DELAY; import static android.app.WindowConfiguration.ACTIVITY_TYPE_ASSISTANT; +import static android.app.WindowConfiguration.ACTIVITY_TYPE_DREAM; import static android.app.WindowConfiguration.ACTIVITY_TYPE_HOME; import static android.app.WindowConfiguration.ACTIVITY_TYPE_RECENTS; import static android.app.WindowConfiguration.ACTIVITY_TYPE_UNDEFINED; @@ -269,6 +270,8 @@ import android.os.SystemClock; import android.os.Trace; import android.os.UserHandle; import android.os.storage.StorageManager; +import android.service.dreams.DreamActivity; +import android.service.dreams.DreamManagerInternal; import android.service.voice.IVoiceInteractionSession; import android.util.ArraySet; import android.util.EventLog; @@ -2035,6 +2038,26 @@ final class ActivityRecord extends WindowToken implements WindowManagerService.A return false; } + static boolean canLaunchDreamActivity(String packageName) { + final DreamManagerInternal dreamManager = + LocalServices.getService(DreamManagerInternal.class); + + // Verify that the package is the current active dream. The getActiveDreamComponent() + // call path does not acquire the DreamManager lock and thus is safe to use. + final ComponentName activeDream = dreamManager.getActiveDreamComponent(false /* doze */); + if (activeDream == null || activeDream.getPackageName() == null + || !activeDream.getPackageName().equals(packageName)) { + return false; + } + + // Verify that the device is dreaming. + if (!LocalServices.getService(ActivityTaskManagerInternal.class).isDreaming()) { + return false; + } + + return true; + } + private void setActivityType(boolean componentSpecified, int launchedFromUid, Intent intent, ActivityOptions options, ActivityRecord sourceRecord) { int activityType = ACTIVITY_TYPE_UNDEFINED; @@ -2054,6 +2077,10 @@ final class ActivityRecord extends WindowToken implements WindowManagerService.A } else if (options != null && options.getLaunchActivityType() == ACTIVITY_TYPE_ASSISTANT && canLaunchAssistActivity(launchedFromPackage)) { activityType = ACTIVITY_TYPE_ASSISTANT; + } else if (options != null && options.getLaunchActivityType() == ACTIVITY_TYPE_DREAM + && canLaunchDreamActivity(launchedFromPackage) + && DreamActivity.class.getName() == info.name) { + activityType = ACTIVITY_TYPE_DREAM; } setActivityType(activityType); } diff --git a/services/core/java/com/android/server/wm/ActivityTaskManagerInternal.java b/services/core/java/com/android/server/wm/ActivityTaskManagerInternal.java index 2263795f9442..d48df9ff8e6c 100644 --- a/services/core/java/com/android/server/wm/ActivityTaskManagerInternal.java +++ b/services/core/java/com/android/server/wm/ActivityTaskManagerInternal.java @@ -289,6 +289,11 @@ public abstract class ActivityTaskManagerInternal { */ public abstract void notifyActiveVoiceInteractionServiceChanged(ComponentName component); + /** + * Called when the device changes its dreaming state. + */ + public abstract void notifyDreamStateChanged(boolean dreaming); + /** * Set a uid that is allowed to bypass stopped app switches, launching an app * whenever it wants. @@ -318,6 +323,7 @@ public abstract class ActivityTaskManagerInternal { public abstract void clearHeavyWeightProcessIfEquals(WindowProcessController proc); public abstract void finishHeavyWeightApp(); + public abstract boolean isDreaming(); public abstract boolean isSleeping(); public abstract boolean isShuttingDown(); public abstract boolean shuttingDown(boolean booted, int timeout); diff --git a/services/core/java/com/android/server/wm/ActivityTaskManagerService.java b/services/core/java/com/android/server/wm/ActivityTaskManagerService.java index 69218164c0ad..e83ac7be8985 100644 --- a/services/core/java/com/android/server/wm/ActivityTaskManagerService.java +++ b/services/core/java/com/android/server/wm/ActivityTaskManagerService.java @@ -598,6 +598,13 @@ public class ActivityTaskManagerService extends IActivityTaskManager.Stub { */ private boolean mSleeping = false; + /** + * The mDreaming state is set by the {@link DreamManagerService} when it receives a request to + * start/stop the dream. It is set to true shortly before the {@link DreamService} is started. + * It is set to false after the {@link DreamService} is stopped. + */ + private boolean mDreaming = false; + /** * The process state used for processes that are running the top activities. * This changes between TOP and TOP_SLEEPING to following mSleeping. @@ -6330,6 +6337,13 @@ public class ActivityTaskManagerService extends IActivityTaskManager.Stub { } } + @Override + public void notifyDreamStateChanged(boolean dreaming) { + synchronized (mGlobalLock) { + mDreaming = dreaming; + } + } + @Override public void setAllowAppSwitches(@NonNull String type, int uid, int userId) { if (!mAmInternal.isUserRunning(userId, ActivityManager.FLAG_OR_STOPPED)) { @@ -6432,6 +6446,13 @@ public class ActivityTaskManagerService extends IActivityTaskManager.Stub { } } + @Override + public boolean isDreaming() { + synchronized (mGlobalLock) { + return mDreaming; + } + } + @HotPath(caller = HotPath.OOM_ADJUSTMENT) @Override public boolean isSleeping() { -- cgit v1.2.3-59-g8ed1b From ed401cc16d27d90c78b9744b580f00529e391e9a Mon Sep 17 00:00:00 2001 From: Galia Peycheva Date: Thu, 19 Mar 2020 20:28:09 +0100 Subject: Introduce security checks to startDreamActivity This CL checks that the caller is the dream package and that the device is dreaming when the activity is started. isDream was instroduced in order to exempt the DreamActivity from background start check. setAllowBackgroundActivityStart does that more cleanly. Bug: 152281628 Test: m && flashall && verify dream works Change-Id: Id6161f923f3350d482b3452a78a792a541dcf1a1 --- core/java/android/service/dreams/DreamService.java | 1 + .../com/android/server/wm/ActivityStarter.java | 16 +---- .../server/wm/ActivityTaskManagerService.java | 75 +++++++++++++--------- .../java/com/android/server/wm/AppTaskImpl.java | 2 +- 4 files changed, 47 insertions(+), 47 deletions(-) diff --git a/core/java/android/service/dreams/DreamService.java b/core/java/android/service/dreams/DreamService.java index c9be1c1fabb2..4aae3da7e8cf 100644 --- a/core/java/android/service/dreams/DreamService.java +++ b/core/java/android/service/dreams/DreamService.java @@ -1049,6 +1049,7 @@ public class DreamService extends Service implements Window.Callback { // DreamServiceWrapper.onActivityCreated. if (!mWindowless) { Intent i = new Intent(this, DreamActivity.class); + i.setPackage(getApplicationContext().getPackageName()); i.setFlags(Intent.FLAG_ACTIVITY_NEW_TASK); i.putExtra(DreamActivity.EXTRA_CALLBACK, mDreamServiceWrapper); diff --git a/services/core/java/com/android/server/wm/ActivityStarter.java b/services/core/java/com/android/server/wm/ActivityStarter.java index da1c045dba16..d63dc192b58e 100644 --- a/services/core/java/com/android/server/wm/ActivityStarter.java +++ b/services/core/java/com/android/server/wm/ActivityStarter.java @@ -341,7 +341,6 @@ class ActivityStarter { int filterCallingUid; PendingIntentRecord originatingPendingIntent; boolean allowBackgroundActivityStart; - boolean isDream; /** * If set to {@code true}, allows this activity start to look into @@ -393,7 +392,6 @@ class ActivityStarter { filterCallingUid = UserHandle.USER_NULL; originatingPendingIntent = null; allowBackgroundActivityStart = false; - isDream = false; } /** @@ -434,7 +432,6 @@ class ActivityStarter { filterCallingUid = request.filterCallingUid; originatingPendingIntent = request.originatingPendingIntent; allowBackgroundActivityStart = request.allowBackgroundActivityStart; - isDream = request.isDream; } /** @@ -985,7 +982,7 @@ class ActivityStarter { restrictedBgActivity = shouldAbortBackgroundActivityStart(callingUid, callingPid, callingPackage, realCallingUid, realCallingPid, callerApp, request.originatingPendingIntent, request.allowBackgroundActivityStart, - request.isDream, intent); + intent); } finally { Trace.traceEnd(Trace.TRACE_TAG_WINDOW_MANAGER); } @@ -1195,7 +1192,7 @@ class ActivityStarter { boolean shouldAbortBackgroundActivityStart(int callingUid, int callingPid, final String callingPackage, int realCallingUid, int realCallingPid, WindowProcessController callerApp, PendingIntentRecord originatingPendingIntent, - boolean allowBackgroundActivityStart, boolean isDream, Intent intent) { + boolean allowBackgroundActivityStart, Intent intent) { // don't abort for the most important UIDs final int callingAppId = UserHandle.getAppId(callingUid); if (callingUid == Process.ROOT_UID || callingAppId == Process.SYSTEM_UID @@ -1203,10 +1200,6 @@ class ActivityStarter { return false; } - // don't abort if this is the dream activity - if (isDream) { - return false; - } // don't abort if the callingUid has a visible window or is a persistent system process final int callingUidProcState = mService.getUidState(callingUid); final boolean callingUidHasAnyVisibleWindow = @@ -2717,11 +2710,6 @@ class ActivityStarter { return this; } - ActivityStarter setIsDream(boolean isDream) { - mRequest.isDream = isDream; - return this; - } - void dump(PrintWriter pw, String prefix) { prefix = prefix + " "; pw.print(prefix); diff --git a/services/core/java/com/android/server/wm/ActivityTaskManagerService.java b/services/core/java/com/android/server/wm/ActivityTaskManagerService.java index e83ac7be8985..a75420e2a84d 100644 --- a/services/core/java/com/android/server/wm/ActivityTaskManagerService.java +++ b/services/core/java/com/android/server/wm/ActivityTaskManagerService.java @@ -212,7 +212,6 @@ import android.os.storage.IStorageManager; import android.os.storage.StorageManager; import android.provider.Settings; import android.service.dreams.DreamActivity; -import android.service.dreams.DreamManagerInternal; import android.service.voice.IVoiceInteractionSession; import android.service.voice.VoiceInteractionManagerInternal; import android.sysprop.DisplayProperties; @@ -1245,36 +1244,29 @@ public class ActivityTaskManagerService extends IActivityTaskManager.Stub { } } - @Override - public boolean startDreamActivity(Intent intent) { - final WindowProcessController process = mProcessMap.getProcess(Binder.getCallingPid()); + private void enforceCallerIsDream(String callerPackageName) { final long origId = Binder.clearCallingIdentity(); - - // The dream activity is only called for non-doze dreams. - final ComponentName currentDream = LocalServices.getService(DreamManagerInternal.class) - .getActiveDreamComponent(/* doze= */ false); - - if (currentDream == null || currentDream.getPackageName() == null - || !currentDream.getPackageName().equals(process.mInfo.packageName)) { - Slog.e(TAG, "Calling package is not the current dream package. " - + "Aborting startDreamActivity..."); - return false; + try { + if (!ActivityRecord.canLaunchDreamActivity(callerPackageName)) { + throw new SecurityException("The dream activity can be started only when the device" + + " is dreaming and only by the active dream package."); + } + } finally { + Binder.restoreCallingIdentity(origId); } + } + + @Override + public boolean startDreamActivity(@NonNull Intent intent) { + assertPackageMatchesCallingUid(intent.getPackage()); + enforceCallerIsDream(intent.getPackage()); final ActivityInfo a = new ActivityInfo(); a.theme = com.android.internal.R.style.Theme_Dream; a.exported = true; a.name = DreamActivity.class.getName(); - - - a.packageName = process.mInfo.packageName; - a.applicationInfo = process.mInfo; - a.processName = process.mInfo.processName; - a.uiOptions = process.mInfo.uiOptions; - a.taskAffinity = "android:" + a.packageName + "/dream"; a.enabled = true; a.launchMode = ActivityInfo.LAUNCH_SINGLE_INSTANCE; - a.persistableMode = ActivityInfo.PERSIST_NEVER; a.screenOrientation = ActivityInfo.SCREEN_ORIENTATION_UNSPECIFIED; a.colorMode = ActivityInfo.COLOR_MODE_DEFAULT; @@ -1283,15 +1275,34 @@ public class ActivityTaskManagerService extends IActivityTaskManager.Stub { final ActivityOptions options = ActivityOptions.makeBasic(); options.setLaunchActivityType(ACTIVITY_TYPE_DREAM); - try { - getActivityStartController().obtainStarter(intent, "dream") - .setActivityInfo(a) - .setActivityOptions(options.toBundle()) - .setIsDream(true) - .execute(); - return true; - } finally { - Binder.restoreCallingIdentity(origId); + synchronized (mGlobalLock) { + final WindowProcessController process = mProcessMap.getProcess(Binder.getCallingPid()); + + a.packageName = process.mInfo.packageName; + a.applicationInfo = process.mInfo; + a.processName = process.mInfo.processName; + a.uiOptions = process.mInfo.uiOptions; + a.taskAffinity = "android:" + a.packageName + "/dream"; + + final int callingUid = Binder.getCallingUid(); + final int callingPid = Binder.getCallingPid(); + + final long origId = Binder.clearCallingIdentity(); + try { + getActivityStartController().obtainStarter(intent, "dream") + .setCallingUid(callingUid) + .setCallingPid(callingPid) + .setActivityInfo(a) + .setActivityOptions(options.toBundle()) + // To start the dream from background, we need to start it from a persistent + // system process. Here we set the real calling uid to the system server uid + .setRealCallingUid(Binder.getCallingUid()) + .setAllowBackgroundActivityStart(true) + .execute(); + return true; + } finally { + Binder.restoreCallingIdentity(origId); + } } } @@ -2478,7 +2489,7 @@ public class ActivityTaskManagerService extends IActivityTaskManager.Stub { final ActivityStarter starter = getActivityStartController().obtainStarter( null /* intent */, "moveTaskToFront"); if (starter.shouldAbortBackgroundActivityStart(callingUid, callingPid, callingPackage, -1, - -1, callerApp, null, false, false, null)) { + -1, callerApp, null, false, null)) { if (!isBackgroundActivityStartsEnabled()) { return; } diff --git a/services/core/java/com/android/server/wm/AppTaskImpl.java b/services/core/java/com/android/server/wm/AppTaskImpl.java index 4cce212cb779..8fa811915fc2 100644 --- a/services/core/java/com/android/server/wm/AppTaskImpl.java +++ b/services/core/java/com/android/server/wm/AppTaskImpl.java @@ -111,7 +111,7 @@ class AppTaskImpl extends IAppTask.Stub { final ActivityStarter starter = mService.getActivityStartController().obtainStarter( null /* intent */, "moveToFront"); if (starter.shouldAbortBackgroundActivityStart(callingUid, callingPid, - callingPackage, -1, -1, callerApp, null, false, false, null)) { + callingPackage, -1, -1, callerApp, null, false, null)) { if (!mService.isBackgroundActivityStartsEnabled()) { return; } -- cgit v1.2.3-59-g8ed1b