From b219d80fb548961970f5e559f4c2d437c123d08d Mon Sep 17 00:00:00 2001
From: Steven Moreland <smoreland@google.com>
Date: Thu, 1 Oct 2020 19:23:30 +0000
Subject: [PATCH] Binder interface tokens: remove extra mallocs

- writeInterfaceToken/enforceInterfaceToken try to get the interface
  token with GetStringRegion, so for classes with short enough
  interface descriptors, the string is allocated on the stack. A size
  of 64 was chosen since the longest interface descriptor in a
  frameworks/base AIDL file is ~40 characters.
- remove extra creation of String16 for writeInterfaceToken

Bug: 169674485
Test: atest aidl_integration_test
Change-Id: I78092abaa4df758ab699fcb103aa619b0b426e6c
---
 core/jni/android_os_Parcel.cpp | 93 ++++++++++++++++++++++------------
 1 file changed, 60 insertions(+), 33 deletions(-)

diff --git a/core/jni/android_os_Parcel.cpp b/core/jni/android_os_Parcel.cpp
index 9ae3d160be981..9c7ee0c641a66 100644
--- a/core/jni/android_os_Parcel.cpp
+++ b/core/jni/android_os_Parcel.cpp
@@ -638,50 +638,77 @@ static jboolean android_os_Parcel_hasFileDescriptors(jlong nativePtr)
     return ret;
 }
 
+// String tries to allocate itself on the stack, within a known size, but will
+// make a heap allocation if not.
+template <size_t StackReserve>
+class StackString {
+public:
+    StackString(JNIEnv* env, jstring str) : mEnv(env), mJStr(str) {
+        LOG_ALWAYS_FATAL_IF(str == nullptr);
+        mSize = env->GetStringLength(str);
+        if (mSize > StackReserve) {
+            mStr = new jchar[mSize];
+        } else {
+            mStr = &mBuffer[0];
+        }
+        mEnv->GetStringRegion(str, 0, mSize, mStr);
+    }
+    ~StackString() {
+        if (mStr != &mBuffer[0]) {
+            delete[] mStr;
+        }
+    }
+    const jchar* str() { return mStr; }
+    jsize size() { return mSize; }
+
+private:
+    JNIEnv* mEnv;
+    jstring mJStr;
+
+    jchar mBuffer[StackReserve];
+    // pointer to &mBuffer[0] if string fits in mBuffer, otherwise owned
+    jchar* mStr;
+    jsize mSize;
+};
+
+// This size is chosen to be longer than most interface descriptors.
+// Ones longer than this will be allocated on the heap.
+typedef StackString<64> InterfaceDescriptorString;
+
 static void android_os_Parcel_writeInterfaceToken(JNIEnv* env, jclass clazz, jlong nativePtr,
                                                   jstring name)
 {
     Parcel* parcel = reinterpret_cast<Parcel*>(nativePtr);
-    if (parcel != NULL) {
-        // In the current implementation, the token is just the serialized interface name that
-        // the caller expects to be invoking
-        const jchar* str = env->GetStringCritical(name, 0);
-        if (str != NULL) {
-            parcel->writeInterfaceToken(String16(
-                  reinterpret_cast<const char16_t*>(str),
-                  env->GetStringLength(name)));
-            env->ReleaseStringCritical(name, str);
-        }
+    if (parcel != nullptr) {
+        InterfaceDescriptorString descriptor(env, name);
+        parcel->writeInterfaceToken(reinterpret_cast<const char16_t*>(descriptor.str()),
+                                    descriptor.size());
     }
 }
 
 static void android_os_Parcel_enforceInterface(JNIEnv* env, jclass clazz, jlong nativePtr, jstring name)
 {
     Parcel* parcel = reinterpret_cast<Parcel*>(nativePtr);
-    if (parcel != NULL) {
-        const jchar* str = env->GetStringCritical(name, 0);
-        if (str) {
-            IPCThreadState* threadState = IPCThreadState::self();
-            const int32_t oldPolicy = threadState->getStrictModePolicy();
-            const bool isValid = parcel->enforceInterface(
-                reinterpret_cast<const char16_t*>(str),
-                env->GetStringLength(name),
-                threadState);
-            env->ReleaseStringCritical(name, str);
-            if (isValid) {
-                const int32_t newPolicy = threadState->getStrictModePolicy();
-                if (oldPolicy != newPolicy) {
-                    // Need to keep the Java-level thread-local strict
-                    // mode policy in sync for the libcore
-                    // enforcements, which involves an upcall back
-                    // into Java.  (We can't modify the
-                    // Parcel.enforceInterface signature, as it's
-                    // pseudo-public, and used via AIDL
-                    // auto-generation...)
-                    set_dalvik_blockguard_policy(env, newPolicy);
-                }
-                return;     // everything was correct -> return silently
+    if (parcel != nullptr) {
+        InterfaceDescriptorString descriptor(env, name);
+        IPCThreadState* threadState = IPCThreadState::self();
+        const int32_t oldPolicy = threadState->getStrictModePolicy();
+        const bool isValid =
+                parcel->enforceInterface(reinterpret_cast<const char16_t*>(descriptor.str()),
+                                         descriptor.size(), threadState);
+        if (isValid) {
+            const int32_t newPolicy = threadState->getStrictModePolicy();
+            if (oldPolicy != newPolicy) {
+                // Need to keep the Java-level thread-local strict
+                // mode policy in sync for the libcore
+                // enforcements, which involves an upcall back
+                // into Java.  (We can't modify the
+                // Parcel.enforceInterface signature, as it's
+                // pseudo-public, and used via AIDL
+                // auto-generation...)
+                set_dalvik_blockguard_policy(env, newPolicy);
             }
+            return; // everything was correct -> return silently
         }
     }