From dcd4c584a14c78d5a9361bd265d2a6c67a7f49a2 Mon Sep 17 00:00:00 2001 From: John Reck Date: Fri, 12 Jul 2019 13:13:05 -0700 Subject: [PATCH 1/2] Fix JNI usage bugs. Avoids calling other JNI stuff with a pending exception by ensuring methods return after jniThrowException. Bug: 137323248 Test: app in bug doesn't throw JNI DETECTED ERROR IN APPLICATION: JNI NewDirectByteBuffer called with pending exception java.lang.UnsupportedOperationException: Pixel format: 0x1 is unsupported 2 Change-Id: Id06cb476a02c4180881772708e3bb58511efe573 --- media/jni/android_media_ImageReader.cpp | 12 +++++++++--- media/jni/android_media_ImageWriter.cpp | 11 ++++++++--- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/media/jni/android_media_ImageReader.cpp b/media/jni/android_media_ImageReader.cpp index 77878f83b0724..0a02156ccf760 100644 --- a/media/jni/android_media_ImageReader.cpp +++ b/media/jni/android_media_ImageReader.cpp @@ -416,11 +416,13 @@ static void ImageReader_init(JNIEnv* env, jobject thiz, jobject weakThiz, jint w if (res != OK) { jniThrowExceptionFmt(env, "java/lang/IllegalStateException", "Failed to set buffer consumer default format 0x%x", nativeFormat); + return; } res = bufferConsumer->setDefaultBufferDataSpace(nativeDataspace); if (res != OK) { jniThrowExceptionFmt(env, "java/lang/IllegalStateException", "Failed to set buffer consumer default dataSpace 0x%x", nativeDataspace); + return; } } @@ -704,7 +706,7 @@ static void Image_getLockedImage(JNIEnv* env, jobject thiz, LockedImage *image) // and we don't set them here. } -static void Image_getLockedImageInfo(JNIEnv* env, LockedImage* buffer, int idx, +static bool Image_getLockedImageInfo(JNIEnv* env, LockedImage* buffer, int idx, int32_t writerFormat, uint8_t **base, uint32_t *size, int *pixelStride, int *rowStride) { ALOGV("%s", __FUNCTION__); @@ -713,7 +715,9 @@ static void Image_getLockedImageInfo(JNIEnv* env, LockedImage* buffer, int idx, if (res != OK) { jniThrowExceptionFmt(env, "java/lang/UnsupportedOperationException", "Pixel format: 0x%x is unsupported", buffer->flexFormat); + return false; } + return true; } static jobjectArray Image_createSurfacePlanes(JNIEnv* env, jobject thiz, @@ -756,8 +760,10 @@ static jobjectArray Image_createSurfacePlanes(JNIEnv* env, jobject thiz, } // Create all SurfacePlanes for (int i = 0; i < numPlanes; i++) { - Image_getLockedImageInfo(env, &lockedImg, i, halReaderFormat, - &pData, &dataSize, &pixelStride, &rowStride); + if (!Image_getLockedImageInfo(env, &lockedImg, i, halReaderFormat, + &pData, &dataSize, &pixelStride, &rowStride)) { + return NULL; + } byteBuffer = env->NewDirectByteBuffer(pData, dataSize); if ((byteBuffer == NULL) && (env->ExceptionCheck() == false)) { jniThrowException(env, "java/lang/IllegalStateException", diff --git a/media/jni/android_media_ImageWriter.cpp b/media/jni/android_media_ImageWriter.cpp index 728c55e5e6f79..08d2947b795f9 100644 --- a/media/jni/android_media_ImageWriter.cpp +++ b/media/jni/android_media_ImageWriter.cpp @@ -777,6 +777,7 @@ static void Image_unlockIfLocked(JNIEnv* env, jobject thiz) { status_t res = buffer->unlock(); if (res != OK) { jniThrowRuntimeException(env, "unlock buffer failed"); + return; } ALOGV("Successfully unlocked the image"); } @@ -872,7 +873,7 @@ static void Image_getLockedImage(JNIEnv* env, jobject thiz, LockedImage *image) // and we don't set them here. } -static void Image_getLockedImageInfo(JNIEnv* env, LockedImage* buffer, int idx, +static bool Image_getLockedImageInfo(JNIEnv* env, LockedImage* buffer, int idx, int32_t writerFormat, uint8_t **base, uint32_t *size, int *pixelStride, int *rowStride) { ALOGV("%s", __FUNCTION__); @@ -881,7 +882,9 @@ static void Image_getLockedImageInfo(JNIEnv* env, LockedImage* buffer, int idx, if (res != OK) { jniThrowExceptionFmt(env, "java/lang/UnsupportedOperationException", "Pixel format: 0x%x is unsupported", buffer->flexFormat); + return false; } + return true; } static jobjectArray Image_createSurfacePlanes(JNIEnv* env, jobject thiz, @@ -920,8 +923,10 @@ static jobjectArray Image_createSurfacePlanes(JNIEnv* env, jobject thiz, PublicFormat publicWriterFormat = static_cast(writerFormat); writerFormat = mapPublicFormatToHalFormat(publicWriterFormat); for (int i = 0; i < numPlanes; i++) { - Image_getLockedImageInfo(env, &lockedImg, i, writerFormat, - &pData, &dataSize, &pixelStride, &rowStride); + if (!Image_getLockedImageInfo(env, &lockedImg, i, writerFormat, + &pData, &dataSize, &pixelStride, &rowStride)) { + return NULL; + } byteBuffer = env->NewDirectByteBuffer(pData, dataSize); if ((byteBuffer == NULL) && (env->ExceptionCheck() == false)) { jniThrowException(env, "java/lang/IllegalStateException", From 03bd07aa0d73d3c3d0431cf4ba8a19ca36b43da7 Mon Sep 17 00:00:00 2001 From: John Reck Date: Fri, 12 Jul 2019 13:53:04 -0700 Subject: [PATCH 2/2] Fix exception message buffer->flexFormat is only used if usingRGBAToJpegOverride is true, which appears to be the uncommon case. As such the unsupported format exception should be using the writerFormat instead, which is what's actually being looked up typically. Bug: 137323248 Test: snippet in bug & compare what format is actually switched on with what the exception message claims is unsupported. Change-Id: I54dda0088fd722126e80e38eebe5da2f26378e1d --- media/jni/android_media_ImageWriter.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/media/jni/android_media_ImageWriter.cpp b/media/jni/android_media_ImageWriter.cpp index 08d2947b795f9..6d8b966f35d56 100644 --- a/media/jni/android_media_ImageWriter.cpp +++ b/media/jni/android_media_ImageWriter.cpp @@ -881,7 +881,7 @@ static bool Image_getLockedImageInfo(JNIEnv* env, LockedImage* buffer, int idx, pixelStride, rowStride); if (res != OK) { jniThrowExceptionFmt(env, "java/lang/UnsupportedOperationException", - "Pixel format: 0x%x is unsupported", buffer->flexFormat); + "Pixel format: 0x%x is unsupported", writerFormat); return false; } return true;