From 5a7e3797c999ebeead812905d5e9ee2ec88248bf Mon Sep 17 00:00:00 2001
From: Benedict Wong <benedictwong@google.com>
Date: Wed, 2 Feb 2022 18:37:39 +0000
Subject: [PATCH] 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
---
 .../android/server/VcnManagementService.java  | 35 +++++++++++++------
 1 file changed, 25 insertions(+), 10 deletions(-)

diff --git a/services/core/java/com/android/server/VcnManagementService.java b/services/core/java/com/android/server/VcnManagementService.java
index 6a7afd90dc96..2d328d8b0949 100644
--- a/services/core/java/com/android/server/VcnManagementService.java
+++ b/services/core/java/com/android/server/VcnManagementService.java
@@ -539,7 +539,13 @@ public class VcnManagementService extends IVcnManagementService.Stub {
     @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 @@ public class VcnManagementService extends IVcnManagementService.Stub {
             @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 @@ public class VcnManagementService extends IVcnManagementService.Stub {
                 // 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);
+                            }
+                        });
                     }
                 }
             }