Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 9cbb20b7 authored by Michal Karpinski's avatar Michal Karpinski
Browse files

Fix bugs for background activity starts from notifications 

We were facing a few fundamental issues due to trampolines
from PendingIntents.

1) Temporarily whitelist processes of receivers kicked off with
performReceiveLocked() to start background activities too. Previously
we only whitelisted those going through
processCurBroadcastLocked()/finishReceiverLocked() pair.

2) If an activity is being started from PendingIntent, caller
object is not passed in, as the PendingIntent creator's process
might not be alive. In that case we make an effort to retrieve
the caller object for the PendingIntent sender, to make a decision
whether to open the activity based on its foreground/whitelisted
state.

3) Whitelist processes for services also in the cases where the
process isn't alive yet - previously we were only whitelisting if
the process was up, which might often not be the case for trampolines
from notifications. Now we also whitelist the processes that were
brought up explicitly to start the service in question.

Bug: 123404215
Bug: 123677969
Bug: 123688728
Bug: 110956953
Test: atest WmTests:ActivityStarterTests
Test: manual with the citymapper notification that caused the bug
Test: manual with dynamite notifications after killing dynamite
      process
Change-Id: I2ddc6701dde064177ff47a9c43efc0c118565ce2
parent 79282ebc

services/core/java/com/android/server/am/ActiveServices.java

+21 −9
Original line number Diff line number Diff line
@@ -333,7 +333,8 @@ public final class ActiveServices { 
                }

                r.delayed = false;

                try {

                    startServiceInnerLocked(this, r.pendingStarts.get(0).intent, r, false, true);

                    startServiceInnerLocked(this, r.pendingStarts.get(0).intent, r, false, true,

                            false);

                } catch (TransactionTooLargeException e) {

                    // Ignore, nobody upstack cares.

                }
@@ -643,7 +644,8 @@ public final class ActiveServices { 
                        SERVICE_BG_ACTIVITY_START_TIMEOUT_MS);

            }

        }

        ComponentName cmp = startServiceInnerLocked(smap, service, r, callerFg, addToStarting);

        ComponentName cmp = startServiceInnerLocked(smap, service, r, callerFg, addToStarting,

                allowBackgroundActivityStarts);

        return cmp;

    }
@@ -702,7 +704,8 @@ public final class ActiveServices { 
    }



    ComponentName startServiceInnerLocked(ServiceMap smap, Intent service, ServiceRecord r,

            boolean callerFg, boolean addToStarting) throws TransactionTooLargeException {

            boolean callerFg, boolean addToStarting, boolean allowBackgroundActivityStarts)

            throws TransactionTooLargeException {

        ServiceState stracker = r.getTracker();

        if (stracker != null) {

            stracker.setStarted(true, mAm.mProcessStats.getMemFactorLocked(), r.lastActivity);
@@ -713,7 +716,8 @@ public final class ActiveServices { 
        synchronized (r.stats.getBatteryStats()) {

            r.stats.startRunningLocked();

        }

        String error = bringUpServiceLocked(r, service.getFlags(), callerFg, false, false);

        String error = bringUpServiceLocked(r, service.getFlags(), callerFg, false, false,

                allowBackgroundActivityStarts);

        if (error != null) {

            return new ComponentName("!!", error);

        }
@@ -1645,7 +1649,7 @@ public final class ActiveServices { 
                                try {

                                    bringUpServiceLocked(serviceRecord,

                                            serviceIntent.getFlags(),

                                            callerFg, false, false);

                                            callerFg, false, false, false);

                                } catch (RemoteException e) {

                                    /* ignore - local call */

                                }
@@ -1748,7 +1752,7 @@ public final class ActiveServices { 
            if ((flags&Context.BIND_AUTO_CREATE) != 0) {

                s.lastActivity = SystemClock.uptimeMillis();

                if (bringUpServiceLocked(s, service.getFlags(), callerFg, false,

                        permissionsReviewRequired) != null) {

                        permissionsReviewRequired, false) != null) {

                    return 0;

                }

            }
@@ -2418,7 +2422,8 @@ public final class ActiveServices { 
            return;

        }

        try {

            bringUpServiceLocked(r, r.intent.getIntent().getFlags(), r.createdFromFg, true, false);

            bringUpServiceLocked(r, r.intent.getIntent().getFlags(), r.createdFromFg, true, false,

                    false);

        } catch (TransactionTooLargeException e) {

            // Ignore, it's been logged and nothing upstack cares.

        }
@@ -2463,8 +2468,8 @@ public final class ActiveServices { 
    }



    private String bringUpServiceLocked(ServiceRecord r, int intentFlags, boolean execInFg,

            boolean whileRestarting, boolean permissionsReviewRequired)

            throws TransactionTooLargeException {

            boolean whileRestarting, boolean permissionsReviewRequired,

            boolean allowBackgroundActivityStarts) throws TransactionTooLargeException {

        //Slog.i(TAG, "Bring up service:");

        //r.dump("  ");
@@ -2575,6 +2580,13 @@ public final class ActiveServices { 
            }

        }



        if (app != null && allowBackgroundActivityStarts) {

            app.addAllowBackgroundActivityStartsToken(r);

            // schedule removal of the whitelisting token after the timeout

            removeAllowBackgroundActivityStartsServiceToken(app, r,

                    SERVICE_BG_ACTIVITY_START_TIMEOUT_MS);

        }



        if (r.fgRequired) {

            if (DEBUG_FOREGROUND_SERVICE) {

                Slog.v(TAG, "Whitelisting " + UserHandle.formatUid(r.appInfo.uid)

services/core/java/com/android/server/am/ActivityManagerService.java

+1 −1
Original line number Diff line number Diff line
@@ -14997,7 +14997,7 @@ public class ActivityManagerService extends IActivityManager.Stub 
                        oldQueue.performReceiveLocked(oldRecord.callerApp, oldRecord.resultTo,

                                oldRecord.intent,

                                Activity.RESULT_CANCELED, null, null,

                                false, false, oldRecord.userId);

                                false, false, oldRecord.userId, oldRecord);

                    } catch (RemoteException e) {

                        Slog.w(TAG, "Failure ["

                                + queue.mQueueName + "] sending broadcast result of "

services/core/java/com/android/server/am/BroadcastQueue.java

+16 −3
Original line number Diff line number Diff line
@@ -74,6 +74,9 @@ public final class BroadcastQueue { 
    static final int MAX_BROADCAST_SUMMARY_HISTORY

            = ActivityManager.isLowRamDeviceStatic() ? 25 : 300;



    // For how long after a whitelisted receiver's start its process can start a background activity

    private static final int RECEIVER_BG_ACTIVITY_START_TIMEOUT_MS = 10_000;



    final ActivityManagerService mService;



    /**
@@ -551,13 +554,23 @@ public final class BroadcastQueue { 


    void performReceiveLocked(ProcessRecord app, IIntentReceiver receiver,

            Intent intent, int resultCode, String data, Bundle extras,

            boolean ordered, boolean sticky, int sendingUser) throws RemoteException {

            boolean ordered, boolean sticky, int sendingUser, BroadcastRecord br)

            throws RemoteException {

        // Send the intent to the receiver asynchronously using one-way binder calls.

        if (app != null) {

            if (app.thread != null) {

                // If we have an app thread, do the call through that so it is

                // correctly ordered with other one-way calls.

                try {

                    if (br.allowBackgroundActivityStarts) {

                        app.addAllowBackgroundActivityStartsToken(br);

                        // schedule removal of the whitelisting token after the timeout

                        mHandler.postDelayed(() -> {

                            if (app != null) {

                                app.removeAllowBackgroundActivityStartsToken(br);

                            }

                        }, RECEIVER_BG_ACTIVITY_START_TIMEOUT_MS);

                    }

                    app.thread.scheduleRegisteredReceiver(receiver, intent, resultCode,

                            data, extras, ordered, sticky, sendingUser, app.getReportedProcState());

                // TODO: Uncomment this when (b/28322359) is fixed and we aren't getting
@@ -783,7 +796,7 @@ public final class BroadcastQueue { 
            } else {

                performReceiveLocked(filter.receiverList.app, filter.receiverList.receiver,

                        new Intent(r.intent), r.resultCode, r.resultData,

                        r.resultExtras, r.ordered, r.initialSticky, r.userId);

                        r.resultExtras, r.ordered, r.initialSticky, r.userId, r);

            }

            if (ordered) {

                r.state = BroadcastRecord.CALL_DONE_RECEIVE;
@@ -1082,7 +1095,7 @@ public final class BroadcastQueue { 
                            }

                            performReceiveLocked(r.callerApp, r.resultTo,

                                    new Intent(r.intent), r.resultCode,

                                    r.resultData, r.resultExtras, false, false, r.userId);

                                    r.resultData, r.resultExtras, false, false, r.userId, r);

                            // Set this to null so that the reference

                            // (local and remote) isn't kept in the mBroadcastHistory.

                            r.resultTo = null;

services/core/java/com/android/server/am/PendingIntentRecord.java

+3 −0
Original line number Diff line number Diff line
@@ -384,6 +384,9 @@ public final class PendingIntentRecord extends IIntentSender.Stub { 
            final boolean allowTrampoline = uid != callingUid

                    && controller.mAtmInternal.isUidForeground(callingUid);



            // note: we on purpose don't pass in the information about the PendingIntent's creator,

            // like pid or ProcessRecord, to the ActivityTaskManagerInternal calls below, because

            // it's not unusual for the creator's process to not be alive at this time

            switch (key.type) {

                case ActivityManager.INTENT_SENDER_ACTIVITY:

                    try {

services/core/java/com/android/server/wm/ActivityStarter.java

+26 −16
Original line number Diff line number Diff line
@@ -753,8 +753,8 @@ class ActivityStarter { 
                Trace.traceBegin(Trace.TRACE_TAG_ACTIVITY_MANAGER,

                        "shouldAbortBackgroundActivityStart");

                abortBackgroundStart = shouldAbortBackgroundActivityStart(callingUid, callingPid,

                        callingPackage, realCallingUid, callerApp, originatingPendingIntent,

                        allowBackgroundActivityStart, intent);

                        callingPackage, realCallingUid, realCallingPid, callerApp,

                        originatingPendingIntent, allowBackgroundActivityStart, intent);

            } finally {

                Trace.traceEnd(Trace.TRACE_TAG_ACTIVITY_MANAGER);

            }
@@ -914,22 +914,14 @@ class ActivityStarter { 
    }



    private boolean shouldAbortBackgroundActivityStart(int callingUid, int callingPid,

            final String callingPackage, int realCallingUid, WindowProcessController callerApp,

            PendingIntentRecord originatingPendingIntent, boolean allowBackgroundActivityStart,

            Intent intent) {

            final String callingPackage, int realCallingUid, int realCallingPid,

            WindowProcessController callerApp, PendingIntentRecord originatingPendingIntent,

            boolean allowBackgroundActivityStart, Intent intent) {

        // don't abort for the most important UIDs

        if (callingUid == Process.ROOT_UID || callingUid == Process.SYSTEM_UID

                || callingUid == Process.NFC_UID) {

            return false;

        }

        // don't abort if the callerApp has any visible activity

        if (callerApp != null && callerApp.hasForegroundActivities()) {

            return false;

        }

        // don't abort if the callerApp is instrumenting with background activity starts privileges

        if (callerApp != null && callerApp.isInstrumentingWithBackgroundActivityStartPrivileges()) {

            return false;

        }

        // don't abort if the callingUid is in the foreground or is a persistent system process

        final int callingUidProcState = mService.getUidStateLocked(callingUid);

        final boolean callingUidHasAnyVisibleWindow =
@@ -967,10 +959,27 @@ class ActivityStarter { 
                return false;

            }

        }

        // If we don't have callerApp at this point, no caller was provided to startActivity().

        // That's the case for PendingIntent-based starts, since the creator's process might not be

        // up and alive. If that's the case, we retrieve the WindowProcessController for the send()

        // caller, so that we can make the decision based on its foreground/whitelisted state.

        if (callerApp == null) {

            callerApp = mService.getProcessController(realCallingPid, realCallingUid);

        }

        if (callerApp != null) {

            // don't abort if the callerApp has any visible activity

            if (callerApp.hasForegroundActivities()) {

                return false;

            }

            // don't abort if the callerApp is instrumenting with background activity starts privs

            if (callerApp.isInstrumentingWithBackgroundActivityStartPrivileges()) {

                return false;

            }

            // don't abort if the caller is currently temporarily whitelisted

        if (callerApp != null && callerApp.areBackgroundActivityStartsAllowed()) {

            if (callerApp.areBackgroundActivityStartsAllowed()) {

                return false;

            }

        }

        // don't abort if the callingUid has START_ACTIVITIES_FROM_BACKGROUND permission

        if (mService.checkPermission(START_ACTIVITIES_FROM_BACKGROUND, callingPid, callingUid)

                == PERMISSION_GRANTED) {
@@ -996,6 +1005,7 @@ class ActivityStarter { 
                + "; originatingPendingIntent: " + originatingPendingIntent

                + "; isBgStartWhitelisted: " + allowBackgroundActivityStart

                + "; intent: " + intent

                + "; callerApp: " + callerApp

                + "]");

        // log aborted activity start to TRON

        if (mService.isActivityStartsLoggingEnabled()) {