diff --git a/core/java/android/os/Binder.java b/core/java/android/os/Binder.java index 26435586f2d77..3c4abab346a47 100644 --- a/core/java/android/os/Binder.java +++ b/core/java/android/os/Binder.java @@ -1219,25 +1219,40 @@ public class Binder implements IBinder { @UnsupportedAppUsage private boolean execTransact(int code, long dataObj, long replyObj, int flags) { + + Parcel data = Parcel.obtain(dataObj); + Parcel reply = Parcel.obtain(replyObj); + // At that point, the parcel request headers haven't been parsed so we do not know what // {@link WorkSource} the caller has set. Use calling UID as the default. - final int callingUid = Binder.getCallingUid(); - final long origWorkSource = ThreadLocalWorkSource.setUid(callingUid); + // + // TODO: this is wrong - we should attribute along the entire call route + // also this attribution logic should move to native code - it only works + // for Java now + // + // This attribution support is not generic and therefore not support in RPC mode + final int callingUid = data.isForRpc() ? -1 : Binder.getCallingUid(); + final long origWorkSource = callingUid == -1 + ? -1 : ThreadLocalWorkSource.setUid(callingUid); + try { - return execTransactInternal(code, dataObj, replyObj, flags, callingUid); + return execTransactInternal(code, data, reply, flags, callingUid); } finally { - ThreadLocalWorkSource.restore(origWorkSource); + reply.recycle(); + data.recycle(); + + if (callingUid != -1) { + ThreadLocalWorkSource.restore(origWorkSource); + } } } - private boolean execTransactInternal(int code, long dataObj, long replyObj, int flags, + private boolean execTransactInternal(int code, Parcel data, Parcel reply, 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, UNSET_WORKSOURCE) : null; - Parcel data = Parcel.obtain(dataObj); - Parcel reply = Parcel.obtain(replyObj); // Theoretically, we should call transact, which will call onTransact, // but all that does is rewind it, and we just got these from an IPC, // so we'll just call it directly. @@ -1268,8 +1283,10 @@ public class Binder implements IBinder { final boolean tracingEnabled = tagEnabled && transactionTraceName != null; try { + // TODO - this logic should not be in Java - it should be in native + // code in libbinder so that it works for all binder users. final BinderCallHeavyHitterWatcher heavyHitterWatcher = sHeavyHitterWatcher; - if (heavyHitterWatcher != null) { + if (heavyHitterWatcher != null && callingUid != -1) { // Notify the heavy hitter watcher, if it's enabled. heavyHitterWatcher.onTransaction(callingUid, getClass(), code); } @@ -1277,7 +1294,10 @@ public class Binder implements IBinder { Trace.traceBegin(Trace.TRACE_TAG_AIDL, transactionTraceName); } - if ((flags & FLAG_COLLECT_NOTED_APP_OPS) != 0) { + // TODO - this logic should not be in Java - it should be in native + // code in libbinder so that it works for all binder users. Further, + // this should not re-use flags. + if ((flags & FLAG_COLLECT_NOTED_APP_OPS) != 0 && callingUid != -1) { AppOpsManager.startNotedAppOpsCollection(callingUid); try { res = onTransact(code, data, reply, flags); @@ -1320,8 +1340,6 @@ public class Binder implements IBinder { } checkParcel(this, code, reply, "Unreasonably large binder reply buffer"); - reply.recycle(); - data.recycle(); } // Just in case -- we are done with the IPC, so there should be no more strict diff --git a/core/java/android/os/Parcel.java b/core/java/android/os/Parcel.java index d451765f022f4..2afa87980c11c 100644 --- a/core/java/android/os/Parcel.java +++ b/core/java/android/os/Parcel.java @@ -367,6 +367,8 @@ public final class Parcel { @FastNative private static native void nativeMarkForBinder(long nativePtr, IBinder binder); @CriticalNative + private static native boolean nativeIsForRpc(long nativePtr); + @CriticalNative private static native int nativeDataSize(long nativePtr); @CriticalNative private static native int nativeDataAvail(long nativePtr); @@ -644,6 +646,15 @@ public final class Parcel { nativeMarkForBinder(mNativePtr, binder); } + /** + * Whether this Parcel is written for an RPC transaction. + * + * @hide + */ + public final boolean isForRpc() { + return nativeIsForRpc(mNativePtr); + } + /** @hide */ @ParcelFlags @TestApi diff --git a/core/jni/android_os_Parcel.cpp b/core/jni/android_os_Parcel.cpp index 1f64df49cb569..4d8dac1daaf0e 100644 --- a/core/jni/android_os_Parcel.cpp +++ b/core/jni/android_os_Parcel.cpp @@ -116,6 +116,11 @@ static void android_os_Parcel_markForBinder(JNIEnv* env, jclass clazz, jlong nat } } +static jboolean android_os_Parcel_isForRpc(jlong nativePtr) { + Parcel* parcel = reinterpret_cast(nativePtr); + return parcel ? parcel->isForRpc() : false; +} + static jint android_os_Parcel_dataSize(jlong nativePtr) { Parcel* parcel = reinterpret_cast(nativePtr); @@ -808,6 +813,8 @@ static const JNINativeMethod gParcelMethods[] = { // @FastNative {"nativeMarkForBinder", "(JLandroid/os/IBinder;)V", (void*)android_os_Parcel_markForBinder}, // @CriticalNative + {"nativeIsForRpc", "(J)Z", (void*)android_os_Parcel_isForRpc}, + // @CriticalNative {"nativeDataSize", "(J)I", (void*)android_os_Parcel_dataSize}, // @CriticalNative {"nativeDataAvail", "(J)I", (void*)android_os_Parcel_dataAvail}, diff --git a/core/tests/coretests/src/android/os/ParcelTest.java b/core/tests/coretests/src/android/os/ParcelTest.java index fdd278b9c6217..e2fe87b4cfe32 100644 --- a/core/tests/coretests/src/android/os/ParcelTest.java +++ b/core/tests/coretests/src/android/os/ParcelTest.java @@ -36,6 +36,13 @@ public class ParcelTest { private static final String INTERFACE_TOKEN_1 = "IBinder interface token"; private static final String INTERFACE_TOKEN_2 = "Another IBinder interface token"; + @Test + public void testIsForRpc() { + Parcel p = Parcel.obtain(); + assertEquals(false, p.isForRpc()); + p.recycle(); + } + @Test public void testCallingWorkSourceUidAfterWrite() { Parcel p = Parcel.obtain();