commit 5a7e3797c999ebeead812905d5e9ee2ec88248bf
author Benedict Wong <benedictwong@google.com> Wed Feb 02 18:37:39 2022 +0000
committer Benedict Wong <benedictwong@google.com> Thu Feb 03 02:54:40 2022 +0000
tree dfb9d41a70a75e44e2686fbde80e505de81ab266
parent 86e1a45b56c0274cf69222891351a97e32ea7943

Fix potential for dead binders when calling callbacks

This patch ensures that all callbacks out of the VcnManagementService
are try/catch guarded to ensure that callbacks are not called and throw
an exception.

This is triggered when telephony dies, which results in the
subscriptions changing, and attempting to notify the policy listeners
triggers a DeadObjectException.

Bug: 217537578
Test: atest FrameworksVcnTests
Change-Id: Ibfe82fb187b7989164e22919a72b8c36d2655902

services/core/java/com/android/server/VcnManagementService.java

diff --git a/services/core/java/com/android/server/VcnManagementService.java b/services/core/java/com/android/server/VcnManagementService.java
index 6a7afd9..2d328d8 100644
--- a/services/core/java/com/android/server/VcnManagementService.java
+++ b/services/core/java/com/android/server/VcnManagementService.java
@@ -539,7 +539,13 @@
     @GuardedBy("mLock")
     private void notifyAllPolicyListenersLocked() {
         for (final PolicyListenerBinderDeath policyListener : mRegisteredPolicyListeners.values()) {
-            Binder.withCleanCallingIdentity(() -> policyListener.mListener.onPolicyChanged());
+            Binder.withCleanCallingIdentity(() -> {
+                try {
+                    policyListener.mListener.onPolicyChanged();
+                } catch (RemoteException e) {
+                    logDbg("VcnStatusCallback threw on VCN status change", e);
+                }
+            });
         }
     }
 
@@ -548,8 +554,13 @@
             @NonNull ParcelUuid subGroup, @VcnStatusCode int statusCode) {
         for (final VcnStatusCallbackInfo cbInfo : mRegisteredStatusCallbacks.values()) {
             if (isCallbackPermissioned(cbInfo, subGroup)) {
-                Binder.withCleanCallingIdentity(
-                        () -> cbInfo.mCallback.onVcnStatusChanged(statusCode));
+                Binder.withCleanCallingIdentity(() -> {
+                    try {
+                        cbInfo.mCallback.onVcnStatusChanged(statusCode);
+                    } catch (RemoteException e) {
+                        logDbg("VcnStatusCallback threw on VCN status change", e);
+                    }
+                });
             }
         }
     }
@@ -1222,13 +1233,17 @@
                 // Notify all registered StatusCallbacks for this subGroup
                 for (VcnStatusCallbackInfo cbInfo : mRegisteredStatusCallbacks.values()) {
                     if (isCallbackPermissioned(cbInfo, mSubGroup)) {
-                        Binder.withCleanCallingIdentity(
-                                () ->
-                                        cbInfo.mCallback.onGatewayConnectionError(
-                                                gatewayConnectionName,
-                                                errorCode,
-                                                exceptionClass,
-                                                exceptionMessage));
+                        Binder.withCleanCallingIdentity(() -> {
+                            try {
+                                cbInfo.mCallback.onGatewayConnectionError(
+                                        gatewayConnectionName,
+                                        errorCode,
+                                        exceptionClass,
+                                        exceptionMessage);
+                            } catch (RemoteException e) {
+                                logDbg("VcnStatusCallback threw on VCN status change", e);
+                            }
+                        });
                     }
                 }
             }