diff options
5 files changed, 116 insertions, 6 deletions
diff --git a/core/java/android/telephony/PhoneStateListener.java b/core/java/android/telephony/PhoneStateListener.java index f6d56eed0fb3..c36a33f7a53d 100644 --- a/core/java/android/telephony/PhoneStateListener.java +++ b/core/java/android/telephony/PhoneStateListener.java @@ -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; @@ -65,6 +66,43 @@ public class 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 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. * * The PhoneStateListener is not tied to any subscription and unregistered for any update. diff --git a/services/core/java/com/android/server/TelephonyRegistry.java b/services/core/java/com/android/server/TelephonyRegistry.java index 895282c5cb59..2bbf27849005 100644 --- a/services/core/java/com/android/server/TelephonyRegistry.java +++ b/services/core/java/com/android/server/TelephonyRegistry.java @@ -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; @@ -39,8 +40,10 @@ import android.os.Bundle; import android.os.Handler; import android.os.IBinder; 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; @@ -177,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>(); @@ -506,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(); @@ -605,7 +639,7 @@ public class TelephonyRegistry extends ITelephonyRegistry.Stub { synchronized (mRecords) { // register IBinder b = callback.asBinder(); - Record r = add(b); + Record r = add(b, Binder.getCallingUid(), Binder.getCallingPid(), false); if (r == null) { return; @@ -659,7 +693,7 @@ public class TelephonyRegistry extends ITelephonyRegistry.Stub { synchronized (mRecords) { // register IBinder b = callback.asBinder(); - Record r = add(b); + Record r = add(b, Binder.getCallingUid(), Binder.getCallingPid(), false); if (r == null) { return; @@ -789,7 +823,11 @@ public class TelephonyRegistry extends ITelephonyRegistry.Stub { synchronized (mRecords) { // register IBinder b = callback.asBinder(); - Record r = add(b); + boolean doesLimitApply = + Binder.getCallingUid() != Process.SYSTEM_UID + && Binder.getCallingUid() != Process.PHONE_UID + && Binder.getCallingUid() != Process.myUid(); + Record r = add(b, Binder.getCallingUid(), Binder.getCallingPid(), doesLimitApply); if (r == null) { return; @@ -1084,18 +1122,44 @@ public class TelephonyRegistry extends ITelephonyRegistry.Stub { return record.canReadCallLog() ? mCallIncomingNumber[phoneId] : ""; } - private Record add(IBinder binder) { + private Record add(IBinder binder, int callingUid, int callingPid, boolean doesLimitApply) { Record r; synchronized (mRecords) { final int N = mRecords.size(); + // While iterating through the records, keep track of how many we have from this pid. + int numRecordsForPid = 0; for (int i = 0; i < N; i++) { r = mRecords.get(i); if (binder == r.binder) { // Already existed. return r; } + if (r.callerPid == callingPid) { + numRecordsForPid++; + } } + // 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); + 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); + } + r = new Record(); r.binder = binder; r.deathRecipient = new TelephonyRegistryDeathRecipient(binder); diff --git a/services/java/com/android/server/SystemServer.java b/services/java/com/android/server/SystemServer.java index c631eebe4a01..d04186940d2a 100644 --- a/services/java/com/android/server/SystemServer.java +++ b/services/java/com/android/server/SystemServer.java @@ -1069,7 +1069,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(); diff --git a/telephony/java/android/telephony/TelephonyManager.java b/telephony/java/android/telephony/TelephonyManager.java index 8d9615bbbe06..4e5be5c453b7 100644 --- a/telephony/java/android/telephony/TelephonyManager.java +++ b/telephony/java/android/telephony/TelephonyManager.java @@ -5577,6 +5577,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, diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index e093c573247b..86d8a820e4bd 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -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; @@ -347,6 +348,7 @@ public class ConnectivityServiceTest { @Mock IBinder mIBinder; @Mock LocationManager mLocationManager; @Mock AppOpsManager mAppOpsManager; + @Mock TelephonyManager mTelephonyManager; private ArgumentCaptor<ResolverParamsParcel> mResolverParamsParcelCaptor = ArgumentCaptor.forClass(ResolverParamsParcel.class); @@ -433,6 +435,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); } |