Add gating, logging for PhoneStateListener's limit

Add gating via PlatformCompat and DeviceConfig and logging via
PlatformCompat to the limit instituted on per-process listeners

Fixes: 152074216
Test: atest CtsTelephonyHostCases
Change-Id: I4d6681d90705b68c3349f4124e434a29b50fd3a2
This commit is contained in:
Hall Liu
2020-03-26 19:09:30 -07:00
parent 2da50cfe33
commit ed6d6e69a8
5 changed files with 90 additions and 14 deletions

View File

@@ -21,6 +21,7 @@ import android.annotation.NonNull;
import android.annotation.RequiresPermission;
import android.annotation.SystemApi;
import android.annotation.TestApi;
import android.compat.annotation.ChangeId;
import android.compat.annotation.UnsupportedAppUsage;
import android.os.Binder;
import android.os.Build;
@@ -64,17 +65,42 @@ public class PhoneStateListener {
private static final String LOG_TAG = "PhoneStateListener";
private static final boolean DBG = false; // STOPSHIP if true
/**
* Experiment flag to set the per-pid registration limit for PhoneStateListeners
*
* Limit on registrations of {@link PhoneStateListener}s on a per-pid
* basis. When this limit is exceeded, any calls to {@link TelephonyManager#listen} will fail
* with an {@link IllegalStateException}.
*
* {@link android.os.Process#PHONE_UID}, {@link android.os.Process#SYSTEM_UID}, and the uid that
* TelephonyRegistry runs under are exempt from this limit.
*
* If the value of the flag is less than 1, enforcement of the limit will be disabled.
* @hide
*/
public static final int PER_PID_REGISTRATION_LIMIT = 50;
public static final String FLAG_PER_PID_REGISTRATION_LIMIT =
"phone_state_listener_per_pid_registration_limit";
/**
* Default value for the per-pid registation limit.
* See {@link #FLAG_PER_PID_REGISTRATION_LIMIT}.
* @hide
*/
public static final int DEFAULT_PER_PID_REGISTRATION_LIMIT = 50;
/**
* This change enables a limit on the number of {@link PhoneStateListener} objects any process
* may register via {@link TelephonyManager#listen}. The default limit is 50, which may change
* via remote device config updates.
*
* This limit is enforced via an {@link IllegalStateException} thrown from
* {@link TelephonyManager#listen} when the offending process attempts to register one too many
* listeners.
*
* @hide
*/
@ChangeId
public static final long PHONE_STATE_LISTENER_LIMIT_CHANGE_ID = 150880553L;
/**
* Stop listening for updates.

View File

@@ -27,6 +27,7 @@ import android.annotation.NonNull;
import android.annotation.Nullable;
import android.app.ActivityManager;
import android.app.AppOpsManager;
import android.app.compat.CompatChanges;
import android.content.BroadcastReceiver;
import android.content.Context;
import android.content.Intent;
@@ -42,6 +43,7 @@ import android.os.Message;
import android.os.Process;
import android.os.RemoteException;
import android.os.UserHandle;
import android.provider.DeviceConfig;
import android.telephony.Annotation;
import android.telephony.Annotation.ApnType;
import android.telephony.Annotation.DataFailureCause;
@@ -178,8 +180,38 @@ public class TelephonyRegistry extends ITelephonyRegistry.Stub {
}
}
/**
* Wrapper class to facilitate testing -- encapsulates bits of configuration that are
* normally fetched from static methods with many dependencies.
*/
public static class ConfigurationProvider {
/**
* @return The per-pid registration limit for PhoneStateListeners, as set from DeviceConfig
* @noinspection ConstantConditions
*/
public int getRegistrationLimit() {
return Binder.withCleanCallingIdentity(() ->
DeviceConfig.getInt(DeviceConfig.NAMESPACE_TELEPHONY,
PhoneStateListener.FLAG_PER_PID_REGISTRATION_LIMIT,
PhoneStateListener.DEFAULT_PER_PID_REGISTRATION_LIMIT));
}
/**
* @param uid uid to check
* @return Whether enforcement of the per-pid registation limit for PhoneStateListeners is
* enabled in PlatformCompat for the given uid.
* @noinspection ConstantConditions
*/
public boolean isRegistrationLimitEnabledInPlatformCompat(int uid) {
return Binder.withCleanCallingIdentity(() -> CompatChanges.isChangeEnabled(
PhoneStateListener.PHONE_STATE_LISTENER_LIMIT_CHANGE_ID, uid));
}
}
private final Context mContext;
private ConfigurationProvider mConfigurationProvider;
// access should be inside synchronized (mRecords) for these two fields
private final ArrayList<IBinder> mRemoveList = new ArrayList<IBinder>();
private final ArrayList<Record> mRecords = new ArrayList<Record>();
@@ -507,10 +539,11 @@ public class TelephonyRegistry extends ITelephonyRegistry.Stub {
// handler before they get to app code.
@VisibleForTesting(visibility = VisibleForTesting.Visibility.PACKAGE)
public TelephonyRegistry(Context context) {
public TelephonyRegistry(Context context, ConfigurationProvider configurationProvider) {
CellLocation location = CellLocation.getEmpty();
mContext = context;
mConfigurationProvider = configurationProvider;
mBatteryStats = BatteryStatsService.getService();
int numPhones = getTelephonyManager().getActiveModemCount();
@@ -606,7 +639,7 @@ public class TelephonyRegistry extends ITelephonyRegistry.Stub {
synchronized (mRecords) {
// register
IBinder b = callback.asBinder();
Record r = add(b, Binder.getCallingPid(), false);
Record r = add(b, Binder.getCallingUid(), Binder.getCallingPid(), false);
if (r == null) {
return;
@@ -660,7 +693,7 @@ public class TelephonyRegistry extends ITelephonyRegistry.Stub {
synchronized (mRecords) {
// register
IBinder b = callback.asBinder();
Record r = add(b, Binder.getCallingPid(), false);
Record r = add(b, Binder.getCallingUid(), Binder.getCallingPid(), false);
if (r == null) {
return;
@@ -790,11 +823,11 @@ public class TelephonyRegistry extends ITelephonyRegistry.Stub {
synchronized (mRecords) {
// register
IBinder b = callback.asBinder();
boolean shouldEnforceListenerLimit =
boolean doesLimitApply =
Binder.getCallingUid() != Process.SYSTEM_UID
&& Binder.getCallingUid() != Process.PHONE_UID
&& Binder.getCallingUid() != Process.myUid();
Record r = add(b, Binder.getCallingPid(), shouldEnforceListenerLimit);
Record r = add(b, Binder.getCallingUid(), Binder.getCallingPid(), doesLimitApply);
if (r == null) {
return;
@@ -1089,7 +1122,7 @@ public class TelephonyRegistry extends ITelephonyRegistry.Stub {
return record.canReadCallLog() ? mCallIncomingNumber[phoneId] : "";
}
private Record add(IBinder binder, int callingPid, boolean enforceLimit) {
private Record add(IBinder binder, int callingUid, int callingPid, boolean doesLimitApply) {
Record r;
synchronized (mRecords) {
@@ -1106,14 +1139,23 @@ public class TelephonyRegistry extends ITelephonyRegistry.Stub {
numRecordsForPid++;
}
}
// If we've exceeded the limit for registrations, log a warning and quit.
if (enforceLimit && numRecordsForPid >= PhoneStateListener.PER_PID_REGISTRATION_LIMIT) {
// If we've exceeded the limit for registrations, log an error and quit.
int registrationLimit = mConfigurationProvider.getRegistrationLimit();
if (doesLimitApply
&& registrationLimit >= 1
&& numRecordsForPid >= registrationLimit) {
String errorMsg = "Pid " + callingPid + " has exceeded the number of permissible"
+ "registered listeners. Ignoring request to add.";
loge(errorMsg);
throw new IllegalStateException(errorMsg);
} else if (enforceLimit
&& numRecordsForPid >= PhoneStateListener.PER_PID_REGISTRATION_LIMIT / 2) {
if (mConfigurationProvider
.isRegistrationLimitEnabledInPlatformCompat(callingUid)) {
throw new IllegalStateException(errorMsg);
}
} else if (doesLimitApply && numRecordsForPid
>= PhoneStateListener.DEFAULT_PER_PID_REGISTRATION_LIMIT / 2) {
// Log the warning independently of the dynamically set limit -- apps shouldn't be
// doing this regardless of whether we're throwing them an exception for it.
Rlog.w(TAG, "Pid " + callingPid + " has exceeded half the number of permissible"
+ "registered listeners. Now at " + numRecordsForPid);
}

View File

@@ -1068,7 +1068,8 @@ public final class SystemServer {
t.traceEnd();
t.traceBegin("StartTelephonyRegistry");
telephonyRegistry = new TelephonyRegistry(context);
telephonyRegistry = new TelephonyRegistry(
context, new TelephonyRegistry.ConfigurationProvider());
ServiceManager.addService("telephony.registry", telephonyRegistry);
t.traceEnd();

View File

@@ -5560,6 +5560,10 @@ public class TelephonyManager {
* call {@link android.os.Binder#clearCallingIdentity()} before calling this method. A
* {@link SecurityException} will be thrown otherwise.
*
* This API should be used sparingly -- large numbers of listeners will cause system
* instability. If a process has registered too many listeners without unregistering them, it
* may encounter an {@link IllegalStateException} when trying to register more listeners.
*
* @param listener The {@link PhoneStateListener} object to register
* (or unregister)
* @param events The telephony state(s) of interest to the listener,

View File

@@ -205,6 +205,7 @@ import android.os.UserManager;
import android.provider.Settings;
import android.security.KeyStore;
import android.system.Os;
import android.telephony.TelephonyManager;
import android.test.mock.MockContentResolver;
import android.text.TextUtils;
import android.util.ArraySet;
@@ -345,6 +346,7 @@ public class ConnectivityServiceTest {
@Mock IBinder mIBinder;
@Mock LocationManager mLocationManager;
@Mock AppOpsManager mAppOpsManager;
@Mock TelephonyManager mTelephonyManager;
private ArgumentCaptor<ResolverParamsParcel> mResolverParamsParcelCaptor =
ArgumentCaptor.forClass(ResolverParamsParcel.class);
@@ -432,6 +434,7 @@ public class ConnectivityServiceTest {
if (Context.ALARM_SERVICE.equals(name)) return mAlarmManager;
if (Context.LOCATION_SERVICE.equals(name)) return mLocationManager;
if (Context.APP_OPS_SERVICE.equals(name)) return mAppOpsManager;
if (Context.TELEPHONY_SERVICE.equals(name)) return mTelephonyManager;
return super.getSystemService(name);
}