Skip to content

Commit 3b3110b

Browse files
Evan SeversonAndroid Build Coastguard Worker
authored andcommitted
Watch uid proc state instead of importance for 1-time permissions
The system process may bind to an app with the flag BIND_FOREGROUND_SERVICE, this will put the client in the foreground service importance level without the normal requirement that foreground services must show a notification. Looking at proc states instead allows us to differentiate between these two levels of foreground service and revoke the client when not in use. This change makes the parameters `importanceToResetTimer` and `importanceToKeepSessionAlive` in PermissionManager#startOneTimePermissionSession obsolete. Test: atest CtsPermissionTestCases + manual testing with mic/cam/loc Bug: 217981062 (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:0be78fbbf7d92bf29858aa0c48b171045ab5057f) Merged-In: I7a725647c001062d1a76a82b680a02e3e2edcb03 Change-Id: I7a725647c001062d1a76a82b680a02e3e2edcb03
1 parent 3f4d138 commit 3b3110b

4 files changed

Lines changed: 116 additions & 127 deletions

File tree

core/java/android/permission/IPermissionManager.aidl

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,8 +77,7 @@ interface IPermissionManager {
7777
List<SplitPermissionInfoParcelable> getSplitPermissions();
7878

7979
void startOneTimePermissionSession(String packageName, int userId, long timeout,
80-
long revokeAfterKilledDelay, int importanceToResetTimer,
81-
int importanceToKeepSessionAlive);
80+
long revokeAfterKilledDelay);
8281

8382
void stopOneTimePermissionSession(String packageName, int userId);
8483

core/java/android/permission/PermissionManager.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1371,8 +1371,7 @@ public void startOneTimePermissionSession(@NonNull String packageName,
13711371
@ActivityManager.RunningAppProcessInfo.Importance int importanceToKeepSessionAlive) {
13721372
try {
13731373
mPermissionManager.startOneTimePermissionSession(packageName, mContext.getUserId(),
1374-
timeoutMillis, revokeAfterKilledDelayMillis, importanceToResetTimer,
1375-
importanceToKeepSessionAlive);
1374+
timeoutMillis, revokeAfterKilledDelayMillis);
13761375
} catch (RemoteException e) {
13771376
e.rethrowFromSystemServer();
13781377
}

services/core/java/com/android/server/pm/permission/OneTimePermissionUserManager.java

Lines changed: 112 additions & 119 deletions
Original file line numberDiff line numberDiff line change
@@ -16,17 +16,18 @@
1616

1717
package com.android.server.pm.permission;
1818

19-
import static android.app.ActivityManager.RunningAppProcessInfo.IMPORTANCE_CACHED;
20-
2119
import android.annotation.NonNull;
2220
import android.app.ActivityManager;
2321
import android.app.AlarmManager;
22+
import android.app.IActivityManager;
23+
import android.app.IUidObserver;
2424
import android.content.BroadcastReceiver;
2525
import android.content.Context;
2626
import android.content.Intent;
2727
import android.content.IntentFilter;
2828
import android.content.pm.PackageManager;
2929
import android.os.Handler;
30+
import android.os.RemoteException;
3031
import android.permission.PermissionControllerManager;
3132
import android.provider.DeviceConfig;
3233
import android.util.Log;
@@ -47,7 +48,7 @@ public class OneTimePermissionUserManager {
4748
"one_time_permissions_killed_delay_millis";
4849

4950
private final @NonNull Context mContext;
50-
private final @NonNull ActivityManager mActivityManager;
51+
private final @NonNull IActivityManager mIActivityManager;
5152
private final @NonNull AlarmManager mAlarmManager;
5253
private final @NonNull PermissionControllerManager mPermissionControllerManager;
5354

@@ -77,49 +78,14 @@ public void onReceive(Context context, Intent intent) {
7778

7879
OneTimePermissionUserManager(@NonNull Context context) {
7980
mContext = context;
80-
mActivityManager = context.getSystemService(ActivityManager.class);
81+
mIActivityManager = ActivityManager.getService();
8182
mAlarmManager = context.getSystemService(AlarmManager.class);
8283
mPermissionControllerManager = context.getSystemService(PermissionControllerManager.class);
8384
mHandler = context.getMainThreadHandler();
8485
}
8586

86-
/**
87-
* Starts a one-time permission session for a given package. A one-time permission session is
88-
* ended if app becomes inactive. Inactivity is defined as the package's uid importance level
89-
* staying > importanceToResetTimer for timeoutMillis milliseconds. If the package's uid
90-
* importance level goes <= importanceToResetTimer then the timer is reset and doesn't start
91-
* until going > importanceToResetTimer.
92-
* <p>
93-
* When this timeoutMillis is reached if the importance level is <= importanceToKeepSessionAlive
94-
* then the session is extended until either the importance goes above
95-
* importanceToKeepSessionAlive which will end the session or <= importanceToResetTimer which
96-
* will continue the session and reset the timer.
97-
* </p>
98-
* <p>
99-
* Importance levels are defined in {@link android.app.ActivityManager.RunningAppProcessInfo}.
100-
* </p>
101-
* <p>
102-
* Once the session ends PermissionControllerService#onNotifyOneTimePermissionSessionTimeout
103-
* is invoked.
104-
* </p>
105-
* <p>
106-
* Note that if there is currently an active session for a package a new one isn't created and
107-
* the existing one isn't changed.
108-
* </p>
109-
* @param packageName The package to start a one-time permission session for
110-
* @param timeoutMillis Number of milliseconds for an app to be in an inactive state
111-
* @param revokeAfterKilledDelayMillis Number of milliseconds to wait after the process dies
112-
* before ending the session. Set to -1 to use default value
113-
* for the device.
114-
* @param importanceToResetTimer The least important level to uid must be to reset the timer
115-
* @param importanceToKeepSessionAlive The least important level the uid must be to keep the
116-
* session alive
117-
*
118-
* @hide
119-
*/
12087
void startPackageOneTimeSession(@NonNull String packageName, long timeoutMillis,
121-
long revokeAfterKilledDelayMillis, int importanceToResetTimer,
122-
int importanceToKeepSessionAlive) {
88+
long revokeAfterKilledDelayMillis) {
12389
int uid;
12490
try {
12591
uid = mContext.getPackageManager().getPackageUid(packageName, 0);
@@ -131,13 +97,11 @@ void startPackageOneTimeSession(@NonNull String packageName, long timeoutMillis,
13197
synchronized (mLock) {
13298
PackageInactivityListener listener = mListeners.get(uid);
13399
if (listener != null) {
134-
listener.updateSessionParameters(timeoutMillis, revokeAfterKilledDelayMillis,
135-
importanceToResetTimer, importanceToKeepSessionAlive);
100+
listener.updateSessionParameters(timeoutMillis, revokeAfterKilledDelayMillis);
136101
return;
137102
}
138103
listener = new PackageInactivityListener(uid, packageName, timeoutMillis,
139-
revokeAfterKilledDelayMillis, importanceToResetTimer,
140-
importanceToKeepSessionAlive);
104+
revokeAfterKilledDelayMillis);
141105
mListeners.put(uid, listener);
142106
}
143107
}
@@ -182,34 +146,58 @@ private class PackageInactivityListener implements AlarmManager.OnAlarmListener
182146

183147
private static final long TIMER_INACTIVE = -1;
184148

149+
private static final int STATE_GONE = 0;
150+
private static final int STATE_TIMER = 1;
151+
private static final int STATE_ACTIVE = 2;
152+
185153
private final int mUid;
186154
private final @NonNull String mPackageName;
187155
private long mTimeout;
188156
private long mRevokeAfterKilledDelay;
189-
private int mImportanceToResetTimer;
190-
private int mImportanceToKeepSessionAlive;
191157

192158
private boolean mIsAlarmSet;
193159
private boolean mIsFinished;
194160

195161
private long mTimerStart = TIMER_INACTIVE;
196162

197-
private final ActivityManager.OnUidImportanceListener mStartTimerListener;
198-
private final ActivityManager.OnUidImportanceListener mSessionKillableListener;
199-
private final ActivityManager.OnUidImportanceListener mGoneListener;
200-
201163
private final Object mInnerLock = new Object();
202164
private final Object mToken = new Object();
165+
private final IUidObserver.Stub mObserver = new IUidObserver.Stub() {
166+
@Override
167+
public void onUidGone(int uid, boolean disabled) {
168+
if (uid == mUid) {
169+
PackageInactivityListener.this.updateUidState(STATE_GONE);
170+
}
171+
}
203172

204-
private PackageInactivityListener(int uid, @NonNull String packageName, long timeout,
205-
long revokeAfterkilledDelay, int importanceToResetTimer,
206-
int importanceToKeepSessionAlive) {
173+
@Override
174+
public void onUidStateChanged(int uid, int procState, long procStateSeq,
175+
int capability) {
176+
if (uid == mUid) {
177+
if (procState > ActivityManager.PROCESS_STATE_FOREGROUND_SERVICE
178+
&& procState != ActivityManager.PROCESS_STATE_NONEXISTENT) {
179+
PackageInactivityListener.this.updateUidState(STATE_TIMER);
180+
} else {
181+
PackageInactivityListener.this.updateUidState(STATE_ACTIVE);
182+
}
183+
}
184+
}
185+
186+
public void onUidActive(int uid) {
187+
}
188+
public void onUidIdle(int uid, boolean disabled) {
189+
}
190+
public void onUidProcAdjChanged(int uid) {
191+
}
192+
public void onUidCachedChanged(int uid, boolean cached) {
193+
}
194+
};
207195

196+
private PackageInactivityListener(int uid, @NonNull String packageName, long timeout,
197+
long revokeAfterkilledDelay) {
208198
Log.i(LOG_TAG,
209199
"Start tracking " + packageName + ". uid=" + uid + " timeout=" + timeout
210-
+ " killedDelay=" + revokeAfterkilledDelay
211-
+ " importanceToResetTimer=" + importanceToResetTimer
212-
+ " importanceToKeepSessionAlive=" + importanceToKeepSessionAlive);
200+
+ " killedDelay=" + revokeAfterkilledDelay);
213201

214202
mUid = uid;
215203
mPackageName = packageName;
@@ -219,27 +207,24 @@ private PackageInactivityListener(int uid, @NonNull String packageName, long tim
219207
DeviceConfig.NAMESPACE_PERMISSIONS, PROPERTY_KILLED_DELAY_CONFIG_KEY,
220208
DEFAULT_KILLED_DELAY_MILLIS)
221209
: revokeAfterkilledDelay;
222-
mImportanceToResetTimer = importanceToResetTimer;
223-
mImportanceToKeepSessionAlive = importanceToKeepSessionAlive;
224-
225-
mStartTimerListener =
226-
(changingUid, importance) -> onImportanceChanged(changingUid, importance);
227-
mSessionKillableListener =
228-
(changingUid, importance) -> onImportanceChanged(changingUid, importance);
229-
mGoneListener =
230-
(changingUid, importance) -> onImportanceChanged(changingUid, importance);
231-
232-
mActivityManager.addOnUidImportanceListener(mStartTimerListener,
233-
importanceToResetTimer);
234-
mActivityManager.addOnUidImportanceListener(mSessionKillableListener,
235-
importanceToKeepSessionAlive);
236-
mActivityManager.addOnUidImportanceListener(mGoneListener, IMPORTANCE_CACHED);
237-
238-
onImportanceChanged(mUid, mActivityManager.getPackageImportance(packageName));
210+
211+
try {
212+
mIActivityManager.registerUidObserver(mObserver,
213+
ActivityManager.UID_OBSERVER_GONE | ActivityManager.UID_OBSERVER_PROCSTATE,
214+
ActivityManager.PROCESS_STATE_FOREGROUND_SERVICE,
215+
null);
216+
} catch (RemoteException e) {
217+
Log.e(LOG_TAG, "Couldn't check uid proc state", e);
218+
// Can't register uid observer, just revoke immediately
219+
synchronized (mInnerLock) {
220+
onPackageInactiveLocked();
221+
}
222+
}
223+
224+
updateUidState();
239225
}
240226

241-
public void updateSessionParameters(long timeoutMillis, long revokeAfterKilledDelayMillis,
242-
int importanceToResetTimer, int importanceToKeepSessionAlive) {
227+
public void updateSessionParameters(long timeoutMillis, long revokeAfterKilledDelayMillis) {
243228
synchronized (mInnerLock) {
244229
mTimeout = Math.min(mTimeout, timeoutMillis);
245230
mRevokeAfterKilledDelay = Math.min(mRevokeAfterKilledDelay,
@@ -248,63 +233,79 @@ public void updateSessionParameters(long timeoutMillis, long revokeAfterKilledDe
248233
DeviceConfig.NAMESPACE_PERMISSIONS,
249234
PROPERTY_KILLED_DELAY_CONFIG_KEY, DEFAULT_KILLED_DELAY_MILLIS)
250235
: revokeAfterKilledDelayMillis);
251-
mImportanceToResetTimer = Math.min(importanceToResetTimer, mImportanceToResetTimer);
252-
mImportanceToKeepSessionAlive = Math.min(importanceToKeepSessionAlive,
253-
mImportanceToKeepSessionAlive);
254236
Log.v(LOG_TAG,
255237
"Updated params for " + mPackageName + ". timeout=" + mTimeout
256-
+ " killedDelay=" + mRevokeAfterKilledDelay
257-
+ " importanceToResetTimer=" + mImportanceToResetTimer
258-
+ " importanceToKeepSessionAlive=" + mImportanceToKeepSessionAlive);
259-
onImportanceChanged(mUid, mActivityManager.getPackageImportance(mPackageName));
238+
+ " killedDelay=" + mRevokeAfterKilledDelay);
239+
updateUidState();
260240
}
261241
}
262242

263-
private void onImportanceChanged(int uid, int importance) {
264-
if (uid != mUid) {
265-
return;
243+
private int getCurrentState() {
244+
try {
245+
return getStateFromProcState(mIActivityManager.getUidProcessState(mUid, null));
246+
} catch (RemoteException e) {
247+
Log.e(LOG_TAG, "Couldn't check uid proc state", e);
266248
}
249+
return STATE_GONE;
250+
}
267251

268-
Log.v(LOG_TAG, "Importance changed for " + mPackageName + " (" + mUid + ")."
269-
+ " importance=" + importance);
252+
private int getStateFromProcState(int procState) {
253+
if (procState == ActivityManager.PROCESS_STATE_NONEXISTENT) {
254+
return STATE_GONE;
255+
} else {
256+
if (procState > ActivityManager.PROCESS_STATE_FOREGROUND_SERVICE) {
257+
return STATE_TIMER;
258+
} else {
259+
return STATE_ACTIVE;
260+
}
261+
}
262+
}
263+
264+
private void updateUidState() {
265+
updateUidState(getCurrentState());
266+
}
267+
268+
private void updateUidState(int state) {
269+
Log.v(LOG_TAG, "Updating state for " + mPackageName + " (" + mUid + ")."
270+
+ " state=" + state);
270271
synchronized (mInnerLock) {
271272
// Remove any pending inactivity callback
272273
mHandler.removeCallbacksAndMessages(mToken);
273274

274-
if (importance > IMPORTANCE_CACHED) {
275+
if (state == STATE_GONE) {
275276
if (mRevokeAfterKilledDelay == 0) {
276277
onPackageInactiveLocked();
277278
return;
278279
}
279280
// Delay revocation in case app is restarting
280281
mHandler.postDelayed(() -> {
281-
int imp = mActivityManager.getUidImportance(mUid);
282-
if (imp > IMPORTANCE_CACHED) {
283-
onPackageInactiveLocked();
284-
} else {
285-
if (DEBUG) {
286-
Log.d(LOG_TAG, "No longer gone after delayed revocation. "
287-
+ "Rechecking for " + mPackageName + " (" + mUid + ").");
282+
int currentState;
283+
synchronized (mInnerLock) {
284+
currentState = getCurrentState();
285+
if (currentState == STATE_GONE) {
286+
onPackageInactiveLocked();
287+
return;
288288
}
289-
onImportanceChanged(mUid, imp);
290289
}
290+
if (DEBUG) {
291+
Log.d(LOG_TAG, "No longer gone after delayed revocation. "
292+
+ "Rechecking for " + mPackageName + " (" + mUid
293+
+ ").");
294+
}
295+
updateUidState(currentState);
291296
}, mToken, mRevokeAfterKilledDelay);
292297
return;
293-
}
294-
if (importance > mImportanceToResetTimer) {
298+
} else if (state == STATE_TIMER) {
295299
if (mTimerStart == TIMER_INACTIVE) {
296300
if (DEBUG) {
297301
Log.d(LOG_TAG, "Start the timer for "
298302
+ mPackageName + " (" + mUid + ").");
299303
}
300304
mTimerStart = System.currentTimeMillis();
305+
setAlarmLocked();
301306
}
302-
} else {
307+
} else if (state == STATE_ACTIVE) {
303308
mTimerStart = TIMER_INACTIVE;
304-
}
305-
if (importance > mImportanceToKeepSessionAlive) {
306-
setAlarmLocked();
307-
} else {
308309
cancelAlarmLocked();
309310
}
310311
}
@@ -318,19 +319,9 @@ private void cancel() {
318319
mIsFinished = true;
319320
cancelAlarmLocked();
320321
try {
321-
mActivityManager.removeOnUidImportanceListener(mStartTimerListener);
322-
} catch (IllegalArgumentException e) {
323-
Log.e(LOG_TAG, "Could not remove start timer listener", e);
324-
}
325-
try {
326-
mActivityManager.removeOnUidImportanceListener(mSessionKillableListener);
327-
} catch (IllegalArgumentException e) {
328-
Log.e(LOG_TAG, "Could not remove session killable listener", e);
329-
}
330-
try {
331-
mActivityManager.removeOnUidImportanceListener(mGoneListener);
332-
} catch (IllegalArgumentException e) {
333-
Log.e(LOG_TAG, "Could not remove gone listener", e);
322+
mIActivityManager.unregisterUidObserver(mObserver);
323+
} catch (RemoteException e) {
324+
Log.e(LOG_TAG, "Unable to unregister uid observer.", e);
334325
}
335326
}
336327
}
@@ -394,9 +385,11 @@ private void onPackageInactiveLocked() {
394385
mPermissionControllerManager.notifyOneTimePermissionSessionTimeout(
395386
mPackageName);
396387
});
397-
mActivityManager.removeOnUidImportanceListener(mStartTimerListener);
398-
mActivityManager.removeOnUidImportanceListener(mSessionKillableListener);
399-
mActivityManager.removeOnUidImportanceListener(mGoneListener);
388+
try {
389+
mIActivityManager.unregisterUidObserver(mObserver);
390+
} catch (RemoteException e) {
391+
Log.e(LOG_TAG, "Unable to unregister uid observer.", e);
392+
}
400393
synchronized (mLock) {
401394
mListeners.remove(mUid);
402395
}

services/core/java/com/android/server/pm/permission/PermissionManagerService.java

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -386,8 +386,7 @@ private OneTimePermissionUserManager getOneTimePermissionUserManager(@UserIdInt
386386

387387
@Override
388388
public void startOneTimePermissionSession(String packageName, @UserIdInt int userId,
389-
long timeoutMillis, long revokeAfterKilledDelayMillis, int importanceToResetTimer,
390-
int importanceToKeepSessionAlive) {
389+
long timeoutMillis, long revokeAfterKilledDelayMillis) {
391390
mContext.enforceCallingOrSelfPermission(
392391
Manifest.permission.MANAGE_ONE_TIME_PERMISSION_SESSIONS,
393392
"Must hold " + Manifest.permission.MANAGE_ONE_TIME_PERMISSION_SESSIONS
@@ -397,8 +396,7 @@ public void startOneTimePermissionSession(String packageName, @UserIdInt int use
397396
final long token = Binder.clearCallingIdentity();
398397
try {
399398
getOneTimePermissionUserManager(userId).startPackageOneTimeSession(packageName,
400-
timeoutMillis, revokeAfterKilledDelayMillis, importanceToResetTimer,
401-
importanceToKeepSessionAlive);
399+
timeoutMillis, revokeAfterKilledDelayMillis);
402400
} finally {
403401
Binder.restoreCallingIdentity(token);
404402
}

0 commit comments

Comments
 (0)