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

Commit 806a71c6 authored by Li Li's avatar Li Li
Browse files

Freezer: fix binder race

As there isn't an atomic operation to freeze the main thread and binder
threads altogether, it's possible the main thread initiates a new binder
transaction while the binder threads are already frozen. This race issue
will result in failed binder transaction and unexpectedly kill the app.

Fix it by rescheduling the ongoing freezing opeartion if there's already
an outstanding binder trasaction or new pending binder transactions. At
the same time, treat the REPLY transactions of those pending ones the
same way as an oneway transaction so that it can successfully reach the
frozen process, preventing it from being killed.

Bug: 198493121
Test: app launch/foreground/background stress test
Change-Id: I1009fa33edbd2b1db14cf51b598f5869d41ee6b6
Merged-In: I1009fa33edbd2b1db14cf51b598f5869d41ee6b6
parent 6322d1e8
Loading
Loading
Loading
Loading
+49 −17
Original line number Diff line number Diff line
@@ -157,6 +157,9 @@ public final class CachedAppOptimizer {
    static final int SYNC_RECEIVED_WHILE_FROZEN = 1;
    static final int ASYNC_RECEIVED_WHILE_FROZEN = 2;

    // Bitfield values for sync transactions received by frozen binder threads
    static final int TXNS_PENDING_WHILE_FROZEN = 4;

    /**
     * This thread must be moved to the system background cpuset.
     * If that doesn't happen, it's probably going to draw a lot of power.
@@ -611,8 +614,9 @@ public final class CachedAppOptimizer {
     * binder for the specificed pid.
     *
     * @throws RuntimeException in case a flush/freeze operation could not complete successfully.
     * @return 0 if success, or -EAGAIN indicating there's pending transaction.
     */
    private static native void freezeBinder(int pid, boolean freeze);
    private static native int freezeBinder(int pid, boolean freeze);

    /**
     * Retrieves binder freeze info about a process.
@@ -948,7 +952,7 @@ public final class CachedAppOptimizer {
            int freezeInfo = getBinderFreezeInfo(pid);

            if ((freezeInfo & SYNC_RECEIVED_WHILE_FROZEN) != 0) {
                Slog.d(TAG_AM, "pid " + pid + " " + app.processName + " "
                Slog.d(TAG_AM, "pid " + pid + " " + app.processName
                        + " received sync transactions while frozen, killing");
                app.killLocked("Sync transaction while in frozen state",
                        ApplicationExitInfo.REASON_OTHER,
@@ -956,8 +960,8 @@ public final class CachedAppOptimizer {
                processKilled = true;
            }

            if ((freezeInfo & ASYNC_RECEIVED_WHILE_FROZEN) != 0) {
                Slog.d(TAG_AM, "pid " + pid + " " + app.processName + " "
            if ((freezeInfo & ASYNC_RECEIVED_WHILE_FROZEN) != 0 && DEBUG_FREEZER) {
                Slog.d(TAG_AM, "pid " + pid + " " + app.processName
                        + " received async transactions while frozen");
            }
        } catch (Exception e) {
@@ -1292,7 +1296,9 @@ public final class CachedAppOptimizer {
        public void handleMessage(Message msg) {
            switch (msg.what) {
                case SET_FROZEN_PROCESS_MSG:
                    synchronized (mAm) {
                        freezeProcess((ProcessRecord) msg.obj);
                    }
                    break;
                case REPORT_UNFREEZE_MSG:
                    int pid = msg.arg1;
@@ -1306,6 +1312,15 @@ public final class CachedAppOptimizer {
            }
        }

        @GuardedBy({"mAm", "mProcLock"})
        private void rescheduleFreeze(final ProcessRecord proc, final String reason) {
            Slog.d(TAG_AM, "Reschedule freeze for process " + proc.getPid()
                    + " " + proc.processName + " (" + reason + ")");
            unfreezeAppLSP(proc);
            freezeAppAsyncLSP(proc);
        }

        @GuardedBy({"mAm"})
        private void freezeProcess(final ProcessRecord proc) {
            int pid = proc.getPid(); // Unlocked intentionally
            final String name = proc.processName;
@@ -1355,10 +1370,15 @@ public final class CachedAppOptimizer {
                    return;
                }

                Slog.d(TAG_AM, "freezing " + pid + " " + name);

                // Freeze binder interface before the process, to flush any
                // transactions that might be pending.
                try {
                    freezeBinder(pid, true);
                    if (freezeBinder(pid, true) != 0) {
                        rescheduleFreeze(proc, "outstanding txns");
                        return;
                    }
                } catch (RuntimeException e) {
                    Slog.e(TAG_AM, "Unable to freeze binder for " + pid + " " + name);
                    mFreezeHandler.post(() -> {
@@ -1402,28 +1422,40 @@ public final class CachedAppOptimizer {
                        unfrozenDuration);
            }

            try {
                // post-check to prevent races
                int freezeInfo = getBinderFreezeInfo(pid);

                if ((freezeInfo & TXNS_PENDING_WHILE_FROZEN) != 0) {
                    synchronized (mProcLock) {
                        rescheduleFreeze(proc, "new pending txns");
                    }
                    return;
                }
            } catch (RuntimeException e) {
                Slog.e(TAG_AM, "Unable to freeze binder for " + pid + " " + name);
                mFreezeHandler.post(() -> {
                    synchronized (mAm) {
                        proc.killLocked("Unable to freeze binder interface",
                                ApplicationExitInfo.REASON_OTHER,
                                ApplicationExitInfo.SUBREASON_FREEZER_BINDER_IOCTL, true);
                    }
                });
            }

            try {
                // post-check to prevent races
                if (mProcLocksReader.hasFileLocks(pid)) {
                    if (DEBUG_FREEZER) {
                        Slog.d(TAG_AM, name + " (" + pid + ") holds file locks, reverting freeze");
                    }

                    synchronized (mAm) {
                        synchronized (mProcLock) {
                    unfreezeAppLSP(proc);
                }
                    }
                }
            } catch (Exception e) {
                Slog.e(TAG_AM, "Unable to check file locks for " + name + "(" + pid + "): " + e);
                synchronized (mAm) {
                    synchronized (mProcLock) {
                unfreezeAppLSP(proc);
            }
        }
            }
        }

        private void reportUnfreeze(int pid, int frozenDuration, String processName) {

+14 −11
Original line number Diff line number Diff line
@@ -55,6 +55,7 @@ using android::base::unique_fd;

#define SYNC_RECEIVED_WHILE_FROZEN (1)
#define ASYNC_RECEIVED_WHILE_FROZEN (2)
#define TXNS_PENDING_WHILE_FROZEN (4)

namespace android {

@@ -232,17 +233,20 @@ static void com_android_server_am_CachedAppOptimizer_compactProcess(JNIEnv*, job
    compactProcessOrFallback(pid, compactionFlags);
}

static void com_android_server_am_CachedAppOptimizer_freezeBinder(
static jint com_android_server_am_CachedAppOptimizer_freezeBinder(
        JNIEnv *env, jobject clazz, jint pid, jboolean freeze) {

    if (IPCThreadState::freeze(pid, freeze, 100 /* timeout [ms] */) != 0) {
    jint retVal = IPCThreadState::freeze(pid, freeze, 100 /* timeout [ms] */);
    if (retVal != 0 && retVal != -EAGAIN) {
        jniThrowException(env, "java/lang/RuntimeException", "Unable to freeze/unfreeze binder");
    }

    return retVal;
}

static jint com_android_server_am_CachedAppOptimizer_getBinderFreezeInfo(JNIEnv *env,
        jobject clazz, jint pid) {
    bool syncReceived = false, asyncReceived = false;
    uint32_t syncReceived = 0, asyncReceived = 0;

    int error = IPCThreadState::getProcessFreezeInfo(pid, &syncReceived, &asyncReceived);

@@ -252,13 +256,12 @@ static jint com_android_server_am_CachedAppOptimizer_getBinderFreezeInfo(JNIEnv

    jint retVal = 0;

    if(syncReceived) {
        retVal |= SYNC_RECEIVED_WHILE_FROZEN;;
    }

    if(asyncReceived) {
        retVal |= ASYNC_RECEIVED_WHILE_FROZEN;
    }
    // bit 0 of sync_recv goes to bit 0 of retVal
    retVal |= syncReceived & SYNC_RECEIVED_WHILE_FROZEN;
    // bit 0 of async_recv goes to bit 1 of retVal
    retVal |= (asyncReceived << 1) & ASYNC_RECEIVED_WHILE_FROZEN;
    // bit 1 of sync_recv goes to bit 2 of retVal
    retVal |= (syncReceived << 1) & TXNS_PENDING_WHILE_FROZEN;

    return retVal;
}
@@ -278,7 +281,7 @@ static const JNINativeMethod sMethods[] = {
        /* name, signature, funcPtr */
        {"compactSystem", "()V", (void*)com_android_server_am_CachedAppOptimizer_compactSystem},
        {"compactProcess", "(II)V", (void*)com_android_server_am_CachedAppOptimizer_compactProcess},
        {"freezeBinder", "(IZ)V", (void*)com_android_server_am_CachedAppOptimizer_freezeBinder},
        {"freezeBinder", "(IZ)I", (void*)com_android_server_am_CachedAppOptimizer_freezeBinder},
        {"getBinderFreezeInfo", "(I)I",
         (void*)com_android_server_am_CachedAppOptimizer_getBinderFreezeInfo},
        {"getFreezerCheckPath", "()Ljava/lang/String;",