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

Commit bab444a7 authored by Olivier Gaillard's avatar Olivier Gaillard
Browse files

Fix a bug with worksource propagation. 

It is properly added to the parcel when Binder.setCallingWorkSource is
called manually, however it does not work when we call
Binder.setCallingWorkSource in Binder#ProxyTransactListener. The problem
is that we are adding the worksource to the parcel too early. It is
called before we add the work source to the thread local
(ThreadLocalWorkSource.setUid)...

What currently happens
- Client code calls an AIDL method
- AIDL generated code calls writeInterfaceToken which add the headers to the parcel (including the worksource)
- AIDL generated code calls Binder#transact
- Binder#transact calls ProxyTransactListener#onTransactStarted --> this code is calling Binder.setCallingWorkSource too late. After writeInterfaceToken is called which is where the code calls Binder.getCallingWorkSource and add it to the parcel.

To fix it, we udpate the parcel request headers if the work source has
been fixed when the listener is called.

Test: atest binderLibTest BinderWorkSourceTest BinderCallsStatsServiceTest android.os.ParcelTest BinderProxyTest
Bug: 123744028
Change-Id: Id1a4565c1f096d38bf1e423bea897da77ff84005
parent 79eed4bf

core/java/android/os/Binder.java

+13 −7
Original line number Diff line number Diff line
@@ -967,7 +967,7 @@ public class Binder implements IBinder { 
     * By default, we use the calling uid since we can always trust it.

     */

    private static volatile BinderInternal.WorkSourceProvider sWorkSourceProvider =

            Binder::getCallingUid;

            (x) -> Binder.getCallingUid();



    /**

     * Sets the work source provider.
@@ -991,21 +991,23 @@ public class Binder implements IBinder { 
    // Entry point from android_util_Binder.cpp's onTransact

    private boolean execTransact(int code, long dataObj, long replyObj,

            int flags) {

        final int workSourceUid = sWorkSourceProvider.resolveWorkSourceUid();

        final long origWorkSource = ThreadLocalWorkSource.setUid(workSourceUid);

        // At that point, the parcel request headers haven't been parsed so we do not know what

        // WorkSource the caller has set. Use calling uid as the default.

        final int callingUid = Binder.getCallingUid();

        final long origWorkSource = ThreadLocalWorkSource.setUid(callingUid);

        try {

            return execTransactInternal(code, dataObj, replyObj, flags, workSourceUid);

            return execTransactInternal(code, dataObj, replyObj, flags, callingUid);

        } finally {

            ThreadLocalWorkSource.restore(origWorkSource);

        }

    }



    private boolean execTransactInternal(int code, long dataObj, long replyObj,

            int flags, int workSourceUid) {

    private boolean execTransactInternal(int code, long dataObj, long replyObj, int flags,

            int callingUid) {

        // Make sure the observer won't change while processing a transaction.

        final BinderInternal.Observer observer = sObserver;

        final CallSession callSession =

                observer != null ? observer.callStarted(this, code, workSourceUid) : null;

                observer != null ? observer.callStarted(this, code, UNSET_WORKSOURCE) : null;

        Parcel data = Parcel.obtain(dataObj);

        Parcel reply = Parcel.obtain(replyObj);

        // theoretically, we should call transact, which will call onTransact,
@@ -1045,6 +1047,10 @@ public class Binder implements IBinder { 
                Trace.traceEnd(Trace.TRACE_TAG_ALWAYS);

            }

            if (observer != null) {

                // The parcel RPC headers have been called during onTransact so we can now access

                // the worksource uid from the parcel.

                final int workSourceUid = sWorkSourceProvider.resolveWorkSourceUid(

                        data.readCallingWorkSourceUid());

                observer.callEnded(callSession, data.dataSize(), reply.dataSize(), workSourceUid);

            }

        }

core/java/android/os/BinderProxy.java

+9 −0
Original line number Diff line number Diff line
@@ -493,8 +493,17 @@ public final class BinderProxy implements IBinder { 
        // Make sure the listener won't change while processing a transaction.

        final Binder.ProxyTransactListener transactListener = sTransactListener;

        Object session = null;



        if (transactListener != null) {

            final int origWorkSourceUid = Binder.getCallingWorkSourceUid();

            session = transactListener.onTransactStarted(this, code);



            // Allow the listener to update the work source uid. We need to update the request

            // header if the uid is updated.

            final int updatedWorkSourceUid = Binder.getCallingWorkSourceUid();

            if (origWorkSourceUid != updatedWorkSourceUid) {

                data.replaceCallingWorkSourceUid(updatedWorkSourceUid);

            }

        }



        try {

core/java/android/os/Parcel.java

+35 −0
Original line number Diff line number Diff line
@@ -333,6 +333,12 @@ public final class Parcel { 
    private static native void nativeWriteInterfaceToken(long nativePtr, String interfaceName);

    private static native void nativeEnforceInterface(long nativePtr, String interfaceName);



    @CriticalNative

    private static native boolean nativeReplaceCallingWorkSourceUid(

            long nativePtr, int workSourceUid);

    @CriticalNative

    private static native int nativeReadCallingWorkSourceUid(long nativePtr);



    /** Last time exception with a stack trace was written */

    private static volatile long sLastWriteExceptionStackTrace;

    /** Used for throttling of writing stack trace, which is costly */
@@ -612,6 +618,35 @@ public final class Parcel { 
        nativeEnforceInterface(mNativePtr, interfaceName);

    }



    /**

     * Writes the work source uid to the request headers.

     *

     * <p>It requires the headers to have been written/read already to replace the work source.

     *

     * @return true if the request headers have been updated.

     *

     * @hide

     */

    public boolean replaceCallingWorkSourceUid(int workSourceUid) {

        return nativeReplaceCallingWorkSourceUid(mNativePtr, workSourceUid);

    }



    /**

     * Reads the work source uid from the request headers.

     *

     * <p>Unlike other read methods, this method does not read the parcel at the current

     * {@link #dataPosition}. It will set the {@link #dataPosition} before the read and restore the

     * position after reading the request header.

     *

     * @return the work source uid or {@link Binder#UNSET_WORKSOURCE} if headers have not been

     * written/parsed yet.

     *

     * @hide

     */

    public int readCallingWorkSourceUid() {

        return nativeReadCallingWorkSourceUid(mNativePtr);

    }



    /**

     * Write a byte array into the parcel at the current {@link #dataPosition},

     * growing {@link #dataCapacity} if needed.

core/java/com/android/internal/os/BinderInternal.java

+2 −1
Original line number Diff line number Diff line
@@ -96,9 +96,10 @@ public class BinderInternal { 
         * <p>The implementation should never execute a binder call since it is called during a

         * binder transaction.

         *

         * @param untrustedWorkSourceUid The work source set by the caller.

         * @return the uid of the process to attribute the binder transaction to.

         */

        int resolveWorkSourceUid();

        int resolveWorkSourceUid(int untrustedWorkSourceUid);

    }



    /**

core/jni/android_os_Parcel.cpp

+23 −0
Original line number Diff line number Diff line
@@ -670,6 +670,24 @@ static jlong android_os_Parcel_getBlobAshmemSize(jlong nativePtr) 
    return 0;

}



static jint android_os_Parcel_readCallingWorkSourceUid(jlong nativePtr)

{

    Parcel* parcel = reinterpret_cast<Parcel*>(nativePtr);

    if (parcel != NULL) {

        return parcel->readCallingWorkSourceUid();

    }

    return IPCThreadState::kUnsetWorkSource;

}



static jboolean android_os_Parcel_replaceCallingWorkSourceUid(jlong nativePtr, jint uid)

{

    Parcel* parcel = reinterpret_cast<Parcel*>(nativePtr);

    if (parcel != NULL) {

        return parcel->replaceCallingWorkSourceUid(uid);

    }

    return false;

}



// ----------------------------------------------------------------------------



static const JNINativeMethod gParcelMethods[] = {
@@ -740,6 +758,11 @@ static const JNINativeMethod gParcelMethods[] = { 


    // @CriticalNative

    {"nativeGetBlobAshmemSize",       "(J)J", (void*)android_os_Parcel_getBlobAshmemSize},



    // @CriticalNative

    {"nativeReadCallingWorkSourceUid", "(J)I", (void*)android_os_Parcel_readCallingWorkSourceUid},

    // @CriticalNative

    {"nativeReplaceCallingWorkSourceUid", "(JI)Z", (void*)android_os_Parcel_replaceCallingWorkSourceUid},

};



const char* const kParcelPathName = "android/os/Parcel";