diff options
| author | 2023-03-21 02:02:52 +0000 | |
|---|---|---|
| committer | 2023-05-05 21:08:34 +0000 | |
| commit | 94e184a09d53dcf34ca97bf7104988ebc0598d49 (patch) | |
| tree | f5f0999751386bf20ad42163a0897cf00ba95f82 | |
| parent | 7b8e729f1b2542e746bd682cfa7ab8fd6cc4575c (diff) | |
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
| -rw-r--r-- | core/java/android/os/Binder.java | 19 | ||||
| -rw-r--r-- | core/java/android/os/Parcel.java | 7 | ||||
| -rw-r--r-- | core/jni/android_util_Binder.cpp | 5 | ||||
| -rw-r--r-- | core/tests/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 0cb7df73f9b7..5d0a723241b0 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 e5ad2f86ad70..cdc8268fa7a1 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 b24dc8a9e63b..8bc52b874ae0 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 e2fe87b4cfe3..4b993fadc1e0 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); + } } |