mobile: Wrap the direct ByteBuffer created by the JVM with a GlobalRef (

envoyproxy#34006) This is to tell the JVM to not garbage collect the ByteBuffer until it's no longer used or else we might end up with a use-after-free. The GlobalRef will be deleted inside the BufferFragmentImpl releasor. Risk Level: low Testing: unit tests Docs Changes: n/a Release Notes: n/a Platform Specific Features: mobile Signed-off-by: Fredy Wijaya <[email protected]>
fredyw · May 7, 2024 · fd4c01c · fd4c01c
1 parent 4b1c8bc
commit fd4c01c
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 2 deletions.
diff --git a/mobile/library/jni/jni_utility.cc b/mobile/library/jni/jni_utility.cc
@@ -537,12 +537,17 @@ bool isJavaDirectByteBuffer(JniHelper& jni_helper, jobject java_byte_buffer) {
 Buffer::InstancePtr javaDirectByteBufferToCppBufferInstance(JniHelper& jni_helper,
                                                             jobject java_byte_buffer,
                                                             jlong length) {
-  void* java_byte_buffer_address = jni_helper.getDirectBufferAddress(java_byte_buffer);
   RELEASE_ASSERT(java_byte_buffer != nullptr,
                  "The ByteBuffer argument is not a direct ByteBuffer.");
+  // Because the direct ByteBuffer is allocated in the JVM, we need to tell the JVM to not garbage
+  // collect it by wrapping with a GlobalRef.
+  auto java_byte_buffer_global_ref = jni_helper.newGlobalRef(java_byte_buffer).release();
+  void* java_byte_buffer_address = jni_helper.getDirectBufferAddress(java_byte_buffer_global_ref);
   Buffer::BufferFragmentImpl* byte_buffer_fragment = new Buffer::BufferFragmentImpl(
       java_byte_buffer_address, static_cast<size_t>(length),
-      [](const void*, size_t, const Buffer::BufferFragmentImpl* this_fragment) {
+      [java_byte_buffer_global_ref](const void*, size_t,
+                                    const Buffer::BufferFragmentImpl* this_fragment) {
+        getEnv()->DeleteGlobalRef(java_byte_buffer_global_ref);
         delete this_fragment;
       });
   Buffer::InstancePtr cpp_buffer_instance = std::make_unique<Buffer::OwnedImpl>();

diff --git a/mobile/test/jni/BUILD b/mobile/test/jni/BUILD
@@ -214,6 +214,7 @@ cc_library(
     deps = [
         "//library/common/http:header_utility_lib",
         "//library/jni:jni_utility_lib",
+        "//library/jni/types:jni_javavm_lib",
         "@envoy//test/test_common:utility_lib",
     ],
     alwayslink = True,

diff --git a/mobile/test/jni/jni_utility_test.cc b/mobile/test/jni/jni_utility_test.cc
@@ -5,12 +5,21 @@
 #include "absl/container/flat_hash_map.h"
 #include "library/common/http/header_utility.h"
 #include "library/jni/jni_utility.h"
+#include "library/jni/types/java_virtual_machine.h"
 
 // NOLINT(namespace-envoy)
 
 // This file contains JNI implementation used by
 // `test/java/io/envoyproxy/envoymobile/jni/JniUtilityTest.java` unit tests.
 
+extern "C" JNIEXPORT jint JNICALL JNI_OnLoad(JavaVM* vm, void* /*reserved*/) {
+  const auto result = Envoy::JNI::JavaVirtualMachine::initialize(vm);
+  if (result != JNI_OK) {
+    return result;
+  }
+  return Envoy::JNI::JavaVirtualMachine::getJNIVersion();
+}
+
 extern "C" JNIEXPORT jbyteArray JNICALL
 Java_io_envoyproxy_envoymobile_jni_JniUtilityTest_protoJavaByteArrayConversion(JNIEnv* env, jclass,
                                                                                jbyteArray source) {