Binder: don't call getCallingUid for RPC calls

All the code using getCallingUid here should be
ripped out anyway because it's Java-centric. Here
are some of the issues with the current stuff around
here:
- it's broken because it won't handle things happening in
  native code
- it's slow (e.g. big transactions are already logged in
  native code anyway)

Bug: 254835230
Test: boots, atest MicrodroidTestApp (with new tests)
Change-Id: I7315059bff138966e9509f1d529a6417466d2b7f
This commit is contained in:
Steven Moreland
2022-10-26 01:10:28 +00:00
parent b6663f9d82
commit b95831cbd9

View File

@@ -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