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

Commit 606039cf authored by Carlos Llamas's avatar Carlos Llamas
Browse files

libbinder: assert freeze notification result 



There is an unknown scenario that leads to a frozen notification state
mismatch between userspace and the kernel. When the app then attempts to
unregister the callback, the driver returns an -EINVAL as there is no
active notification setup for that handle.

However, flushCommands() simply drops the error without handling it and
the now-obsolete BC_CLEAR_FREEZE_NOTIFICATION remains at the head of the
mOut buffer. Every subsequent communication attemps fail as the driver
continues to process the same failing command.

It was reported that this scenario caused thousands of binder references
to be leaked, as subsequent commands such as BC_RELEASE and BC_DECREFS
would fail to be processed. This renders the phone unresponsive.

For now, use a big hammer and crash the app when we find errors sent
from the driver. This should help to root-cause how the state mismatch
happens when we add/remove the frozen notifications.

Bug: 416678481
Bug: 422905364
Flag: EXEMPT bugfix
Change-Id: I1a689e9aebb06d1ccffbc2faa504edce9e754065
Signed-off-by: default avatarCarlos Llamas <cmllamas@google.com>
parent c8305547

libs/binder/IPCThreadState.cpp

+16 −4
Original line number Diff line number Diff line
@@ -625,14 +625,15 @@ void IPCThreadState::clearCaller() 
    mCallingUid = getuid();

}



void IPCThreadState::flushCommands() {

status_t IPCThreadState::flushCommands() {

    if (mProcess->mDriverFD < 0)

        return;

        return -EBADF;



    if (status_t res = talkWithDriver(false); res != OK) {

        // TODO: we may want to abort for some of these cases

        ALOGW("1st call to talkWithDriver returned error in flushCommands: %s",

              statusToString(res).c_str());

        return res;

    }



    // The flush could have caused post-write refcount decrements to have
@@ -643,11 +644,14 @@ void IPCThreadState::flushCommands() { 
            // TODO: we may want to abort for some of these cases

            ALOGW("2nd call to talkWithDriver returned error in flushCommands: %s",

                  statusToString(res).c_str());

            return res;

        }

    }

    if (mOut.dataSize() > 0) {

        ALOGW("mOut.dataSize() > 0 after flushCommands()");

    }



    return NO_ERROR;

}



bool IPCThreadState::flushIfNeeded()
@@ -1020,7 +1024,11 @@ status_t IPCThreadState::addFrozenStateChangeCallback(int32_t handle, BpBinder* 
    mOut.writeInt32(BC_REQUEST_FREEZE_NOTIFICATION);

    mOut.writeInt32((int32_t)handle);

    mOut.writePointer((uintptr_t)proxy);

    flushCommands();



    if (status_t res = flushCommands(); res != OK) {

        LOG_ALWAYS_FATAL("%s(%d): %s", __func__, handle, statusToString(res).c_str());

    }



    return NO_ERROR;

}
@@ -1033,7 +1041,11 @@ status_t IPCThreadState::removeFrozenStateChangeCallback(int32_t handle, BpBinde 
    mOut.writeInt32(BC_CLEAR_FREEZE_NOTIFICATION);

    mOut.writeInt32((int32_t)handle);

    mOut.writePointer((uintptr_t)proxy);

    flushCommands();



    if (status_t res = flushCommands(); res != OK) {

        LOG_ALWAYS_FATAL("%s(%d): %s", __func__, handle, statusToString(res).c_str());

    }



    return NO_ERROR;

}

libs/binder/include/binder/IPCThreadState.h

+1 −1
Original line number Diff line number Diff line
@@ -154,7 +154,7 @@ public: 
    // For main functions - dangerous for libraries to use

    LIBBINDER_EXPORTED status_t setupPolling(int* fd);

    LIBBINDER_EXPORTED status_t handlePolledCommands();

    LIBBINDER_EXPORTED void flushCommands();

    LIBBINDER_EXPORTED status_t flushCommands();

    LIBBINDER_EXPORTED bool flushIfNeeded();



    // Adds the current thread into the binder threadpool.