From 33011754e616286d68b36bee0d48543ea64808b5 Mon Sep 17 00:00:00 2001 From: DRC Date: Fri, 12 Apr 2019 08:47:28 -0500 Subject: [PATCH] Android: Fix "using JNI after critical get" errors 5ea77d8b771bf7b0be7acc7fb70c255f8c9c8f77 was insufficient to fix all of these. In particular, we need to always release the primitive arrays before throwing an exception, because throwing an exception qualifies as "using JNI." Refer to #300 --- turbojpeg-jni.c | 228 ++++++++++++++++++++++++++++++------------------ 1 file changed, 144 insertions(+), 84 deletions(-) diff --git a/turbojpeg-jni.c b/turbojpeg-jni.c index 13f18f7..070dd90 100644 --- a/turbojpeg-jni.c +++ b/turbojpeg-jni.c @@ -98,6 +98,12 @@ } \ } +#define SAFE_RELEASE(javaArray, cArray) { \ + if (javaArray && cArray) \ + (*env)->ReleasePrimitiveArrayCritical(env, javaArray, (void *)cArray, 0); \ + cArray = NULL; \ +} + int ProcessSystemProperties(JNIEnv *env) { jclass cls; @@ -242,12 +248,15 @@ static jint TJCompressor_compress if (tjCompress2(handle, &srcBuf[y * actualPitch + x * tjPixelSize[pf]], width, pitch, height, pf, &jpegBuf, &jpegSize, jpegSubsamp, - jpegQual, flags | TJFLAG_NOREALLOC) == -1) + jpegQual, flags | TJFLAG_NOREALLOC) == -1) { + SAFE_RELEASE(dst, jpegBuf); + SAFE_RELEASE(src, srcBuf); THROW_TJ(); + } bailout: - if (jpegBuf) (*env)->ReleasePrimitiveArrayCritical(env, dst, jpegBuf, 0); - if (srcBuf) (*env)->ReleasePrimitiveArrayCritical(env, src, srcBuf, 0); + SAFE_RELEASE(dst, jpegBuf); + SAFE_RELEASE(src, srcBuf); return (jint)jpegSize; } @@ -343,23 +352,40 @@ JNIEXPORT jint JNICALL Java_org_libjpegturbo_turbojpeg_TJCompressor_compressFrom if (ProcessSystemProperties(env) < 0) goto bailout; +#define RELEASE_ARRAYS_COMPRESSFROMYUV() { \ + SAFE_RELEASE(dst, jpegBuf); \ + for (i = 0; i < nc; i++) \ + SAFE_RELEASE(jSrcPlanes[i], srcPlanes[i]); \ + SAFE_RELEASE(jSrcStrides, srcStrides); \ + SAFE_RELEASE(jSrcOffsets, srcOffsets); \ +} + BAILIF0(srcOffsets = (*env)->GetPrimitiveArrayCritical(env, jSrcOffsets, 0)); BAILIF0(srcStrides = (*env)->GetPrimitiveArrayCritical(env, jSrcStrides, 0)); for (i = 0; i < nc; i++) { int planeSize = tjPlaneSizeYUV(i, width, srcStrides[i], height, subsamp); int pw = tjPlaneWidth(i, width, subsamp); - if (planeSize < 0 || pw < 0) + if (planeSize < 0 || pw < 0) { + RELEASE_ARRAYS_COMPRESSFROMYUV(); THROW_ARG(tjGetErrorStr()); + } - if (srcOffsets[i] < 0) + if (srcOffsets[i] < 0) { + RELEASE_ARRAYS_COMPRESSFROMYUV(); THROW_ARG("Invalid argument in compressFromYUV()"); - if (srcStrides[i] < 0 && srcOffsets[i] - planeSize + pw < 0) + } + if (srcStrides[i] < 0 && srcOffsets[i] - planeSize + pw < 0) { + RELEASE_ARRAYS_COMPRESSFROMYUV(); THROW_ARG("Negative plane stride would cause memory to be accessed below plane boundary"); + } BAILIF0(jSrcPlanes[i] = (*env)->GetObjectArrayElement(env, srcobjs, i)); - if ((*env)->GetArrayLength(env, jSrcPlanes[i]) < srcOffsets[i] + planeSize) + if ((*env)->GetArrayLength(env, jSrcPlanes[i]) < + srcOffsets[i] + planeSize) { + RELEASE_ARRAYS_COMPRESSFROMYUV(); THROW_ARG("Source plane is not large enough"); + } BAILIF0(srcPlanes[i] = (*env)->GetPrimitiveArrayCritical(env, jSrcPlanes[i], 0)); @@ -369,20 +395,13 @@ JNIEXPORT jint JNICALL Java_org_libjpegturbo_turbojpeg_TJCompressor_compressFrom if (tjCompressFromYUVPlanes(handle, srcPlanes, width, srcStrides, height, subsamp, &jpegBuf, &jpegSize, jpegQual, - flags | TJFLAG_NOREALLOC) == -1) + flags | TJFLAG_NOREALLOC) == -1) { + RELEASE_ARRAYS_COMPRESSFROMYUV(); THROW_TJ(); + } bailout: - if (jpegBuf) (*env)->ReleasePrimitiveArrayCritical(env, dst, jpegBuf, 0); - for (i = 0; i < nc; i++) { - if (srcPlanes[i] && jSrcPlanes[i]) - (*env)->ReleasePrimitiveArrayCritical(env, jSrcPlanes[i], - (unsigned char *)srcPlanes[i], 0); - } - if (srcStrides) - (*env)->ReleasePrimitiveArrayCritical(env, jSrcStrides, srcStrides, 0); - if (srcOffsets) - (*env)->ReleasePrimitiveArrayCritical(env, jSrcOffsets, srcOffsets, 0); + RELEASE_ARRAYS_COMPRESSFROMYUV(); return (jint)jpegSize; } @@ -420,23 +439,40 @@ static void TJCompressor_encodeYUV if ((*env)->GetArrayLength(env, src) * srcElementSize < arraySize) THROW_ARG("Source buffer is not large enough"); +#define RELEASE_ARRAYS_ENCODEYUV() { \ + SAFE_RELEASE(src, srcBuf); \ + for (i = 0; i < nc; i++) \ + SAFE_RELEASE(jDstPlanes[i], dstPlanes[i]); \ + SAFE_RELEASE(jDstStrides, dstStrides); \ + SAFE_RELEASE(jDstOffsets, dstOffsets); \ +} + BAILIF0(dstOffsets = (*env)->GetPrimitiveArrayCritical(env, jDstOffsets, 0)); BAILIF0(dstStrides = (*env)->GetPrimitiveArrayCritical(env, jDstStrides, 0)); for (i = 0; i < nc; i++) { int planeSize = tjPlaneSizeYUV(i, width, dstStrides[i], height, subsamp); int pw = tjPlaneWidth(i, width, subsamp); - if (planeSize < 0 || pw < 0) + if (planeSize < 0 || pw < 0) { + RELEASE_ARRAYS_ENCODEYUV(); THROW_ARG(tjGetErrorStr()); + } - if (dstOffsets[i] < 0) + if (dstOffsets[i] < 0) { + RELEASE_ARRAYS_ENCODEYUV(); THROW_ARG("Invalid argument in encodeYUV()"); - if (dstStrides[i] < 0 && dstOffsets[i] - planeSize + pw < 0) + } + if (dstStrides[i] < 0 && dstOffsets[i] - planeSize + pw < 0) { + RELEASE_ARRAYS_ENCODEYUV(); THROW_ARG("Negative plane stride would cause memory to be accessed below plane boundary"); + } BAILIF0(jDstPlanes[i] = (*env)->GetObjectArrayElement(env, dstobjs, i)); - if ((*env)->GetArrayLength(env, jDstPlanes[i]) < dstOffsets[i] + planeSize) + if ((*env)->GetArrayLength(env, jDstPlanes[i]) < + dstOffsets[i] + planeSize) { + RELEASE_ARRAYS_ENCODEYUV(); THROW_ARG("Destination plane is not large enough"); + } BAILIF0(dstPlanes[i] = (*env)->GetPrimitiveArrayCritical(env, jDstPlanes[i], 0)); @@ -446,20 +482,13 @@ static void TJCompressor_encodeYUV if (tjEncodeYUVPlanes(handle, &srcBuf[y * actualPitch + x * tjPixelSize[pf]], width, pitch, height, pf, dstPlanes, dstStrides, - subsamp, flags) == -1) + subsamp, flags) == -1) { + RELEASE_ARRAYS_ENCODEYUV(); THROW_TJ(); + } bailout: - if (srcBuf) (*env)->ReleasePrimitiveArrayCritical(env, src, srcBuf, 0); - for (i = 0; i < nc; i++) { - if (dstPlanes[i] && jDstPlanes[i]) - (*env)->ReleasePrimitiveArrayCritical(env, jDstPlanes[i], dstPlanes[i], - 0); - } - if (dstStrides) - (*env)->ReleasePrimitiveArrayCritical(env, jDstStrides, dstStrides, 0); - if (dstOffsets) - (*env)->ReleasePrimitiveArrayCritical(env, jDstOffsets, dstOffsets, 0); + RELEASE_ARRAYS_ENCODEYUV(); } /* TurboJPEG 1.4.x: TJCompressor::encodeYUV() byte source */ @@ -518,12 +547,15 @@ JNIEXPORT void JNICALL TJCompressor_encodeYUV_12 BAILIF0(dstBuf = (*env)->GetPrimitiveArrayCritical(env, dst, 0)); if (tjEncodeYUV2(handle, srcBuf, width, pitch, height, pf, dstBuf, subsamp, - flags) == -1) + flags) == -1) { + SAFE_RELEASE(dst, dstBuf); + SAFE_RELEASE(src, srcBuf); THROW_TJ(); + } bailout: - if (dstBuf) (*env)->ReleasePrimitiveArrayCritical(env, dst, dstBuf, 0); - if (srcBuf) (*env)->ReleasePrimitiveArrayCritical(env, src, srcBuf, 0); + SAFE_RELEASE(dst, dstBuf); + SAFE_RELEASE(src, srcBuf); } /* TurboJPEG 1.2.x: TJCompressor::encodeYUV() byte source */ @@ -634,11 +666,12 @@ JNIEXPORT void JNICALL Java_org_libjpegturbo_turbojpeg_TJDecompressor_decompress BAILIF0(jpegBuf = (*env)->GetPrimitiveArrayCritical(env, src, 0)); if (tjDecompressHeader3(handle, jpegBuf, (unsigned long)jpegSize, &width, - &height, &jpegSubsamp, &jpegColorspace) == -1) + &height, &jpegSubsamp, &jpegColorspace) == -1) { + SAFE_RELEASE(src, jpegBuf); THROW_TJ(); + } - (*env)->ReleasePrimitiveArrayCritical(env, src, jpegBuf, 0); - jpegBuf = NULL; + SAFE_RELEASE(src, jpegBuf); BAILIF0(_fid = (*env)->GetFieldID(env, _cls, "jpegSubsamp", "I")); (*env)->SetIntField(env, obj, _fid, jpegSubsamp); @@ -652,7 +685,7 @@ JNIEXPORT void JNICALL Java_org_libjpegturbo_turbojpeg_TJDecompressor_decompress (*env)->SetIntField(env, obj, _fid, height); bailout: - if (jpegBuf) (*env)->ReleasePrimitiveArrayCritical(env, src, jpegBuf, 0); + SAFE_RELEASE(src, jpegBuf); } static void TJDecompressor_decompress @@ -683,12 +716,15 @@ static void TJDecompressor_decompress if (tjDecompress2(handle, jpegBuf, (unsigned long)jpegSize, &dstBuf[y * actualPitch + x * tjPixelSize[pf]], width, - pitch, height, pf, flags) == -1) + pitch, height, pf, flags) == -1) { + SAFE_RELEASE(dst, dstBuf); + SAFE_RELEASE(src, jpegBuf); THROW_TJ(); + } bailout: - if (dstBuf) (*env)->ReleasePrimitiveArrayCritical(env, dst, dstBuf, 0); - if (jpegBuf) (*env)->ReleasePrimitiveArrayCritical(env, src, jpegBuf, 0); + SAFE_RELEASE(dst, dstBuf); + SAFE_RELEASE(src, jpegBuf); } /* TurboJPEG 1.3.x: TJDecompressor::decompress() byte destination */ @@ -786,6 +822,14 @@ JNIEXPORT void JNICALL Java_org_libjpegturbo_turbojpeg_TJDecompressor_decompress if (i >= nsf) THROW_ARG("Could not scale down to desired image dimensions"); +#define RELEASE_ARRAYS_DECOMPRESSTOYUV() { \ + SAFE_RELEASE(src, jpegBuf); \ + for (i = 0; i < nc; i++) \ + SAFE_RELEASE(jDstPlanes[i], dstPlanes[i]); \ + SAFE_RELEASE(jDstStrides, dstStrides); \ + SAFE_RELEASE(jDstOffsets, dstOffsets); \ +} + BAILIF0(dstOffsets = (*env)->GetPrimitiveArrayCritical(env, jDstOffsets, 0)); BAILIF0(dstStrides = (*env)->GetPrimitiveArrayCritical(env, jDstStrides, 0)); for (i = 0; i < nc; i++) { @@ -793,17 +837,26 @@ JNIEXPORT void JNICALL Java_org_libjpegturbo_turbojpeg_TJDecompressor_decompress jpegSubsamp); int pw = tjPlaneWidth(i, scaledWidth, jpegSubsamp); - if (planeSize < 0 || pw < 0) + if (planeSize < 0 || pw < 0) { + RELEASE_ARRAYS_DECOMPRESSTOYUV(); THROW_ARG(tjGetErrorStr()); + } - if (dstOffsets[i] < 0) + if (dstOffsets[i] < 0) { + RELEASE_ARRAYS_DECOMPRESSTOYUV(); THROW_ARG("Invalid argument in decompressToYUV()"); - if (dstStrides[i] < 0 && dstOffsets[i] - planeSize + pw < 0) + } + if (dstStrides[i] < 0 && dstOffsets[i] - planeSize + pw < 0) { + RELEASE_ARRAYS_DECOMPRESSTOYUV(); THROW_ARG("Negative plane stride would cause memory to be accessed below plane boundary"); + } BAILIF0(jDstPlanes[i] = (*env)->GetObjectArrayElement(env, dstobjs, i)); - if ((*env)->GetArrayLength(env, jDstPlanes[i]) < dstOffsets[i] + planeSize) + if ((*env)->GetArrayLength(env, jDstPlanes[i]) < + dstOffsets[i] + planeSize) { + RELEASE_ARRAYS_DECOMPRESSTOYUV(); THROW_ARG("Destination plane is not large enough"); + } BAILIF0(dstPlanes[i] = (*env)->GetPrimitiveArrayCritical(env, jDstPlanes[i], 0)); @@ -813,20 +866,13 @@ JNIEXPORT void JNICALL Java_org_libjpegturbo_turbojpeg_TJDecompressor_decompress if (tjDecompressToYUVPlanes(handle, jpegBuf, (unsigned long)jpegSize, dstPlanes, desiredWidth, dstStrides, - desiredHeight, flags) == -1) + desiredHeight, flags) == -1) { + RELEASE_ARRAYS_DECOMPRESSTOYUV(); THROW_TJ(); + } bailout: - if (jpegBuf) (*env)->ReleasePrimitiveArrayCritical(env, src, jpegBuf, 0); - for (i = 0; i < nc; i++) { - if (dstPlanes[i] && jDstPlanes[i]) - (*env)->ReleasePrimitiveArrayCritical(env, jDstPlanes[i], dstPlanes[i], - 0); - } - if (dstStrides) - (*env)->ReleasePrimitiveArrayCritical(env, jDstStrides, dstStrides, 0); - if (dstOffsets) - (*env)->ReleasePrimitiveArrayCritical(env, jDstOffsets, dstOffsets, 0); + RELEASE_ARRAYS_DECOMPRESSTOYUV(); } /* TurboJPEG 1.2.x: TJDecompressor::decompressToYUV() */ @@ -856,12 +902,15 @@ JNIEXPORT void JNICALL Java_org_libjpegturbo_turbojpeg_TJDecompressor_decompress BAILIF0(dstBuf = (*env)->GetPrimitiveArrayCritical(env, dst, 0)); if (tjDecompressToYUV(handle, jpegBuf, (unsigned long)jpegSize, dstBuf, - flags) == -1) + flags) == -1) { + SAFE_RELEASE(dst, dstBuf); + SAFE_RELEASE(src, jpegBuf); THROW_TJ(); + } bailout: - if (dstBuf) (*env)->ReleasePrimitiveArrayCritical(env, dst, dstBuf, 0); - if (jpegBuf) (*env)->ReleasePrimitiveArrayCritical(env, src, jpegBuf, 0); + SAFE_RELEASE(dst, dstBuf); + SAFE_RELEASE(src, jpegBuf); } static void TJDecompressor_decodeYUV @@ -898,23 +947,40 @@ static void TJDecompressor_decodeYUV if ((*env)->GetArrayLength(env, dst) * dstElementSize < arraySize) THROW_ARG("Destination buffer is not large enough"); +#define RELEASE_ARRAYS_DECODEYUV() { \ + SAFE_RELEASE(dst, dstBuf); \ + for (i = 0; i < nc; i++) \ + SAFE_RELEASE(jSrcPlanes[i], srcPlanes[i]); \ + SAFE_RELEASE(jSrcStrides, srcStrides); \ + SAFE_RELEASE(jSrcOffsets, srcOffsets); \ +} + BAILIF0(srcOffsets = (*env)->GetPrimitiveArrayCritical(env, jSrcOffsets, 0)); BAILIF0(srcStrides = (*env)->GetPrimitiveArrayCritical(env, jSrcStrides, 0)); for (i = 0; i < nc; i++) { int planeSize = tjPlaneSizeYUV(i, width, srcStrides[i], height, subsamp); int pw = tjPlaneWidth(i, width, subsamp); - if (planeSize < 0 || pw < 0) + if (planeSize < 0 || pw < 0) { + RELEASE_ARRAYS_DECODEYUV(); THROW_ARG(tjGetErrorStr()); + } - if (srcOffsets[i] < 0) + if (srcOffsets[i] < 0) { + RELEASE_ARRAYS_DECODEYUV(); THROW_ARG("Invalid argument in decodeYUV()"); - if (srcStrides[i] < 0 && srcOffsets[i] - planeSize + pw < 0) + } + if (srcStrides[i] < 0 && srcOffsets[i] - planeSize + pw < 0) { + RELEASE_ARRAYS_DECODEYUV(); THROW_ARG("Negative plane stride would cause memory to be accessed below plane boundary"); + } BAILIF0(jSrcPlanes[i] = (*env)->GetObjectArrayElement(env, srcobjs, i)); - if ((*env)->GetArrayLength(env, jSrcPlanes[i]) < srcOffsets[i] + planeSize) + if ((*env)->GetArrayLength(env, jSrcPlanes[i]) < + srcOffsets[i] + planeSize) { + RELEASE_ARRAYS_DECODEYUV(); THROW_ARG("Source plane is not large enough"); + } BAILIF0(srcPlanes[i] = (*env)->GetPrimitiveArrayCritical(env, jSrcPlanes[i], 0)); @@ -924,20 +990,13 @@ static void TJDecompressor_decodeYUV if (tjDecodeYUVPlanes(handle, srcPlanes, srcStrides, subsamp, &dstBuf[y * actualPitch + x * tjPixelSize[pf]], width, - pitch, height, pf, flags) == -1) + pitch, height, pf, flags) == -1) { + RELEASE_ARRAYS_DECODEYUV(); THROW_TJ(); + } bailout: - if (dstBuf) (*env)->ReleasePrimitiveArrayCritical(env, dst, dstBuf, 0); - for (i = 0; i < nc; i++) { - if (srcPlanes[i] && jSrcPlanes[i]) - (*env)->ReleasePrimitiveArrayCritical(env, jSrcPlanes[i], - (unsigned char *)srcPlanes[i], 0); - } - if (srcStrides) - (*env)->ReleasePrimitiveArrayCritical(env, jSrcStrides, srcStrides, 0); - if (srcOffsets) - (*env)->ReleasePrimitiveArrayCritical(env, jSrcOffsets, srcOffsets, 0); + RELEASE_ARRAYS_DECODEYUV(); } /* TurboJPEG 1.4.x: TJDecompressor::decodeYUV() byte destination */ @@ -1149,15 +1208,16 @@ JNIEXPORT jintArray JNICALL Java_org_libjpegturbo_turbojpeg_TJTransformer_transf (*env)->GetPrimitiveArrayCritical(env, jdstBufs[i], 0)); if (tjTransform(handle, jpegBuf, jpegSize, n, dstBufs, dstSizes, t, - flags | TJFLAG_NOREALLOC) == -1) + flags | TJFLAG_NOREALLOC) == -1) { + for (i = 0; i < n; i++) + SAFE_RELEASE(jdstBufs[i], dstBufs[i]); + SAFE_RELEASE(jsrcBuf, jpegBuf); THROW_TJ(); - - for (i = 0; i < n; i++) { - (*env)->ReleasePrimitiveArrayCritical(env, jdstBufs[i], dstBufs[i], 0); - dstBufs[i] = NULL; } - (*env)->ReleasePrimitiveArrayCritical(env, jsrcBuf, jpegBuf, 0); - jpegBuf = NULL; + + for (i = 0; i < n; i++) + SAFE_RELEASE(jdstBufs[i], dstBufs[i]); + SAFE_RELEASE(jsrcBuf, jpegBuf); jdstSizes = (*env)->NewIntArray(env, n); BAILIF0(dstSizesi = (*env)->GetIntArrayElements(env, jdstSizes, 0)); @@ -1172,7 +1232,7 @@ bailout: } free(dstBufs); } - if (jpegBuf) (*env)->ReleasePrimitiveArrayCritical(env, jsrcBuf, jpegBuf, 0); + SAFE_RELEASE(jsrcBuf, jpegBuf); if (jdstBufs) free(jdstBufs); if (dstSizes) free(dstSizes); if (t) free(t); -- 2.50.1