Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8212034: Potential memory leaks in jpegLoader.c in error case #54

Open
wants to merge 2 commits into
base: master
from
Open
Changes from all commits
Commits
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.

Always

Just for now

@@ -1,5 +1,5 @@
/*
* Copyright (c) 2009, 2018, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2009, 2019, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -686,6 +686,11 @@ static void imageio_abort(JNIEnv *env, jobject this,
data->abortFlag = JNI_TRUE;
}

static void disposeIIO(JNIEnv *env, imageIODataPtr data) {
j_common_ptr info = destroyImageioData(env, data);
imageio_dispose(info);
}

/*************** end of shared utility code ****************/

/********************** Loader Support **************************/
@@ -1275,9 +1280,7 @@ JNIEXPORT void JNICALL Java_com_sun_javafx_iio_jpeg_JPEGImageLoader_initJPEGMeth
JNIEXPORT void JNICALL Java_com_sun_javafx_iio_jpeg_JPEGImageLoader_disposeNative
(JNIEnv *env, jclass cls, jlong ptr) {
imageIODataPtr data = (imageIODataPtr) jlong_to_ptr(ptr);
j_common_ptr info = destroyImageioData(env, data);

imageio_dispose(info);
disposeIIO(env, data);
}

#define JPEG_APP1 (JPEG_APP0 + 1) /* EXIF APP1 marker code */
@@ -1338,6 +1341,8 @@ JNIEXPORT jlong JNICALL Java_com_sun_javafx_iio_jpeg_JPEGImageLoader_initDecompr
char buffer[JMSG_LENGTH_MAX];
(*cinfo->err->format_message) ((struct jpeg_common_struct *) cinfo,
buffer);
free(cinfo->err);
free(cinfo);

This comment has been minimized.

Copy link
@johanvos

johanvos Nov 27, 2019

Collaborator

jerr_mgr is also allocated via malloc, but not freed. Do you want to free that too?

This comment has been minimized.

Copy link
@arapte

arapte Nov 28, 2019

Author

Line: 1332=> jerr_mgr ptr is stored in cinfor->err
cinfo->err = jpeg_std_error(&(jerr_mgr->pub));

so free(cinfo->err) is same as free (jerr_mgr).

free(cinfo->err) is used here [instead of free(jerr_mgr)] to match the free call in imageio_dispose() method.

ThrowByName(env, "java/io/IOException", buffer);
return 0;
}
@@ -1355,6 +1360,7 @@ JNIEXPORT jlong JNICALL Java_com_sun_javafx_iio_jpeg_JPEGImageLoader_initDecompr
cinfo->src =
(struct jpeg_source_mgr *) malloc(sizeof (struct jpeg_source_mgr));
if (cinfo->src == NULL) {
imageio_dispose((j_common_ptr) cinfo);
ThrowByName(env,
"java/lang/OutOfMemoryError",
"Initializing Reader");
@@ -1371,6 +1377,7 @@ JNIEXPORT jlong JNICALL Java_com_sun_javafx_iio_jpeg_JPEGImageLoader_initDecompr
/* set up the association to persist for future calls */
data = initImageioData(env, (j_common_ptr) cinfo, this);
if (data == NULL) {
imageio_dispose((j_common_ptr) cinfo);
ThrowByName(env,
"java/lang/OutOfMemoryError",
"Initializing Reader");
@@ -1379,7 +1386,10 @@ JNIEXPORT jlong JNICALL Java_com_sun_javafx_iio_jpeg_JPEGImageLoader_initDecompr

imageio_set_stream(env, (j_common_ptr) cinfo, data, stream);

if ((*env)->ExceptionCheck(env)) return 0;
if ((*env)->ExceptionCheck(env)) {
disposeIIO(env, data);
return 0;
}

imageio_init_source((j_decompress_ptr) cinfo);

@@ -1390,20 +1400,22 @@ JNIEXPORT jlong JNICALL Java_com_sun_javafx_iio_jpeg_JPEGImageLoader_initDecompr
if (setjmp(jerr->setjmp_buffer)) {
/* If we get here, the JPEG code has signaled an error
while reading the header. */
RELEASE_ARRAYS(env, data, src->next_input_byte);
disposeIIO(env, data);
if (!(*env)->ExceptionOccurred(env)) {
char buffer[JMSG_LENGTH_MAX];
(*cinfo->err->format_message) ((struct jpeg_common_struct *) cinfo,
buffer);
ThrowByName(env, "java/io/IOException", buffer);
}

return 0;
}

if (GET_ARRAYS(env, data, &src->next_input_byte) == NOT_OK) {
ThrowByName(env,
"java/io/IOException",
"Array pin failed");
disposeIIO(env, data);
return 0;
}

@@ -1500,6 +1512,7 @@ JNIEXPORT jlong JNICALL Java_com_sun_javafx_iio_jpeg_JPEGImageLoader_initDecompr
profileData = read_icc_profile(env, cinfo);

if ((*env)->ExceptionCheck(env)) {
disposeIIO(env, data);
return 0;
}

@@ -1512,6 +1525,7 @@ JNIEXPORT jlong JNICALL Java_com_sun_javafx_iio_jpeg_JPEGImageLoader_initDecompr
cinfo->num_components,
profileData);
if ((*env)->ExceptionCheck(env)) {
disposeIIO(env, data);
return 0;
}
}
@@ -1606,8 +1620,8 @@ JNIEXPORT jboolean JNICALL Java_com_sun_javafx_iio_jpeg_JPEGImageLoader_decompre
struct jpeg_source_mgr *src = cinfo->src;
sun_jpeg_error_ptr jerr;
int bytes_per_row = cinfo->output_width * cinfo->output_components;
JSAMPROW scanline_ptr = (JSAMPROW) malloc(bytes_per_row * sizeof (JSAMPLE));
int offset = 0;
JSAMPROW scanline_ptr = NULL;

if (!SAFE_TO_MULT(cinfo->output_width, cinfo->output_components) ||
!SAFE_TO_MULT(bytes_per_row, cinfo->output_height) ||
@@ -1627,14 +1641,6 @@ JNIEXPORT jboolean JNICALL Java_com_sun_javafx_iio_jpeg_JPEGImageLoader_decompre
return JNI_FALSE;
}

if (scanline_ptr == NULL) {
ThrowByName(env,
"java/lang/OutOfMemoryError",
"Reading JPEG Stream");
RELEASE_ARRAYS(env, data, cinfo->src->next_input_byte);
return JNI_FALSE;
}

/* Establish the setjmp return context for sun_jpeg_error_exit to use. */
jerr = (sun_jpeg_error_ptr) cinfo->err;

@@ -1647,26 +1653,35 @@ JNIEXPORT jboolean JNICALL Java_com_sun_javafx_iio_jpeg_JPEGImageLoader_decompre
buffer);
ThrowByName(env, "java/io/IOException", buffer);
}
if (scanline_ptr != NULL) {
free(scanline_ptr);
}
RELEASE_ARRAYS(env, data, cinfo->src->next_input_byte);
return JNI_FALSE;
}

scanline_ptr = (JSAMPROW) malloc(bytes_per_row * sizeof(JSAMPLE));
if (scanline_ptr == NULL) {
ThrowByName(env,
"java/lang/OutOfMemoryError",
"Reading JPEG Stream");
return JNI_FALSE;
}

while (cinfo->output_scanline < cinfo->output_height) {
int num_scanlines;
if (report_progress == JNI_TRUE) {
RELEASE_ARRAYS(env, data, cinfo->src->next_input_byte);
(*env)->CallVoidMethod(env, this,
JPEGImageLoader_updateImageProgressID,
cinfo->output_scanline);
if ((*env)->ExceptionCheck(env)) return JNI_FALSE;
if ((*env)->ExceptionCheck(env)) {
free(scanline_ptr);
return JNI_FALSE;
}
if (GET_ARRAYS(env, data, &cinfo->src->next_input_byte) == NOT_OK) {
ThrowByName(env,
free(scanline_ptr);
ThrowByName(env,
"java/io/IOException",
"Array pin failed");
return JNI_FALSE;
return JNI_FALSE;
}
}

@@ -1676,30 +1691,33 @@ JNIEXPORT jboolean JNICALL Java_com_sun_javafx_iio_jpeg_JPEGImageLoader_decompre
jbyte *body = (*env)->GetPrimitiveArrayCritical(env, barray, &iscopy);
if (body == NULL) {
fprintf(stderr, "decompressIndirect: GetPrimitiveArrayCritical returns NULL: out of memory\n");
free(scanline_ptr);
return JNI_FALSE;
}
memcpy(body+offset,scanline_ptr, bytes_per_row);
(*env)->ReleasePrimitiveArrayCritical(env, barray, body, JNI_ABORT);
offset += bytes_per_row;
}
}
free(scanline_ptr);

if (report_progress == JNI_TRUE) {
RELEASE_ARRAYS(env, data, cinfo->src->next_input_byte);
(*env)->CallVoidMethod(env, this,
JPEGImageLoader_updateImageProgressID,
cinfo->output_height);
if ((*env)->ExceptionCheck(env)) return JNI_FALSE;
if (GET_ARRAYS(env, data, &cinfo->src->next_input_byte) == NOT_OK) {
ThrowByName(env,
if ((*env)->ExceptionCheck(env)) {
return JNI_FALSE;
}
if (GET_ARRAYS(env, data, &cinfo->src->next_input_byte) == NOT_OK) {
ThrowByName(env,
"java/io/IOException",
"Array pin failed");
return JNI_FALSE;
}
return JNI_FALSE;
}
}

jpeg_finish_decompress(cinfo);
free(scanline_ptr);

RELEASE_ARRAYS(env, data, cinfo->src->next_input_byte);
return JNI_TRUE;
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.