commit 3b60890f6ee807a8bfc44056443f77603c23e6b0
author Tom Sepez <tsepez@chromium.org> Mon May 18 15:46:54 2015 -0700
committer Tom Sepez <tsepez@chromium.org> Mon May 18 15:46:54 2015 -0700
tree 2b573d9f0f62d0a03a5b6e1eeb5e78c4b24ed734
parent 3fea540931b6b2c700c50809a3d4d8a506f4f797

Cleanup if early return from opj_j2k_copy_default_tcp_and_create_tcd().

The opj_j2k_copy_default_tcp_and_create_tcp() function memcpy's a top-level
struct, and then replaces pointers to memory owned by the original struct
with new blocks of memory. Unfortunately, an early return can leave the
copy with pointers to memory it doesn't own, which causes problems when
cleaning up the partially-initialized struct.

The referenced bug is triggered when we get a return at original
line 7969 or 7385 due to OOM.

Moral of the story: creating a "copy constructor" equivalent
based on memcpy() instead of copying field by field for structs
containing pointers is usually a bad idea.

BUG=486538
R=jun_fang@foxitsoftware.com

Review URL: https://codereview.chromium.org/1138033007

core/src/fxcodec/fx_libopenjpeg/libopenjpeg20/j2k.c

diff --git a/core/src/fxcodec/fx_libopenjpeg/libopenjpeg20/j2k.c b/core/src/fxcodec/fx_libopenjpeg/libopenjpeg20/j2k.c
index 73dc5ab..c40ecc3 100644
--- a/core/src/fxcodec/fx_libopenjpeg/libopenjpeg20/j2k.c
+++ b/core/src/fxcodec/fx_libopenjpeg/libopenjpeg20/j2k.c
@@ -7352,6 +7352,12 @@
                 /* Initialize some values of the current tile coding parameters*/
                 l_tcp->ppt = 0;
                 l_tcp->ppt_data = 00;
+                /* Remove memory not owned by this tile in case of early error return. */
+                l_tcp->m_mct_decoding_matrix = 00;
+                l_tcp->m_nb_max_mct_records = 0;
+                l_tcp->m_mct_records = 00;
+                l_tcp->m_nb_max_mcc_records = 0;
+                l_tcp->m_mcc_records = 00;
                 /* Reconnect the tile-compo coding parameters pointer to the current tile coding parameters*/
                 l_tcp->tccps = l_current_tccp;
 
@@ -7389,6 +7395,9 @@
 
                         ++l_src_mct_rec;
                         ++l_dest_mct_rec;
+
+                        /* Update with each pass to free exactly what has been allocated on early return. */
+                        l_tcp->m_nb_max_mct_records += 1;
                 }
 
                 /* Get the mcc_record of the dflt_tile_cp and copy them into the current tile cp*/
@@ -7398,6 +7407,7 @@
                         return OPJ_FALSE;
                 }
                 memcpy(l_tcp->m_mcc_records,l_default_tcp->m_mcc_records,l_mcc_records_size);
+                l_tcp->m_nb_max_mcc_records = l_default_tcp->m_nb_max_mcc_records;
 
                 /* Copy the mcc record data from dflt_tile_cp to the current tile*/
                 l_src_mcc_rec = l_default_tcp->m_mcc_records;