From 94e184a09d53dcf34ca97bf7104988ebc0598d49 Mon Sep 17 00:00:00 2001 From: Pawan Wagh Date: Tue, 21 Mar 2023 02:02:52 +0000 Subject: [PATCH] Adding tests for OOM fix Renaming method isDirectlyHandlingTransaction to isDirectlyHandlingTransactionNative and adding test which should verify - Whether large allocations greate than 1MB for primitive types are failing - Array length limited to 1M for complex objects - Allocations within allowed limit are succeeding Adding a test api in Parcel to mock that binder transaction is being handled. Test: atest -c android.os.ParcelTest Bug: 205282403 Change-Id: I46bc719508c03613646d182571bf773149223bd1 --- core/java/android/os/Binder.java | 19 +++- core/java/android/os/Parcel.java | 7 -- core/jni/android_util_Binder.cpp | 5 +- .../coretests/src/android/os/ParcelTest.java | 89 +++++++++++++++++++ 4 files changed, 110 insertions(+), 10 deletions(-) diff --git a/core/java/android/os/Binder.java b/core/java/android/os/Binder.java index 0cb7df73f9b7b..5d0a723241b0e 100644 --- a/core/java/android/os/Binder.java +++ b/core/java/android/os/Binder.java @@ -325,7 +325,24 @@ public class Binder implements IBinder { * @hide */ @CriticalNative - public static final native boolean isDirectlyHandlingTransaction(); + public static final native boolean isDirectlyHandlingTransactionNative(); + + private static boolean sIsHandlingBinderTransaction = false; + + /** + * @hide + */ + public static final boolean isDirectlyHandlingTransaction() { + return sIsHandlingBinderTransaction || isDirectlyHandlingTransactionNative(); + } + + /** + * This is Test API which will be used to override output of isDirectlyHandlingTransactionNative + * @hide + */ + public static void setIsDirectlyHandlingTransactionOverride(boolean isInTransaction) { + sIsHandlingBinderTransaction = isInTransaction; + } /** * Returns {@code true} if the current thread has had its identity diff --git a/core/java/android/os/Parcel.java b/core/java/android/os/Parcel.java index e5ad2f86ad70c..cdc8268fa7a1e 100644 --- a/core/java/android/os/Parcel.java +++ b/core/java/android/os/Parcel.java @@ -1584,7 +1584,6 @@ public final class Parcel { @Nullable public final boolean[] createBooleanArray() { int N = readInt(); - // Assuming size of 4 byte for boolean. ensureWithinMemoryLimit(SIZE_BOOLEAN, N); // >>2 as a fast divide-by-4 works in the create*Array() functions // because dataAvail() will never return a negative number. 4 is @@ -1628,7 +1627,6 @@ public final class Parcel { @Nullable public short[] createShortArray() { int n = readInt(); - // Assuming size of 2 byte for short. ensureWithinMemoryLimit(SIZE_SHORT, n); if (n >= 0 && n <= (dataAvail() >> 2)) { short[] val = new short[n]; @@ -1668,7 +1666,6 @@ public final class Parcel { @Nullable public final char[] createCharArray() { int N = readInt(); - // Assuming size of 2 byte for char. ensureWithinMemoryLimit(SIZE_CHAR, N); if (N >= 0 && N <= (dataAvail() >> 2)) { char[] val = new char[N]; @@ -1707,7 +1704,6 @@ public final class Parcel { @Nullable public final int[] createIntArray() { int N = readInt(); - // Assuming size of 4 byte for int. ensureWithinMemoryLimit(SIZE_INT, N); if (N >= 0 && N <= (dataAvail() >> 2)) { int[] val = new int[N]; @@ -1746,7 +1742,6 @@ public final class Parcel { @Nullable public final long[] createLongArray() { int N = readInt(); - // Assuming size of 8 byte for long. ensureWithinMemoryLimit(SIZE_LONG, N); // >>3 because stored longs are 64 bits if (N >= 0 && N <= (dataAvail() >> 3)) { @@ -1786,7 +1781,6 @@ public final class Parcel { @Nullable public final float[] createFloatArray() { int N = readInt(); - // Assuming size of 4 byte for float. ensureWithinMemoryLimit(SIZE_FLOAT, N); // >>2 because stored floats are 4 bytes if (N >= 0 && N <= (dataAvail() >> 2)) { @@ -1826,7 +1820,6 @@ public final class Parcel { @Nullable public final double[] createDoubleArray() { int N = readInt(); - // Assuming size of 8 byte for double. ensureWithinMemoryLimit(SIZE_DOUBLE, N); // >>3 because stored doubles are 8 bytes if (N >= 0 && N <= (dataAvail() >> 3)) { diff --git a/core/jni/android_util_Binder.cpp b/core/jni/android_util_Binder.cpp index b24dc8a9e63b5..8bc52b874ae0f 100644 --- a/core/jni/android_util_Binder.cpp +++ b/core/jni/android_util_Binder.cpp @@ -974,7 +974,7 @@ static jint android_os_Binder_getCallingUid() return IPCThreadState::self()->getCallingUid(); } -static jboolean android_os_Binder_isDirectlyHandlingTransaction() { +static jboolean android_os_Binder_isDirectlyHandlingTransactionNative() { return getCurrentServingCall() == BinderCallType::BINDER; } @@ -1082,7 +1082,8 @@ static const JNINativeMethod gBinderMethods[] = { // @CriticalNative { "getCallingUid", "()I", (void*)android_os_Binder_getCallingUid }, // @CriticalNative - { "isDirectlyHandlingTransaction", "()Z", (void*)android_os_Binder_isDirectlyHandlingTransaction }, + { "isDirectlyHandlingTransactionNative", "()Z", + (void*)android_os_Binder_isDirectlyHandlingTransactionNative }, // @CriticalNative { "clearCallingIdentity", "()J", (void*)android_os_Binder_clearCallingIdentity }, // @CriticalNative diff --git a/core/tests/coretests/src/android/os/ParcelTest.java b/core/tests/coretests/src/android/os/ParcelTest.java index e2fe87b4cfe32..4b993fadc1e06 100644 --- a/core/tests/coretests/src/android/os/ParcelTest.java +++ b/core/tests/coretests/src/android/os/ParcelTest.java @@ -246,4 +246,93 @@ public class ParcelTest { assertThrows(IllegalArgumentException.class, () -> Parcel.compareData(pA, -1, pB, iB, 0)); assertThrows(IllegalArgumentException.class, () -> Parcel.compareData(pA, 0, pB, -1, 0)); } + + /*** + * Tests for b/205282403 + * This test checks if allocations made over limit of 1MB for primitive types + * and 1M length for complex objects are not allowed. + */ + @Test + public void testAllocationsOverLimit_whenOverLimit_throws() { + Binder.setIsDirectlyHandlingTransactionOverride(true); + Parcel p = Parcel.obtain(); + p.setDataPosition(0); + p.writeInt(Integer.MAX_VALUE); + + p.setDataPosition(0); + assertThrows(BadParcelableException.class, () ->p.createBooleanArray()); + + p.setDataPosition(0); + assertThrows(BadParcelableException.class, () ->p.createCharArray()); + + p.setDataPosition(0); + assertThrows(BadParcelableException.class, () ->p.createIntArray()); + + p.setDataPosition(0); + assertThrows(BadParcelableException.class, () ->p.createLongArray()); + + p.setDataPosition(0); + assertThrows(BadParcelableException.class, () ->p.createBinderArray()); + + int[] dimensions = new int[]{Integer.MAX_VALUE, 100, 100}; + p.setDataPosition(0); + assertThrows(BadParcelableException.class, + () -> p.createFixedArray(int[][][].class, dimensions)); + + p.setDataPosition(0); + assertThrows(BadParcelableException.class, + () -> p.createFixedArray(String[][][].class, dimensions)); + + p.setDataPosition(0); + assertThrows(BadParcelableException.class, + () -> p.createFixedArray(IBinder[][][].class, dimensions)); + + p.recycle(); + Binder.setIsDirectlyHandlingTransactionOverride(false); + } + + /*** + * Tests for b/205282403 + * This test checks if allocations made under limit of 1MB for primitive types + * and 1M length for complex objects are allowed. + */ + @Test + public void testAllocations_whenWithinLimit() { + Binder.setIsDirectlyHandlingTransactionOverride(true); + Parcel p = Parcel.obtain(); + p.setDataPosition(0); + p.writeInt(100000); + + p.setDataPosition(0); + p.createByteArray(); + + p.setDataPosition(0); + p.createCharArray(); + + p.setDataPosition(0); + p.createIntArray(); + + p.setDataPosition(0); + p.createLongArray(); + + p.setDataPosition(0); + p.createBinderArray(); + + int[] dimensions = new int[]{ 100, 100, 100 }; + + p.setDataPosition(0); + int[][][] data = new int[100][100][100]; + p.writeFixedArray(data, 0, dimensions); + p.setDataPosition(0); + p.createFixedArray(int[][][].class, dimensions); + + p.setDataPosition(0); + IBinder[][][] parcelables = new IBinder[100][100][100]; + p.writeFixedArray(parcelables, 0, dimensions); + p.setDataPosition(0); + p.createFixedArray(IBinder[][][].class, dimensions); + + p.recycle(); + Binder.setIsDirectlyHandlingTransactionOverride(false); + } }