Tighten up Binder.clearCallingIdentity() usage.

This is a third CL in a chain that adjusts existing malformed code
to follow AndroidFrameworkBinderIdentity best-practices.

Specifically, if a thread clears an identity they need to restore it
to avoid obscure security vulnerabilities.  In addition, the relevant
"try" block must start immediately after the identity is cleared to
ensure that its restored if/when any exceptions are thrown.

Bug: 155703208
Test: make
Exempt-From-Owner-Approval: trivial refactoring
Change-Id: I74cb958b68d55a647547aae21baff6ddc364859b

10 files changed, 106 insertions, 51 deletions

diff --git a/core/java/android/app/backup/BackupAgent.java b/core/java/android/app/backup/BackupAgent.java
index a62459523c83..065127138ecc 100644
--- a/core/java/android/app/backup/BackupAgent.java
+++ b/core/java/android/app/backup/BackupAgent.java
@@ -1054,14 +1054,15 @@ public abstract class BackupAgent extends ContextWrapper {
                 long quotaBytes,
                 IBackupCallback callbackBinder,
                 int transportFlags) throws RemoteException {
-            // Ensure that we're running with the app's normal permission level
-            final long ident = Binder.clearCallingIdentity();
-
             if (DEBUG) Log.v(TAG, "doBackup() invoked");
+
             BackupDataOutput output = new BackupDataOutput(
                     data.getFileDescriptor(), quotaBytes, transportFlags);
 
             long result = RESULT_ERROR;
+
+            // Ensure that we're running with the app's normal permission level
+            final long ident = Binder.clearCallingIdentity();
             try {
                 BackupAgent.this.onBackup(oldState, output, newState);
                 result = RESULT_SUCCESS;
@@ -1111,9 +1112,6 @@ public abstract class BackupAgent extends ContextWrapper {
         private void doRestoreInternal(ParcelFileDescriptor data, long appVersionCode,
                 ParcelFileDescriptor newState, int token, IBackupManager callbackBinder,
                 List<String> excludedKeys) throws RemoteException {
-            // Ensure that we're running with the app's normal permission level
-            final long ident = Binder.clearCallingIdentity();
-
             if (DEBUG) Log.v(TAG, "doRestore() invoked");
 
             // Ensure that any side-effect SharedPreferences writes have landed *before*
@@ -1121,6 +1119,9 @@ public abstract class BackupAgent extends ContextWrapper {
             waitForSharedPrefs();
 
             BackupDataInput input = new BackupDataInput(data.getFileDescriptor());
+
+            // Ensure that we're running with the app's normal permission level
+            final long ident = Binder.clearCallingIdentity();
             try {
                 BackupAgent.this.onRestore(input, appVersionCode, newState,
                         excludedKeys != null ? new HashSet<>(excludedKeys)
@@ -1152,15 +1153,14 @@ public abstract class BackupAgent extends ContextWrapper {
         @Override
         public void doFullBackup(ParcelFileDescriptor data,
                 long quotaBytes, int token, IBackupManager callbackBinder, int transportFlags) {
-            // Ensure that we're running with the app's normal permission level
-            final long ident = Binder.clearCallingIdentity();
-
             if (DEBUG) Log.v(TAG, "doFullBackup() invoked");
 
             // Ensure that any SharedPreferences writes have landed *before*
             // we potentially try to back up the underlying files directly.
             waitForSharedPrefs();
 
+            // Ensure that we're running with the app's normal permission level
+            final long ident = Binder.clearCallingIdentity();
             try {
                 BackupAgent.this.onFullBackup(new FullBackupDataOutput(
                         data, quotaBytes, transportFlags));
@@ -1199,12 +1199,13 @@ public abstract class BackupAgent extends ContextWrapper {
 
         public void doMeasureFullBackup(long quotaBytes, int token, IBackupManager callbackBinder,
                 int transportFlags) {
-            // Ensure that we're running with the app's normal permission level
-            final long ident = Binder.clearCallingIdentity();
             FullBackupDataOutput measureOutput =
                     new FullBackupDataOutput(quotaBytes, transportFlags);
 
             waitForSharedPrefs();
+
+            // Ensure that we're running with the app's normal permission level
+            final long ident = Binder.clearCallingIdentity();
             try {
                 BackupAgent.this.onFullBackup(measureOutput);
             } catch (IOException ex) {
@@ -1284,9 +1285,10 @@ public abstract class BackupAgent extends ContextWrapper {
                 long backupDataBytes,
                 long quotaBytes,
                 IBackupCallback callbackBinder) {
-            final long ident = Binder.clearCallingIdentity();
-
             long result = RESULT_ERROR;
+
+            // Ensure that we're running with the app's normal permission level
+            final long ident = Binder.clearCallingIdentity();
             try {
                 BackupAgent.this.onQuotaExceeded(backupDataBytes, quotaBytes);
                 result = RESULT_SUCCESS;
diff --git a/core/java/android/bluetooth/BluetoothHidDevice.java b/core/java/android/bluetooth/BluetoothHidDevice.java
index b5959c06cc1a..2baa73822c9c 100644
--- a/core/java/android/bluetooth/BluetoothHidDevice.java
+++ b/core/java/android/bluetooth/BluetoothHidDevice.java
@@ -342,44 +342,72 @@ public final class BluetoothHidDevice implements BluetoothProfile {
 
         @Override
         public void onAppStatusChanged(BluetoothDevice pluggedDevice, boolean registered) {
-            clearCallingIdentity();
-            mExecutor.execute(() -> mCallback.onAppStatusChanged(pluggedDevice, registered));
+            final long token = clearCallingIdentity();
+            try {
+                mExecutor.execute(() -> mCallback.onAppStatusChanged(pluggedDevice, registered));
+            } finally {
+                restoreCallingIdentity(token);
+            }
         }
 
         @Override
         public void onConnectionStateChanged(BluetoothDevice device, int state) {
-            clearCallingIdentity();
-            mExecutor.execute(() -> mCallback.onConnectionStateChanged(device, state));
+            final long token = clearCallingIdentity();
+            try {
+                mExecutor.execute(() -> mCallback.onConnectionStateChanged(device, state));
+            } finally {
+                restoreCallingIdentity(token);
+            }
         }
 
         @Override
         public void onGetReport(BluetoothDevice device, byte type, byte id, int bufferSize) {
-            clearCallingIdentity();
-            mExecutor.execute(() -> mCallback.onGetReport(device, type, id, bufferSize));
+            final long token = clearCallingIdentity();
+            try {
+                mExecutor.execute(() -> mCallback.onGetReport(device, type, id, bufferSize));
+            } finally {
+                restoreCallingIdentity(token);
+            }
         }
 
         @Override
         public void onSetReport(BluetoothDevice device, byte type, byte id, byte[] data) {
-            clearCallingIdentity();
-            mExecutor.execute(() -> mCallback.onSetReport(device, type, id, data));
+            final long token = clearCallingIdentity();
+            try {
+                mExecutor.execute(() -> mCallback.onSetReport(device, type, id, data));
+            } finally {
+                restoreCallingIdentity(token);
+            }
         }
 
         @Override
         public void onSetProtocol(BluetoothDevice device, byte protocol) {
-            clearCallingIdentity();
-            mExecutor.execute(() -> mCallback.onSetProtocol(device, protocol));
+            final long token = clearCallingIdentity();
+            try {
+                mExecutor.execute(() -> mCallback.onSetProtocol(device, protocol));
+            } finally {
+                restoreCallingIdentity(token);
+            }
         }
 
         @Override
         public void onInterruptData(BluetoothDevice device, byte reportId, byte[] data) {
-            clearCallingIdentity();
-            mExecutor.execute(() -> mCallback.onInterruptData(device, reportId, data));
+            final long token = clearCallingIdentity();
+            try {
+                mExecutor.execute(() -> mCallback.onInterruptData(device, reportId, data));
+            } finally {
+                restoreCallingIdentity(token);
+            }
         }
 
         @Override
         public void onVirtualCableUnplug(BluetoothDevice device) {
-            clearCallingIdentity();
-            mExecutor.execute(() -> mCallback.onVirtualCableUnplug(device));
+            final long token = clearCallingIdentity();
+            try {
+                mExecutor.execute(() -> mCallback.onVirtualCableUnplug(device));
+            } finally {
+                restoreCallingIdentity(token);
+            }
         }
     }
 
diff --git a/core/java/android/content/ContentProvider.java b/core/java/android/content/ContentProvider.java
index 33be50d34777..6cb5b92abb37 100644
--- a/core/java/android/content/ContentProvider.java
+++ b/core/java/android/content/ContentProvider.java
@@ -1043,6 +1043,7 @@ public abstract class ContentProvider implements ContentInterface, ComponentCall
      *         calling identity by passing it to
      *         {@link #restoreCallingIdentity}.
      */
+    @SuppressWarnings("AndroidFrameworkBinderIdentity")
     public final @NonNull CallingIdentity clearCallingIdentity() {
         return new CallingIdentity(Binder.clearCallingIdentity(), setCallingPackage(null));
     }
diff --git a/core/java/android/hardware/camera2/impl/CameraDeviceImpl.java b/core/java/android/hardware/camera2/impl/CameraDeviceImpl.java
index e21b45ab6577..48ec3fd808fe 100644
--- a/core/java/android/hardware/camera2/impl/CameraDeviceImpl.java
+++ b/core/java/android/hardware/camera2/impl/CameraDeviceImpl.java
@@ -1587,7 +1587,7 @@ public class CameraDeviceImpl extends CameraDevice
             }
 
             switch (errorCode) {
-                case CameraDeviceCallbacks.ERROR_CAMERA_DISCONNECTED:
+                case CameraDeviceCallbacks.ERROR_CAMERA_DISCONNECTED: {
                     final long ident = Binder.clearCallingIdentity();
                     try {
                         mDeviceExecutor.execute(mCallOnDisconnected);
@@ -1595,6 +1595,7 @@ public class CameraDeviceImpl extends CameraDevice
                         Binder.restoreCallingIdentity(ident);
                     }
                     break;
+                }
                 case CameraDeviceCallbacks.ERROR_CAMERA_REQUEST:
                 case CameraDeviceCallbacks.ERROR_CAMERA_RESULT:
                 case CameraDeviceCallbacks.ERROR_CAMERA_BUFFER:
diff --git a/core/java/android/hardware/camera2/impl/CameraOfflineSessionImpl.java b/core/java/android/hardware/camera2/impl/CameraOfflineSessionImpl.java
index 413caf5e22e0..1d8b2a123c6a 100644
--- a/core/java/android/hardware/camera2/impl/CameraOfflineSessionImpl.java
+++ b/core/java/android/hardware/camera2/impl/CameraOfflineSessionImpl.java
@@ -147,7 +147,7 @@ public class CameraOfflineSessionImpl extends CameraOfflineSession
                     case CameraDeviceCallbacks.ERROR_CAMERA_BUFFER:
                         onCaptureErrorLocked(errorCode, resultExtras);
                         break;
-                    default:
+                    default: {
                         Runnable errorDispatch = new Runnable() {
                             @Override
                             public void run() {
@@ -164,6 +164,7 @@ public class CameraOfflineSessionImpl extends CameraOfflineSession
                         } finally {
                             Binder.restoreCallingIdentity(ident);
                         }
+                    }
                 }
             }
         }
diff --git a/core/java/android/hardware/hdmi/HdmiControlManager.java b/core/java/android/hardware/hdmi/HdmiControlManager.java
index da4d8e06ad48..e5e73200c8ef 100644
--- a/core/java/android/hardware/hdmi/HdmiControlManager.java
+++ b/core/java/android/hardware/hdmi/HdmiControlManager.java
@@ -1006,8 +1006,12 @@ public final class HdmiControlManager {
         return new IHdmiHotplugEventListener.Stub() {
             @Override
             public void onReceived(HdmiHotplugEvent event) {
-                Binder.clearCallingIdentity();
-                executor.execute(() -> listener.onReceived(event));
+                final long token = Binder.clearCallingIdentity();
+                try {
+                    executor.execute(() -> listener.onReceived(event));
+                } finally {
+                    Binder.restoreCallingIdentity(token);
+                }
             }
         };
     }
@@ -1098,8 +1102,12 @@ public final class HdmiControlManager {
         return new IHdmiControlStatusChangeListener.Stub() {
             @Override
             public void onStatusChange(boolean isCecEnabled, boolean isCecAvailable) {
-                Binder.clearCallingIdentity();
-                executor.execute(() -> listener.onStatusChange(isCecEnabled, isCecAvailable));
+                final long token = Binder.clearCallingIdentity();
+                try {
+                    executor.execute(() -> listener.onStatusChange(isCecEnabled, isCecAvailable));
+                } finally {
+                    Binder.restoreCallingIdentity(token);
+                }
             }
         };
     }
@@ -1171,8 +1179,12 @@ public final class HdmiControlManager {
         return new android.hardware.hdmi.IHdmiCecVolumeControlFeatureListener.Stub() {
             @Override
             public void onHdmiCecVolumeControlFeature(boolean enabled) {
-                Binder.clearCallingIdentity();
-                executor.execute(() -> listener.onHdmiCecVolumeControlFeature(enabled));
+                final long token = Binder.clearCallingIdentity();
+                try {
+                    executor.execute(() -> listener.onHdmiCecVolumeControlFeature(enabled));
+                } finally {
+                    Binder.restoreCallingIdentity(token);
+                }
             }
         };
     }
diff --git a/core/java/android/net/NetworkScoreManager.java b/core/java/android/net/NetworkScoreManager.java
index a190c473f0a0..0ba266345a60 100644
--- a/core/java/android/net/NetworkScoreManager.java
+++ b/core/java/android/net/NetworkScoreManager.java
@@ -511,18 +511,26 @@ public class NetworkScoreManager {
 
         @Override
         public void updateScores(@NonNull List<ScoredNetwork> networks) {
-            Binder.clearCallingIdentity();
-            mExecutor.execute(() -> {
-                mCallback.onScoresUpdated(networks);
-            });
+            final long token = Binder.clearCallingIdentity();
+            try {
+                mExecutor.execute(() -> {
+                    mCallback.onScoresUpdated(networks);
+                });
+            } finally {
+                Binder.restoreCallingIdentity(token);
+            }
         }
 
         @Override
         public void clearScores() {
-            Binder.clearCallingIdentity();
-            mExecutor.execute(() -> {
-                mCallback.onScoresInvalidated();
-            });
+            final long token = Binder.clearCallingIdentity();
+            try {
+                mExecutor.execute(() -> {
+                    mCallback.onScoresInvalidated();
+                });
+            } finally {
+                Binder.restoreCallingIdentity(token);
+            }
         }
     }
 
diff --git a/core/java/android/os/Binder.java b/core/java/android/os/Binder.java
index b737c9f49c18..d492e0870b2e 100644
--- a/core/java/android/os/Binder.java
+++ b/core/java/android/os/Binder.java
@@ -391,8 +391,8 @@ public class Binder implements IBinder {
      * @hide
      */
     public static final void withCleanCallingIdentity(@NonNull ThrowingRunnable action) {
-        final long callingIdentity = clearCallingIdentity();
         Throwable throwableToPropagate = null;
+        final long callingIdentity = clearCallingIdentity();
         try {
             action.runOrThrow();
         } catch (Throwable throwable) {
@@ -415,8 +415,8 @@ public class Binder implements IBinder {
      * @hide
      */
     public static final <T> T withCleanCallingIdentity(@NonNull ThrowingSupplier<T> action) {
-        final long callingIdentity = clearCallingIdentity();
         Throwable throwableToPropagate = null;
+        final long callingIdentity = clearCallingIdentity();
         try {
             return action.getOrThrow();
         } catch (Throwable throwable) {
diff --git a/core/java/android/os/Looper.java b/core/java/android/os/Looper.java
index b05ea39850f8..c39fd4d1bc43 100644
--- a/core/java/android/os/Looper.java
+++ b/core/java/android/os/Looper.java
@@ -155,6 +155,7 @@ public final class Looper {
     /**
      * Poll and deliver single message, return true if the outer loop should continue.
      */
+    @SuppressWarnings("AndroidFrameworkBinderIdentity")
     private static boolean loopOnce(final Looper me,
             final long ident, final int thresholdOverride) {
         Message msg = me.mQueue.next(); // might block
@@ -255,6 +256,7 @@ public final class Looper {
      * Run the message queue in this thread. Be sure to call
      * {@link #quit()} to end the loop.
      */
+    @SuppressWarnings("AndroidFrameworkBinderIdentity")
     public static void loop() {
         final Looper me = myLooper();
         if (me == null) {
diff --git a/core/java/android/view/accessibility/AccessibilityInteractionClient.java b/core/java/android/view/accessibility/AccessibilityInteractionClient.java
index 299ae2f0c55e..d803f8bfa6cc 100644
--- a/core/java/android/view/accessibility/AccessibilityInteractionClient.java
+++ b/core/java/android/view/accessibility/AccessibilityInteractionClient.java
@@ -441,8 +441,8 @@ public final class AccessibilityInteractionClient
                     prefetchFlags &= ~AccessibilityNodeInfo.FLAG_PREFETCH_MASK;
                 }
                 final int interactionId = mInteractionIdCounter.getAndIncrement();
-                final long identityToken = Binder.clearCallingIdentity();
                 final String[] packageNames;
+                final long identityToken = Binder.clearCallingIdentity();
                 try {
                     packageNames = connection.findAccessibilityNodeInfoByAccessibilityId(
                             accessibilityWindowId, accessibilityNodeId, interactionId, this,
@@ -501,8 +501,8 @@ public final class AccessibilityInteractionClient
             IAccessibilityServiceConnection connection = getConnection(connectionId);
             if (connection != null) {
                 final int interactionId = mInteractionIdCounter.getAndIncrement();
-                final long identityToken = Binder.clearCallingIdentity();
                 final String[] packageNames;
+                final long identityToken = Binder.clearCallingIdentity();
                 try {
                     packageNames = connection.findAccessibilityNodeInfosByViewId(
                             accessibilityWindowId, accessibilityNodeId, viewId, interactionId, this,
@@ -555,8 +555,8 @@ public final class AccessibilityInteractionClient
             IAccessibilityServiceConnection connection = getConnection(connectionId);
             if (connection != null) {
                 final int interactionId = mInteractionIdCounter.getAndIncrement();
-                final long identityToken = Binder.clearCallingIdentity();
                 final String[] packageNames;
+                final long identityToken = Binder.clearCallingIdentity();
                 try {
                     packageNames = connection.findAccessibilityNodeInfosByText(
                             accessibilityWindowId, accessibilityNodeId, text, interactionId, this,
@@ -608,8 +608,8 @@ public final class AccessibilityInteractionClient
             IAccessibilityServiceConnection connection = getConnection(connectionId);
             if (connection != null) {
                 final int interactionId = mInteractionIdCounter.getAndIncrement();
-                final long identityToken = Binder.clearCallingIdentity();
                 final String[] packageNames;
+                final long identityToken = Binder.clearCallingIdentity();
                 try {
                     packageNames = connection.findFocus(accessibilityWindowId,
                             accessibilityNodeId, focusType, interactionId, this,
@@ -657,8 +657,8 @@ public final class AccessibilityInteractionClient
             IAccessibilityServiceConnection connection = getConnection(connectionId);
             if (connection != null) {
                 final int interactionId = mInteractionIdCounter.getAndIncrement();
-                final long identityToken = Binder.clearCallingIdentity();
                 final String[] packageNames;
+                final long identityToken = Binder.clearCallingIdentity();
                 try {
                     packageNames = connection.focusSearch(accessibilityWindowId,
                             accessibilityNodeId, direction, interactionId, this,
@@ -705,8 +705,8 @@ public final class AccessibilityInteractionClient
             IAccessibilityServiceConnection connection = getConnection(connectionId);
             if (connection != null) {
                 final int interactionId = mInteractionIdCounter.getAndIncrement();
-                final long identityToken = Binder.clearCallingIdentity();
                 final boolean success;
+                final long identityToken = Binder.clearCallingIdentity();
                 try {
                     success = connection.performAccessibilityAction(
                             accessibilityWindowId, accessibilityNodeId, action, arguments,