From 122843343021b75fced01ebbca46e1f95b7151ab Mon Sep 17 00:00:00 2001 From: Yunqing Wang Date: Wed, 29 Oct 2014 18:38:18 -0700 Subject: [PATCH] Modify the frame context memory deallocation This patch was to fix the vpxdec fuzzing3 test failure. When an error occurs, setjmp() is invoked, which calls the decoder removing routine. In multiple thread situation, other threads could try to access the frame context memory that is already deallocated, thus causing a segfault. An invalid unit test was added for this issue. Change-Id: Ida7442154f3d89759483f0f4fe0324041fffb952 --- test/invalid_file_test.cc | 1 + test/test-data.mk | 2 ++ test/test-data.sha1 | 2 ++ vp9/common/vp9_alloccommon.c | 5 +++++ vp9/decoder/vp9_decoder.c | 5 ----- vp9/encoder/vp9_encoder.c | 9 +++------ 6 files changed, 13 insertions(+), 11 deletions(-) diff --git a/test/invalid_file_test.cc b/test/invalid_file_test.cc index b61d490..039b726 100644 --- a/test/invalid_file_test.cc +++ b/test/invalid_file_test.cc @@ -151,6 +151,7 @@ const DecodeParam kMultiThreadedVP9InvalidFileTests[] = { {4, "invalid-vp90-2-08-tile_1x4_frame_parallel_all_key.webm"}, {4, "invalid-" "vp90-2-08-tile_1x2_frame_parallel.webm.ivf.s47039_r01-05_b6-.ivf"}, + {4, "invalid-vp90-2-08-tile_1x8_frame_parallel.webm.ivf.s288_r01-05_b6-.ivf"}, {2, "invalid-vp90-2-09-aq2.webm.ivf.s3984_r01-05_b6-.v2.ivf"}, {4, "invalid-vp90-2-09-subpixel-00.ivf.s19552_r01-05_b6-.v2.ivf"}, }; diff --git a/test/test-data.mk b/test/test-data.mk index e2da193..0a40eb5 100644 --- a/test/test-data.mk +++ b/test/test-data.mk @@ -695,6 +695,8 @@ LIBVPX_TEST_DATA-$(CONFIG_VP9_DECODER) += invalid-vp90-2-05-resize.ivf.s59293_r0 LIBVPX_TEST_DATA-$(CONFIG_VP9_DECODER) += invalid-vp90-2-05-resize.ivf.s59293_r01-05_b6-.ivf.res LIBVPX_TEST_DATA-$(CONFIG_VP9_DECODER) += invalid-vp90-2-08-tile_1x2_frame_parallel.webm.ivf.s47039_r01-05_b6-.ivf LIBVPX_TEST_DATA-$(CONFIG_VP9_DECODER) += invalid-vp90-2-08-tile_1x2_frame_parallel.webm.ivf.s47039_r01-05_b6-.ivf.res +LIBVPX_TEST_DATA-$(CONFIG_VP9_DECODER) += invalid-vp90-2-08-tile_1x8_frame_parallel.webm.ivf.s288_r01-05_b6-.ivf +LIBVPX_TEST_DATA-$(CONFIG_VP9_DECODER) += invalid-vp90-2-08-tile_1x8_frame_parallel.webm.ivf.s288_r01-05_b6-.ivf.res LIBVPX_TEST_DATA-$(CONFIG_VP9_DECODER) += invalid-vp90-2-08-tile_1x4_frame_parallel_all_key.webm LIBVPX_TEST_DATA-$(CONFIG_VP9_DECODER) += invalid-vp90-2-08-tile_1x4_frame_parallel_all_key.webm.res LIBVPX_TEST_DATA-$(CONFIG_VP9_DECODER) += invalid-vp90-2-09-aq2.webm.ivf.s3984_r01-05_b6-.v2.ivf diff --git a/test/test-data.sha1 b/test/test-data.sha1 index 3d1cd65..428bd56 100644 --- a/test/test-data.sha1 +++ b/test/test-data.sha1 @@ -706,6 +706,8 @@ c12918cf0a716417fba2de35c3fc5ab90e52dfce vp90-2-18-resize.ivf.md5 717da707afcaa1f692ff1946f291054eb75a4f06 screendata.y4m b7c1296630cdf1a7ef493d15ff4f9eb2999202f6 invalid-vp90-2-08-tile_1x2_frame_parallel.webm.ivf.s47039_r01-05_b6-.ivf 0a3884edb3fd8f9d9b500223e650f7de257b67d8 invalid-vp90-2-08-tile_1x2_frame_parallel.webm.ivf.s47039_r01-05_b6-.ivf.res +359e138dfb66863828397b77000ea7a83c844d02 invalid-vp90-2-08-tile_1x8_frame_parallel.webm.ivf.s288_r01-05_b6-.ivf +bbd33de01c17b165b4ce00308e8a19a942023ab8 invalid-vp90-2-08-tile_1x8_frame_parallel.webm.ivf.s288_r01-05_b6-.ivf.res fac89b5735be8a86b0dc05159f996a5c3208ae32 invalid-vp90-2-09-aq2.webm.ivf.s3984_r01-05_b6-.v2.ivf 0a3884edb3fd8f9d9b500223e650f7de257b67d8 invalid-vp90-2-09-aq2.webm.ivf.s3984_r01-05_b6-.v2.ivf.res 4506dfdcdf8ee4250924b075a0dcf1f070f72e5a invalid-vp90-2-09-subpixel-00.ivf.s19552_r01-05_b6-.v2.ivf diff --git a/vp9/common/vp9_alloccommon.c b/vp9/common/vp9_alloccommon.c index 24141d1..284d3a2 100644 --- a/vp9/common/vp9_alloccommon.c +++ b/vp9/common/vp9_alloccommon.c @@ -145,6 +145,11 @@ void vp9_remove_common(VP9_COMMON *cm) { vp9_free_ref_frame_buffers(cm); vp9_free_context_buffers(cm); vp9_free_internal_frame_buffers(&cm->int_frame_buffers); + + vpx_free(cm->fc); + cm->fc = NULL; + vpx_free(cm->frame_contexts); + cm->frame_contexts = NULL; } void vp9_init_context_buffers(VP9_COMMON *cm) { diff --git a/vp9/decoder/vp9_decoder.c b/vp9/decoder/vp9_decoder.c index b0a4065..16f3cd4 100644 --- a/vp9/decoder/vp9_decoder.c +++ b/vp9/decoder/vp9_decoder.c @@ -117,11 +117,6 @@ void vp9_decoder_remove(VP9Decoder *pbi) { VP9_COMMON *const cm = &pbi->common; int i; - vpx_free(cm->fc); - cm->fc = NULL; - vpx_free(cm->frame_contexts); - cm->frame_contexts = NULL; - vp9_get_worker_interface()->end(&pbi->lf_worker); vpx_free(pbi->lf_worker.data1); vpx_free(pbi->tile_data); diff --git a/vp9/encoder/vp9_encoder.c b/vp9/encoder/vp9_encoder.c index 6ddf122..72e11d6 100644 --- a/vp9/encoder/vp9_encoder.c +++ b/vp9/encoder/vp9_encoder.c @@ -201,10 +201,6 @@ static void dealloc_compressor_data(VP9_COMP *cpi) { VP9_COMMON *const cm = &cpi->common; int i; - vpx_free(cm->fc); - cm->fc = NULL; - vpx_free(cm->frame_contexts); - cm->frame_contexts = NULL; vpx_free(cpi->tile_data); cpi->tile_data = NULL; @@ -1733,12 +1729,13 @@ VP9_COMP *vp9_create_compressor(VP9EncoderConfig *oxcf) { } void vp9_remove_compressor(VP9_COMP *cpi) { + VP9_COMMON *const cm = &cpi->common; unsigned int i; if (!cpi) return; - if (cpi && (cpi->common.current_video_frame > 0)) { + if (cpi && (cm->current_video_frame > 0)) { #if CONFIG_INTERNAL_STATS vp9_clear_system_state(); @@ -1821,7 +1818,7 @@ void vp9_remove_compressor(VP9_COMP *cpi) { } #endif - vp9_remove_common(&cpi->common); + vp9_remove_common(cm); vpx_free(cpi); #if CONFIG_VP9_TEMPORAL_DENOISING -- 2.7.4