diff options
| author | 2024-09-26 00:25:20 +0000 | |
|---|---|---|
| committer | 2024-09-26 10:56:31 -0700 | |
| commit | 4235ddb0ef0a07cfe21bba65d39b8cd1c5bee39a (patch) | |
| tree | 3caf12e6217e83ca503766a7fefdaf5e88dafb7c | |
| parent | 1c8ab2cfa871540a96bb58610dcd244e8ca92c19 (diff) | |
Add binder unhandled exception notification
Exceptions which are not parcelled by binder are dropped on the service
side. For oneway txns, all exceptions are dropped.
Add an extension point to Java binder interface implementations to
generically handle an exception which will be dropped, by
logging/crashing or some other facility.
Bug: 150808347
Test: atest BinderUncaughtExceptionHandlerTest
Test: Manual (handled vs unhandled exception in subsequent CL)
Flag: EXEMPT safe addition, no api
Merged-In: I6aa97c363ccf21af262ad95efc20611c78796e32
Change-Id: I6aa97c363ccf21af262ad95efc20611c78796e32
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:38e04d4ee890cb87e2a5952fd2cb928638bdf8b3)
Change-Id: Ic7696dcb7fee6b1f69ade78e5b496c0b3a0015f6
9 files changed, 423 insertions, 0 deletions
diff --git a/core/java/android/os/Binder.java b/core/java/android/os/Binder.java index b7556dfb51af..5b05cb54ed54 100644 --- a/core/java/android/os/Binder.java +++ b/core/java/android/os/Binder.java @@ -1213,6 +1213,21 @@ public class Binder implements IBinder { } /** + * Called whenever the stub implementation throws an exception which isn't propagated to the + * remote caller by the binder. If this method isn't overridden, this exception is swallowed, + * and some default return values are propagated to the caller. + * + * <br> <b> This should not throw. </b> Doing so would defeat the purpose of this handler, and + * suppress the exception it is handling. + * + * @param code The transaction code being handled + * @param e The exception which was thrown. + * @hide + */ + protected void onUnhandledException(int code, int flags, Exception e) { + } + + /** * @param in The raw file descriptor that an input data stream can be read from. * @param out The raw file descriptor that normal command messages should be written to. * @param err The raw file descriptor that command error messages should be written to. @@ -1517,10 +1532,15 @@ public class Binder implements IBinder { } else { Log.w(TAG, "Caught a RuntimeException from the binder stub implementation.", e); } + onUnhandledException(code, flags, e); } else { // Clear the parcel before writing the exception. reply.setDataSize(0); reply.setDataPosition(0); + // The writeException below won't do anything useful if this is the case. + if (Parcel.getExceptionCode(e) == 0) { + onUnhandledException(code, flags, e); + } reply.writeException(e); } res = true; diff --git a/core/tests/coretests/Android.bp b/core/tests/coretests/Android.bp index 23203f029992..a9ac67db2cbc 100644 --- a/core/tests/coretests/Android.bp +++ b/core/tests/coretests/Android.bp @@ -25,6 +25,7 @@ filegroup { "BinderProxyCountingTestApp/src/**/*.java", "BinderProxyCountingTestService/src/**/*.java", "BinderDeathRecipientHelperApp/src/**/*.java", + "AppThatCallsBinderMethods/src/**/*.kt", ], visibility: ["//visibility:private"], } @@ -141,6 +142,7 @@ android_test { ":BinderFrozenStateChangeCallbackTestApp", ":BinderProxyCountingTestApp", ":BinderProxyCountingTestService", + ":AppThatCallsBinderMethods", ], } diff --git a/core/tests/coretests/AndroidTest.xml b/core/tests/coretests/AndroidTest.xml index 3fdd72935217..dbe0b49f2c32 100644 --- a/core/tests/coretests/AndroidTest.xml +++ b/core/tests/coretests/AndroidTest.xml @@ -25,6 +25,7 @@ <option name="test-file-name" value="BinderFrozenStateChangeCallbackTestApp.apk" /> <option name="test-file-name" value="BinderProxyCountingTestApp.apk" /> <option name="test-file-name" value="BinderProxyCountingTestService.apk" /> + <option name="test-file-name" value="AppThatCallsBinderMethods.apk" /> </target_preparer> <target_preparer class="com.android.tradefed.targetprep.RunCommandTargetPreparer"> diff --git a/core/tests/coretests/AppThatCallsBinderMethods/Android.bp b/core/tests/coretests/AppThatCallsBinderMethods/Android.bp new file mode 100644 index 000000000000..dcc0d4f76bf2 --- /dev/null +++ b/core/tests/coretests/AppThatCallsBinderMethods/Android.bp @@ -0,0 +1,20 @@ +// Copyright (C) 2024 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +android_test_helper_app { + name: "AppThatCallsBinderMethods", + srcs: ["src/**/*.kt"], + platform_apis: true, + static_libs: ["coretests-aidl"], +} diff --git a/core/tests/coretests/AppThatCallsBinderMethods/AndroidManifest.xml b/core/tests/coretests/AppThatCallsBinderMethods/AndroidManifest.xml new file mode 100644 index 000000000000..b2f6d7897681 --- /dev/null +++ b/core/tests/coretests/AppThatCallsBinderMethods/AndroidManifest.xml @@ -0,0 +1,26 @@ +<?xml version="1.0" encoding="utf-8"?> + +<!-- + ~ Copyright (C) 2024 The Android Open Source Project + ~ + ~ Licensed under the Apache License, Version 2.0 (the "License"); + ~ you may not use this file except in compliance with the License. + ~ You may obtain a copy of the License at + ~ + ~ http://www.apache.org/licenses/LICENSE-2.0 + ~ + ~ Unless required by applicable law or agreed to in writing, software + ~ distributed under the License is distributed on an "AS IS" BASIS, + ~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + ~ See the License for the specific language governing permissions and + ~ limitations under the License. + --> + +<manifest xmlns:android="http://schemas.android.com/apk/res/android" + package="com.android.frameworks.coretests.methodcallerhelperapp"> + <application> + <receiver android:name="com.android.frameworks.coretests.methodcallerhelperapp.CallMethodsReceiver" + android:exported="true"/> + </application> + +</manifest> diff --git a/core/tests/coretests/AppThatCallsBinderMethods/src/com/android/frameworks/coretests/methodcallerhelperapp/CallMethodsReceiver.kt b/core/tests/coretests/AppThatCallsBinderMethods/src/com/android/frameworks/coretests/methodcallerhelperapp/CallMethodsReceiver.kt new file mode 100644 index 000000000000..638cc3b7692f --- /dev/null +++ b/core/tests/coretests/AppThatCallsBinderMethods/src/com/android/frameworks/coretests/methodcallerhelperapp/CallMethodsReceiver.kt @@ -0,0 +1,57 @@ +/* + * Copyright (C) 2024 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.frameworks.coretests.methodcallerhelperapp + +import android.content.BroadcastReceiver +import android.content.Context +import android.content.Intent +import android.util.Log + +import com.android.frameworks.coretests.aidl.ITestInterface + +/** + * Receiver used to call methods when a binder is received + * {@link android.os.BinderUncaughtExceptionHandlerTest}. + */ +class CallMethodsReceiver : BroadcastReceiver() { + private val TAG = "CallMethodsReceiver" + + override fun onReceive(context: Context, intent: Intent) { + try { + when (intent.getAction()) { + ACTION_CALL_METHOD -> intent.getExtras()!!.let { + Log.i(TAG, "Received ACTION_CALL_METHOD with extras: $it") + val iface = it.getBinder(EXTRA_BINDER)!!.let(ITestInterface.Stub::asInterface)!! + val name = it.getString(EXTRA_METHOD_NAME)!! + try { + when (name) { + "foo" -> iface.foo(5) + "onewayFoo" -> iface.onewayFoo(5) + "bar" -> iface.bar(5) + else -> Log.e(TAG, "Unknown method name") + } + } catch (e: Exception) { + // Exceptions expected + } + } + else -> Log.e(TAG, "Unknown action " + intent.getAction()) + } + } catch (e: Exception) { + Log.e(TAG, "Exception: ", e) + } + } +} diff --git a/core/tests/coretests/AppThatCallsBinderMethods/src/com/android/frameworks/coretests/methodcallerhelperapp/Constants.kt b/core/tests/coretests/AppThatCallsBinderMethods/src/com/android/frameworks/coretests/methodcallerhelperapp/Constants.kt new file mode 100644 index 000000000000..37c6268164a7 --- /dev/null +++ b/core/tests/coretests/AppThatCallsBinderMethods/src/com/android/frameworks/coretests/methodcallerhelperapp/Constants.kt @@ -0,0 +1,23 @@ +/* + * Copyright (C) 2024 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.frameworks.coretests.methodcallerhelperapp + +const val PACKAGE_NAME = "com.android.frameworks.coretests.methodcallerhelperapp" +const val RECEIVER_NAME = "CallMethodsReceiver" +const val ACTION_CALL_METHOD = PACKAGE_NAME + ".ACTION_CALL_METHOD" +const val EXTRA_METHOD_NAME = PACKAGE_NAME + ".EXTRA_METHOD_NAME" +const val EXTRA_BINDER = PACKAGE_NAME + ".EXTRA_BINDER" diff --git a/core/tests/coretests/aidl/com/android/frameworks/coretests/aidl/ITestInterface.aidl b/core/tests/coretests/aidl/com/android/frameworks/coretests/aidl/ITestInterface.aidl new file mode 100644 index 000000000000..ffcf178beda4 --- /dev/null +++ b/core/tests/coretests/aidl/com/android/frameworks/coretests/aidl/ITestInterface.aidl @@ -0,0 +1,27 @@ +/* + * Copyright (C) 2024 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.frameworks.coretests.aidl; + +/** + * Just an interface with a oneway, void and non-oneway method. + */ +interface ITestInterface { + // Method order matters, since we verify transaction codes + int foo(int a); + oneway void onewayFoo(int a); + void bar(int a); +} diff --git a/core/tests/coretests/src/android/os/BinderUncaughtExceptionHandlerTest.kt b/core/tests/coretests/src/android/os/BinderUncaughtExceptionHandlerTest.kt new file mode 100644 index 000000000000..791c209e4473 --- /dev/null +++ b/core/tests/coretests/src/android/os/BinderUncaughtExceptionHandlerTest.kt @@ -0,0 +1,247 @@ +/* + * Copyright (C) 2024 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package android.os + +import android.content.Intent +import android.platform.test.annotations.DisabledOnRavenwood +import android.platform.test.annotations.Presubmit + +import androidx.test.ext.junit.runners.AndroidJUnit4 +import androidx.test.platform.app.InstrumentationRegistry + +import com.android.frameworks.coretests.aidl.ITestInterface +import com.android.frameworks.coretests.methodcallerhelperapp.* + +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith + +import org.mockito.ArgumentMatcher +import org.mockito.ArgumentMatchers.any +import org.mockito.ArgumentMatchers.anyInt +import org.mockito.ArgumentMatchers.eq +import org.mockito.ArgumentMatchers.intThat +import org.mockito.Mockito.after +import org.mockito.Mockito.doThrow +import org.mockito.Mockito.never +import org.mockito.Mockito.timeout +import org.mockito.Mockito.verify +import org.mockito.Mockito.`when` +import org.mockito.Spy +import org.mockito.junit.MockitoJUnit +import org.mockito.quality.Strictness.STRICT_STUBS + +private const val TIMEOUT_DURATION_MS = 2000L +private const val FALSE_NEG_DURATION_MS = 500L +private const val FLAG_ONEWAY = 1 +// From ITestInterface.Stub class, these values are package private +private const val TRANSACTION_foo = 1 +private const val TRANSACTION_onewayFoo = 2 +private const val TRANSACTION_bar = 3 + +/** Tests functionality of {@link android.os.Binder.onUnhandledException}. */ +@DisabledOnRavenwood(reason = "multi-app") +@Presubmit +@RunWith(AndroidJUnit4::class) +class BinderUncaughtExceptionHandlerTest { + + val mContext = InstrumentationRegistry.getInstrumentation().getTargetContext() + + @Rule @JvmField val rule = MockitoJUnit.rule().strictness(STRICT_STUBS) + + @Spy var mInterfaceImpl: ITestImpl = ITestImpl() + + // This subclass is needed for visibility issues (via protected), since the method we are + // verifying lives on the boot classpath, it is not enough to be in the same package. + open class ITestImpl : ITestInterface.Stub() { + override fun onUnhandledException(code: Int, flags: Int, e: Exception?) = + onUnhandledExceptionVisible(code, flags, e) + + public open fun onUnhandledExceptionVisible(code: Int, flags: Int, e: Exception?) {} + + @Throws(RemoteException::class) + override open fun foo(x: Int): Int = throw UnsupportedOperationException() + + @Throws(RemoteException::class) + override open fun onewayFoo(x: Int): Unit = throw UnsupportedOperationException() + + @Throws(RemoteException::class) + override open fun bar(x: Int): Unit = throw UnsupportedOperationException() + } + + class OnewayMatcher(private val isOneway: Boolean) : ArgumentMatcher<Int> { + override fun matches(argument: Int?) = + (argument!! and FLAG_ONEWAY) == if (isOneway) 1 else 0 + + override fun toString() = "Expected oneway: $isOneway" + } + + @Test + fun testRegularMethod_ifThrowsRuntimeException_HandlerCalled() { + val myException = RuntimeException("Test exception") + doThrow(myException).`when`(mInterfaceImpl).foo(anyInt()) + + dispatchActionCall("foo") + + verify(mInterfaceImpl, timeout(TIMEOUT_DURATION_MS)) + .onUnhandledExceptionVisible( + /* transactionCode = */ eq(TRANSACTION_foo), + /* flags= */ intThat(OnewayMatcher(false)), + /* exception= */ eq(myException), + ) + // No unexpected calls + verify(mInterfaceImpl).onUnhandledExceptionVisible(anyInt(), anyInt(), any()) + } + + @Test + fun testRegularMethod_ifThrowsRemoteException_HandlerCalled() { + val myException = RemoteException("Test exception") + doThrow(myException).`when`(mInterfaceImpl).foo(anyInt()) + + dispatchActionCall("foo") + + verify(mInterfaceImpl, timeout(TIMEOUT_DURATION_MS)) + .onUnhandledExceptionVisible( + /* transactionCode = */ eq(TRANSACTION_foo), + /* flags= */ intThat(OnewayMatcher(false)), + /* exception= */ eq(myException), + ) + // No unexpected calls + verify(mInterfaceImpl).onUnhandledExceptionVisible(anyInt(), anyInt(), any()) + } + + @Test + fun testRegularMethod_ifThrowsSecurityException_HandlerNotCalled() { + val myException = SecurityException("Test exception") + doThrow(myException).`when`(mInterfaceImpl).foo(anyInt()) + + dispatchActionCall("foo") + + // No unexpected calls + verify(mInterfaceImpl, after(FALSE_NEG_DURATION_MS).never()) + .onUnhandledExceptionVisible(anyInt(), anyInt(), any()) + } + + @Test + fun testVoidMethod_ifThrowsRuntimeException_HandlerCalled() { + val myException = RuntimeException("Test exception") + doThrow(myException).`when`(mInterfaceImpl).bar(anyInt()) + + dispatchActionCall("bar") + + verify(mInterfaceImpl, timeout(TIMEOUT_DURATION_MS)) + .onUnhandledExceptionVisible( + /* transactionCode = */ eq(TRANSACTION_bar), + /* flags= */ intThat(OnewayMatcher(false)), + /* exception= */ eq(myException), + ) + // No unexpected calls + verify(mInterfaceImpl).onUnhandledExceptionVisible(anyInt(), anyInt(), any()) + } + + @Test + fun testVoidMethod_ifThrowsRemoteException_HandlerCalled() { + val myException = RemoteException("Test exception") + doThrow(myException).`when`(mInterfaceImpl).bar(anyInt()) + + dispatchActionCall("bar") + + verify(mInterfaceImpl, timeout(TIMEOUT_DURATION_MS)) + .onUnhandledExceptionVisible( + /* transactionCode = */ eq(TRANSACTION_bar), + /* flags= */ intThat(OnewayMatcher(false)), + /* exception= */ eq(myException), + ) + // No unexpected calls + verify(mInterfaceImpl).onUnhandledExceptionVisible(anyInt(), anyInt(), any()) + } + + @Test + fun testVoidMethod_ifThrowsSecurityException_HandlerNotCalled() { + val myException = SecurityException("Test exception") + doThrow(myException).`when`(mInterfaceImpl).bar(anyInt()) + + dispatchActionCall("bar") + + // No unexpected calls + verify(mInterfaceImpl, after(FALSE_NEG_DURATION_MS).never()) + .onUnhandledExceptionVisible(anyInt(), anyInt(), any()) + } + + @Test + fun testOnewayMethod_ifThrowsRuntimeException_HandlerCalled() { + val myException = RuntimeException("Test exception") + doThrow(myException).doNothing().`when`(mInterfaceImpl).onewayFoo(anyInt()) + + dispatchActionCall("onewayFoo") + + verify(mInterfaceImpl, timeout(TIMEOUT_DURATION_MS)) + .onUnhandledExceptionVisible( + /* transactionCode = */ eq(TRANSACTION_onewayFoo), + /* flags= */ intThat(OnewayMatcher(true)), + /* exception= */ eq(myException), + ) + // No unexpected calls + verify(mInterfaceImpl).onUnhandledExceptionVisible(anyInt(), anyInt(), any()) + } + + @Test + fun testOnewayMethod_ifThrowsRemoteException_HandlerCalled() { + val myException = RemoteException("Test exception") + doThrow(myException).`when`(mInterfaceImpl).onewayFoo(anyInt()) + + dispatchActionCall("onewayFoo") + + verify(mInterfaceImpl, timeout(TIMEOUT_DURATION_MS)) + .onUnhandledExceptionVisible( + /* transactionCode = */ eq(TRANSACTION_onewayFoo), + /* flags= */ intThat(OnewayMatcher(true)), + /* exception= */ eq(myException), + ) + // No unexpected calls + verify(mInterfaceImpl).onUnhandledExceptionVisible(anyInt(), anyInt(), any()) + } + + // All exceptions are uncaught for oneway + @Test + fun testOnewayMethod_ifThrowsSecurityException_HandlerCalled() { + val myException = SecurityException("Test exception") + doThrow(myException).`when`(mInterfaceImpl).onewayFoo(anyInt()) + + dispatchActionCall("onewayFoo") + + verify(mInterfaceImpl, timeout(TIMEOUT_DURATION_MS)) + .onUnhandledExceptionVisible( + /* transactionCode = */ eq(TRANSACTION_onewayFoo), + /* flags= */ intThat(OnewayMatcher(true)), + /* exception= */ eq(myException), + ) + // No unexpected calls + verify(mInterfaceImpl).onUnhandledExceptionVisible(anyInt(), anyInt(), any()) + } + + private fun dispatchActionCall(methodName: String) = + Intent(ACTION_CALL_METHOD).apply { + putExtras( + Bundle().apply { + putBinder(EXTRA_BINDER, mInterfaceImpl as IBinder) + putString(EXTRA_METHOD_NAME, methodName) + } + ) + setClassName(PACKAGE_NAME, CallMethodsReceiver::class.java.getName()) + }.let { mContext.sendBroadcast(it) } +} |