Collect AsyncNotedAppOp in same call as noteOp
This reduces the overhead of collecting noted app-ops to the same amount
of binder calls as if we would not have the feature
Before (conceptionally):
---------------------------------
mode = service.noteOp()
if (mode == allowed && shouldCollectAsyncOp) {
fixup(message)
service.noteAsyncOp()
}
----------------------------------
After (conceptionally):
----------------------------------
if (shouldCollectAsyncOp) {
fixup(message)
}
mode = service.noteOp(shouldCollectAsyncOp, message)
----------------------------------
Bug: 136505050
Test: atest CtsAppOpsTestCases
Change-Id: If1b535a7c4b0f431f251c5d06cdf496c34920e23
diff --git a/services/core/java/com/android/server/am/ActiveServices.java b/services/core/java/com/android/server/am/ActiveServices.java
index 154d16f..a58bd9b 100644
--- a/services/core/java/com/android/server/am/ActiveServices.java
+++ b/services/core/java/com/android/server/am/ActiveServices.java
@@ -570,7 +570,7 @@
}
mAm.mAppOpsService.startOperation(AppOpsManager.getToken(mAm.mAppOpsService),
AppOpsManager.OP_START_FOREGROUND, r.appInfo.uid, r.packageName, null,
- true);
+ true, false, null);
}
final ServiceMap smap = getServiceMapLocked(r.userId);
@@ -1395,7 +1395,7 @@
mAm.mAppOpsService.startOperation(
AppOpsManager.getToken(mAm.mAppOpsService),
AppOpsManager.OP_START_FOREGROUND, r.appInfo.uid, r.packageName,
- null, true);
+ null, true, false, "");
StatsLog.write(StatsLog.FOREGROUND_SERVICE_STATE_CHANGED,
r.appInfo.uid, r.shortInstanceName,
StatsLog.FOREGROUND_SERVICE_STATE_CHANGED__STATE__ENTER);
diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java
index d7a46fe..dd1f370 100644
--- a/services/core/java/com/android/server/am/ActivityManagerService.java
+++ b/services/core/java/com/android/server/am/ActivityManagerService.java
@@ -324,6 +324,7 @@
import com.android.internal.util.FastPrintWriter;
import com.android.internal.util.MemInfoReader;
import com.android.internal.util.Preconditions;
+import com.android.internal.util.function.HexFunction;
import com.android.internal.util.function.QuadFunction;
import com.android.internal.util.function.TriFunction;
import com.android.server.AlarmManagerInternal;
@@ -3120,7 +3121,7 @@
private boolean hasUsageStatsPermission(String callingPackage) {
final int mode = mAppOpsService.noteOperation(AppOpsManager.OP_GET_USAGE_STATS,
- Binder.getCallingUid(), callingPackage, null);
+ Binder.getCallingUid(), callingPackage, null, false, "");
if (mode == AppOpsManager.MODE_DEFAULT) {
return checkCallingPermission(Manifest.permission.PACKAGE_USAGE_STATS)
== PackageManager.PERMISSION_GRANTED;
@@ -5816,7 +5817,8 @@
public int noteOp(String op, int uid, String packageName) {
// TODO moltmann: Allow to specify featureId
return mActivityManagerService.mAppOpsService
- .noteOperation(AppOpsManager.strOpToOp(op), uid, packageName, null);
+ .noteOperation(AppOpsManager.strOpToOp(op), uid, packageName, null,
+ false, "");
}
@Override
@@ -5968,7 +5970,7 @@
}
// ...and legacy apps get an AppOp check
int appop = mAppOpsService.noteOperation(AppOpsManager.OP_RUN_IN_BACKGROUND,
- uid, packageName, null);
+ uid, packageName, null, false, "");
if (DEBUG_BACKGROUND_CHECK) {
Slog.i(TAG, "Legacy app " + uid + "/" + packageName + " bg appop " + appop);
}
@@ -19253,18 +19255,22 @@
@Override
public int noteOperation(int code, int uid, @Nullable String packageName,
- @Nullable String featureId,
- @NonNull QuadFunction<Integer, Integer, String, String, Integer> superImpl) {
+ @Nullable String featureId, boolean shouldCollectAsyncNotedOp,
+ @Nullable String message,
+ @NonNull HexFunction<Integer, Integer, String, String, Boolean, String, Integer>
+ superImpl) {
if (uid == mTargetUid && isTargetOp(code)) {
final long identity = Binder.clearCallingIdentity();
try {
return mAppOpsService.noteProxyOperation(code, Process.SHELL_UID,
- "com.android.shell", null, uid, packageName, featureId);
+ "com.android.shell", null, uid, packageName, featureId,
+ shouldCollectAsyncNotedOp, message);
} finally {
Binder.restoreCallingIdentity(identity);
}
}
- return superImpl.apply(code, uid, packageName, featureId);
+ return superImpl.apply(code, uid, packageName, featureId, shouldCollectAsyncNotedOp,
+ message);
}
@Override
diff --git a/services/core/java/com/android/server/am/BroadcastQueue.java b/services/core/java/com/android/server/am/BroadcastQueue.java
index 10492a7..6fca3f6 100644
--- a/services/core/java/com/android/server/am/BroadcastQueue.java
+++ b/services/core/java/com/android/server/am/BroadcastQueue.java
@@ -650,7 +650,7 @@
// TODO moltmann: Set featureId from caller
if (opCode != AppOpsManager.OP_NONE
&& mService.mAppOpsService.noteOperation(opCode, r.callingUid,
- r.callerPackage, null) != AppOpsManager.MODE_ALLOWED) {
+ r.callerPackage, null, false, "") != AppOpsManager.MODE_ALLOWED) {
Slog.w(TAG, "Appop Denial: broadcasting "
+ r.intent.toString()
+ " from " + r.callerPackage + " (pid="
@@ -680,10 +680,10 @@
break;
}
int appOp = AppOpsManager.permissionToOpCode(requiredPermission);
- // TODO moltmann: Set componentId from caller
+ // TODO moltmann: Set featureId from caller
if (appOp != AppOpsManager.OP_NONE && appOp != r.appOp
&& mService.mAppOpsService.noteOperation(appOp,
- filter.receiverList.uid, filter.packageName, null)
+ filter.receiverList.uid, filter.packageName, null, false, "")
!= AppOpsManager.MODE_ALLOWED) {
Slog.w(TAG, "Appop Denial: receiving "
+ r.intent.toString()
@@ -713,10 +713,10 @@
skip = true;
}
}
- // TODO moltmann: Set componentId from caller
+ // TODO moltmann: Set featureId from caller
if (!skip && r.appOp != AppOpsManager.OP_NONE
&& mService.mAppOpsService.noteOperation(r.appOp,
- filter.receiverList.uid, filter.packageName, null)
+ filter.receiverList.uid, filter.packageName, null, false, "")
!= AppOpsManager.MODE_ALLOWED) {
Slog.w(TAG, "Appop Denial: receiving "
+ r.intent.toString()
@@ -1370,10 +1370,10 @@
skip = true;
} else if (!skip && info.activityInfo.permission != null) {
final int opCode = AppOpsManager.permissionToOpCode(info.activityInfo.permission);
- // TODO moltmann: Set componentId from caller
+ // TODO moltmann: Set featureId from caller
if (opCode != AppOpsManager.OP_NONE
- && mService.mAppOpsService.noteOperation(opCode, r.callingUid,
- r.callerPackage, null) != AppOpsManager.MODE_ALLOWED) {
+ && mService.mAppOpsService.noteOperation(opCode, r.callingUid, r.callerPackage,
+ null, false, "") != AppOpsManager.MODE_ALLOWED) {
Slog.w(TAG, "Appop Denial: broadcasting "
+ r.intent.toString()
+ " from " + r.callerPackage + " (pid="
@@ -1409,10 +1409,11 @@
break;
}
int appOp = AppOpsManager.permissionToOpCode(requiredPermission);
- // TODO moltmann: Set componentId from caller
+ // TODO moltmann: Set featureId from caller
if (appOp != AppOpsManager.OP_NONE && appOp != r.appOp
&& mService.mAppOpsService.noteOperation(appOp,
- info.activityInfo.applicationInfo.uid, info.activityInfo.packageName, null)
+ info.activityInfo.applicationInfo.uid, info.activityInfo.packageName, null,
+ false, "")
!= AppOpsManager.MODE_ALLOWED) {
Slog.w(TAG, "Appop Denial: receiving "
+ r.intent + " to "
@@ -1426,10 +1427,11 @@
}
}
}
- // TODO moltmann: Set componentId from caller
+ // TODO moltmann: Set featureId from caller
if (!skip && r.appOp != AppOpsManager.OP_NONE
&& mService.mAppOpsService.noteOperation(r.appOp,
- info.activityInfo.applicationInfo.uid, info.activityInfo.packageName, null)
+ info.activityInfo.applicationInfo.uid, info.activityInfo.packageName, null, false,
+ "")
!= AppOpsManager.MODE_ALLOWED) {
Slog.w(TAG, "Appop Denial: receiving "
+ r.intent + " to "
diff --git a/services/core/java/com/android/server/appop/AppOpsService.java b/services/core/java/com/android/server/appop/AppOpsService.java
index 1e13661..5bbb517 100644
--- a/services/core/java/com/android/server/appop/AppOpsService.java
+++ b/services/core/java/com/android/server/appop/AppOpsService.java
@@ -221,7 +221,7 @@
= new AppOpsManagerInternalImpl();
/**
- * Registered callbacks, called from {@link #noteAsyncOp}.
+ * Registered callbacks, called from {@link #collectAsyncNotedOp}.
*
* <p>(package name, uid) -> callbacks
*
@@ -232,7 +232,7 @@
mAsyncOpWatchers = new ArrayMap<>();
/**
- * Async note-ops collected from {@link #noteAsyncOp} that have not been delivered to a
+ * Async note-ops collected from {@link #collectAsyncNotedOp} that have not been delivered to a
* callback yet.
*
* <p>(package name, uid) -> list<ops>
@@ -1436,11 +1436,13 @@
return Zygote.MOUNT_EXTERNAL_NONE;
}
if (noteOperation(AppOpsManager.OP_READ_EXTERNAL_STORAGE, uid,
- packageName, null) != AppOpsManager.MODE_ALLOWED) {
+ packageName, null, true, "External storage policy")
+ != AppOpsManager.MODE_ALLOWED) {
return Zygote.MOUNT_EXTERNAL_NONE;
}
if (noteOperation(AppOpsManager.OP_WRITE_EXTERNAL_STORAGE, uid,
- packageName, null) != AppOpsManager.MODE_ALLOWED) {
+ packageName, null, true, "External storage policy")
+ != AppOpsManager.MODE_ALLOWED) {
return Zygote.MOUNT_EXTERNAL_READ;
}
return Zygote.MOUNT_EXTERNAL_WRITE;
@@ -2521,7 +2523,7 @@
@Override
public int noteProxyOperation(int code, int proxiedUid, String proxiedPackageName,
String proxiedFeatureId, int proxyUid, String proxyPackageName,
- String proxyFeatureId) {
+ String proxyFeatureId, boolean shouldCollectAsyncNotedOp, String message) {
verifyIncomingUid(proxyUid);
verifyIncomingOp(code);
@@ -2537,7 +2539,8 @@
final int proxyFlags = isProxyTrusted ? AppOpsManager.OP_FLAG_TRUSTED_PROXY
: AppOpsManager.OP_FLAG_UNTRUSTED_PROXY;
final int proxyMode = noteOperationUnchecked(code, proxyUid, resolveProxyPackageName,
- proxyFeatureId, Process.INVALID_UID, null, null, proxyFlags);
+ proxyFeatureId, Process.INVALID_UID, null, null, proxyFlags,
+ !isProxyTrusted, "proxy " + message);
if (proxyMode != AppOpsManager.MODE_ALLOWED || Binder.getCallingUid() == proxiedUid) {
return proxyMode;
}
@@ -2550,23 +2553,27 @@
: AppOpsManager.OP_FLAG_UNTRUSTED_PROXIED;
return noteOperationUnchecked(code, proxiedUid, resolveProxiedPackageName,
proxiedFeatureId, proxyUid, resolveProxyPackageName, proxyFeatureId,
- proxiedFlags);
+ proxiedFlags, shouldCollectAsyncNotedOp, message);
}
@Override
- public int noteOperation(int code, int uid, String packageName, String featureId) {
+ public int noteOperation(int code, int uid, String packageName, String featureId,
+ boolean shouldCollectAsyncNotedOp, String message) {
final CheckOpsDelegate delegate;
synchronized (this) {
delegate = mCheckOpsDelegate;
}
if (delegate == null) {
- return noteOperationImpl(code, uid, packageName, featureId);
+ return noteOperationImpl(code, uid, packageName, featureId, shouldCollectAsyncNotedOp,
+ message);
}
- return delegate.noteOperation(code, uid, packageName, featureId,
- AppOpsService.this::noteOperationImpl);
+ return delegate.noteOperation(code, uid, packageName, featureId, shouldCollectAsyncNotedOp,
+ message, AppOpsService.this::noteOperationImpl);
}
- private int noteOperationImpl(int code, int uid, String packageName, String featureId) {
+ private int noteOperationImpl(int code, int uid, @Nullable String packageName,
+ @Nullable String featureId, boolean shouldCollectAsyncNotedOp,
+ @Nullable String message) {
verifyIncomingUid(uid);
verifyIncomingOp(code);
String resolvedPackageName = resolvePackageName(uid, packageName);
@@ -2574,12 +2581,14 @@
return AppOpsManager.MODE_IGNORED;
}
return noteOperationUnchecked(code, uid, resolvedPackageName, featureId,
- Process.INVALID_UID, null, null, AppOpsManager.OP_FLAG_SELF);
+ Process.INVALID_UID, null, null, AppOpsManager.OP_FLAG_SELF,
+ shouldCollectAsyncNotedOp, message);
}
- private int noteOperationUnchecked(int code, int uid, String packageName, String featureId,
- int proxyUid, String proxyPackageName, @Nullable String proxyFeatureId,
- @OpFlags int flags) {
+ private int noteOperationUnchecked(int code, int uid, @NonNull String packageName,
+ @Nullable String featureId, int proxyUid, String proxyPackageName,
+ @Nullable String proxyFeatureId, @OpFlags int flags, boolean shouldCollectAsyncNotedOp,
+ @Nullable String message) {
boolean isPrivileged;
try {
isPrivileged = verifyAndGetIsPrivileged(uid, packageName, featureId);
@@ -2650,6 +2659,11 @@
uidState.state, flags);
scheduleOpNotedIfNeededLocked(code, uid, packageName,
AppOpsManager.MODE_ALLOWED);
+
+ if (shouldCollectAsyncNotedOp) {
+ collectAsyncNotedOp(uid, packageName, code, featureId, message);
+ }
+
return AppOpsManager.MODE_ALLOWED;
}
}
@@ -2746,21 +2760,20 @@
}
}
- @Override
- public void noteAsyncOp(String callingPackageName, int uid, String packageName, int opCode,
- String featureId, String message) {
+ /**
+ * Collect an {@link AsyncNotedAppOp}.
+ *
+ * @param uid The uid the op was noted for
+ * @param packageName The package the op was noted for
+ * @param opCode The code of the op noted
+ * @param featureId The id of the feature to op was noted for
+ * @param message The message for the op noting
+ */
+ private void collectAsyncNotedOp(int uid, @NonNull String packageName, int opCode,
+ @Nullable String featureId, @NonNull String message) {
Objects.requireNonNull(message);
- verifyAndGetIsPrivileged(uid, packageName, featureId);
-
- verifyIncomingUid(uid);
- verifyIncomingOp(opCode);
int callingUid = Binder.getCallingUid();
- long now = System.currentTimeMillis();
-
- if (callingPackageName != null) {
- verifyAndGetIsPrivileged(callingUid, callingPackageName, featureId);
- }
long token = Binder.clearCallingIdentity();
try {
@@ -2769,7 +2782,7 @@
RemoteCallbackList<IAppOpsAsyncNotedCallback> callbacks = mAsyncOpWatchers.get(key);
AsyncNotedAppOp asyncNotedOp = new AsyncNotedAppOp(opCode, callingUid,
- callingPackageName, featureId, message, now);
+ featureId, message, System.currentTimeMillis());
final boolean[] wasNoteForwarded = {false};
if (callbacks != null) {
@@ -2882,7 +2895,8 @@
@Override
public int startOperation(IBinder clientId, int code, int uid, String packageName,
- String featureId, boolean startIfModeDefault) {
+ String featureId, boolean startIfModeDefault, boolean shouldCollectAsyncNotedOp,
+ String message) {
verifyIncomingUid(uid);
verifyIncomingOp(code);
String resolvedPackageName = resolvePackageName(uid, packageName);
@@ -2953,6 +2967,10 @@
}
}
+ if (shouldCollectAsyncNotedOp) {
+ collectAsyncNotedOp(uid, packageName, code, featureId, message);
+ }
+
return AppOpsManager.MODE_ALLOWED;
}
@@ -4379,7 +4397,8 @@
if (shell.packageName != null) {
shell.mInterface.startOperation(shell.mToken, shell.op, shell.packageUid,
- shell.packageName, shell.featureId, true);
+ shell.packageName, shell.featureId, true, true,
+ "appops start shell command");
} else {
return -1;
}
diff --git a/services/core/java/com/android/server/wm/ActivityStackSupervisor.java b/services/core/java/com/android/server/wm/ActivityStackSupervisor.java
index a380efc4..c9ef851 100644
--- a/services/core/java/com/android/server/wm/ActivityStackSupervisor.java
+++ b/services/core/java/com/android/server/wm/ActivityStackSupervisor.java
@@ -1211,8 +1211,8 @@
}
// TODO moltmann b/136595429: Set featureId from caller
- if (mService.getAppOpsService().noteOperation(opCode, callingUid, callingPackage, /* featureId */ null)
- != AppOpsManager.MODE_ALLOWED) {
+ if (mService.getAppOpsService().noteOperation(opCode, callingUid,
+ callingPackage, /* featureId */ null, false, "") != AppOpsManager.MODE_ALLOWED) {
if (!ignoreTargetSecurity) {
return ACTIVITY_RESTRICTION_APPOP;
}
@@ -1254,9 +1254,9 @@
return ACTIVITY_RESTRICTION_NONE;
}
- // TODO moltmann b/136595429: Set componentId from caller
- if (mService.getAppOpsService().noteOperation(opCode, callingUid, callingPackage, /* featureId */ null)
- != AppOpsManager.MODE_ALLOWED) {
+ // TODO moltmann b/136595429: Set featureId from caller
+ if (mService.getAppOpsService().noteOperation(opCode, callingUid,
+ callingPackage, /* featureId */ null, false, "") != AppOpsManager.MODE_ALLOWED) {
return ACTIVITY_RESTRICTION_APPOP;
}
diff --git a/services/core/java/com/android/server/wm/ActivityTaskManagerService.java b/services/core/java/com/android/server/wm/ActivityTaskManagerService.java
index 6c3b580..fb3343e 100644
--- a/services/core/java/com/android/server/wm/ActivityTaskManagerService.java
+++ b/services/core/java/com/android/server/wm/ActivityTaskManagerService.java
@@ -902,7 +902,7 @@
boolean hasSystemAlertWindowPermission(int callingUid, int callingPid, String callingPackage) {
final int mode = getAppOpsService().noteOperation(AppOpsManager.OP_SYSTEM_ALERT_WINDOW,
- callingUid, callingPackage, /* featureId */ null);
+ callingUid, callingPackage, /* featureId */ null, false, "");
if (mode == AppOpsManager.MODE_DEFAULT) {
return checkPermission(Manifest.permission.SYSTEM_ALERT_WINDOW, callingPid, callingUid)
== PERMISSION_GRANTED;