Add check to validate allocation size of src_sad_blk_64x64 Fixes heap buffer overflow in dynamic svc. Added unittest to repro the issue. Bug: 502735235 Change-Id: I52e52c8b14a25f453a62d74575ea1cd8c56a3e72 (cherry picked from commit 79401f2946940984aaf3dc50e42d34be2e7bbac9)
diff --git a/av1/encoder/encoder.c b/av1/encoder/encoder.c index a149da7..2d492dd 100644 --- a/av1/encoder/encoder.c +++ b/av1/encoder/encoder.c
@@ -753,6 +753,7 @@ cpi->svc.number_temporal_layers = 1; cm->spatial_layer_id = 0; cm->temporal_layer_id = 0; + cpi->src_sad_blk_alloc_size = 0; // Init rtc_ref parameters. cpi->ppi->rtc_ref.set_ref_frame_config = 0; cpi->ppi->rtc_ref.non_reference_frame = 0;
diff --git a/av1/encoder/encoder.h b/av1/encoder/encoder.h index b4e25d4..82d7168 100644 --- a/av1/encoder/encoder.h +++ b/av1/encoder/encoder.h
@@ -3642,6 +3642,11 @@ uint64_t *src_sad_blk_64x64; /*! + * Size of allocated buffer to store 64x64 SAD, in units of uint64_t. + */ + int src_sad_blk_alloc_size; + + /*! * SSE between the current frame and the reconstructed last frame * It is only used for CBR mode. * It is not used if the reference frame has a different frame size.
diff --git a/av1/encoder/ratectrl.c b/av1/encoder/ratectrl.c index 491e7a5..a27de1a 100644 --- a/av1/encoder/ratectrl.c +++ b/av1/encoder/ratectrl.c
@@ -3293,9 +3293,10 @@ // is changed dynamically without re-alloc of encoder. if (width != cm->render_width || height != cm->render_height || cpi->svc.number_spatial_layers > 1 || unscaled_src == NULL || - unscaled_last_src == NULL) { + unscaled_last_src == NULL || is_one_pass_rt_lag_params(cpi)) { aom_free(cpi->src_sad_blk_64x64); cpi->src_sad_blk_64x64 = NULL; + cpi->src_sad_blk_alloc_size = 0; } if (unscaled_src == NULL || unscaled_last_src == NULL) return; src_y = unscaled_src->y_buffer; @@ -3309,6 +3310,7 @@ if (src_width != last_src_width || src_height != last_src_height) { aom_free(cpi->src_sad_blk_64x64); cpi->src_sad_blk_64x64 = NULL; + cpi->src_sad_blk_alloc_size = 0; return; } rc->high_source_sad = 0; @@ -3360,10 +3362,12 @@ // Store blkwise SAD for later use. Disable for spatial layers for now. if (width == cm->render_width && height == cm->render_height && cpi->svc.number_spatial_layers == 1 && !is_one_pass_rt_lag_params(cpi)) { - if (cpi->src_sad_blk_64x64 == NULL) { + if (cpi->src_sad_blk_alloc_size != sb_cols * sb_rows) { + aom_free(cpi->src_sad_blk_64x64); CHECK_MEM_ERROR(cm, cpi->src_sad_blk_64x64, (uint64_t *)aom_calloc(sb_cols * sb_rows, sizeof(*cpi->src_sad_blk_64x64))); + cpi->src_sad_blk_alloc_size = sb_cols * sb_rows; } } const CommonModeInfoParams *const mi_params = &cpi->common.mi_params; @@ -3931,6 +3935,7 @@ } else { aom_free(cpi->src_sad_blk_64x64); cpi->src_sad_blk_64x64 = NULL; + cpi->src_sad_blk_alloc_size = 0; } } if (cpi->sf.rt_sf.rc_compute_spatial_var_sc_kf &&
diff --git a/test/encode_api_test.cc b/test/encode_api_test.cc index 0dd0af1..8582c15 100644 --- a/test/encode_api_test.cc +++ b/test/encode_api_test.cc
@@ -2002,4 +2002,89 @@ ASSERT_EQ(aom_codec_destroy(&enc), AOM_CODEC_OK); } +TEST(EncodeAPI, DynamicSvcTemporalIssue502735235) { + aom_codec_iface_t *iface = aom_codec_av1_cx(); + aom_codec_enc_cfg_t cfg; + ASSERT_EQ(aom_codec_enc_config_default(iface, &cfg, AOM_USAGE_REALTIME), + AOM_CODEC_OK); + + // Init at 256x512. + cfg.g_w = 256; + cfg.g_h = 512; + cfg.g_timebase.num = 1; + cfg.g_timebase.den = 30; + cfg.rc_end_usage = AOM_CBR; + cfg.rc_target_bitrate = 1000; + cfg.g_lag_in_frames = 0; + + aom_codec_ctx_t codec; + ASSERT_EQ(aom_codec_enc_init(&codec, iface, &cfg, 0), AOM_CODEC_OK); + ASSERT_EQ(aom_codec_control(&codec, AOME_SET_CPUUSED, 10), AOM_CODEC_OK); + + // AV1E_SET_SVC_PARAMS + aom_svc_params_t svc_params = {}; + svc_params.number_spatial_layers = 1; + svc_params.number_temporal_layers = 2; + svc_params.scaling_factor_num[0] = 1; + svc_params.scaling_factor_den[0] = 1; + svc_params.framerate_factor[0] = 2; + svc_params.framerate_factor[1] = 1; + svc_params.max_quantizers[0] = 56; + svc_params.min_quantizers[0] = 10; + svc_params.max_quantizers[1] = 56; + svc_params.min_quantizers[1] = 10; + svc_params.layer_target_bitrate[0] = cfg.rc_target_bitrate * 60 / 100; + svc_params.layer_target_bitrate[1] = cfg.rc_target_bitrate; + EXPECT_EQ(aom_codec_control(&codec, AV1E_SET_SVC_PARAMS, &svc_params), + AOM_CODEC_OK); + + // Encode at 256x512. TL0. + aom_svc_layer_id_t layer_id = {}; + layer_id.spatial_layer_id = 0; + layer_id.spatial_layer_id = 0; + ASSERT_EQ(aom_codec_control(&codec, AV1E_SET_SVC_LAYER_ID, &layer_id), + AOM_CODEC_OK); + aom_image_t *raw = aom_img_alloc(nullptr, AOM_IMG_FMT_I420, 256, 512, 1); + ASSERT_NE(raw, nullptr); + ASSERT_EQ(aom_codec_encode(&codec, raw, /*pts=*/0, /*duration=*/1, + /*flags=*/0), + AOM_CODEC_OK); + aom_img_free(raw); + + // Encode at 256x64 TL1, twice, set keyframe for both. + cfg.g_w = 256; + cfg.g_h = 64; + ASSERT_EQ(aom_codec_enc_config_set(&codec, &cfg), AOM_CODEC_OK); + layer_id.spatial_layer_id = 0; + layer_id.temporal_layer_id = 1; + ASSERT_EQ(aom_codec_control(&codec, AV1E_SET_SVC_LAYER_ID, &layer_id), + AOM_CODEC_OK); + raw = aom_img_alloc(nullptr, AOM_IMG_FMT_I420, 256, 64, 1); + ASSERT_NE(raw, nullptr); + ASSERT_EQ(aom_codec_encode(&codec, raw, /*pts=*/1, /*duration=*/1, + /*flags=*/AOM_EFLAG_FORCE_KF), + AOM_CODEC_OK); + ASSERT_EQ(aom_codec_encode(&codec, raw, /*pts=*/2, /*duration=*/1, + /*flags=*/0), + AOM_CODEC_OK); + aom_img_free(raw); + + // Encode TL0 back at original resolution 256x512. + cfg.g_w = 256; + cfg.g_h = 512; + ASSERT_EQ(aom_codec_enc_config_set(&codec, &cfg), AOM_CODEC_OK); + layer_id.spatial_layer_id = 0; + layer_id.temporal_layer_id = 0; + ASSERT_EQ(aom_codec_control(&codec, AV1E_SET_SVC_LAYER_ID, &layer_id), + AOM_CODEC_OK); + raw = aom_img_alloc(nullptr, AOM_IMG_FMT_I420, 256, 512, 1); + ASSERT_NE(raw, nullptr); + ASSERT_EQ(aom_codec_encode(&codec, raw, /*pts=*/3, /*duration=*/1, + /*flags=*/0), + AOM_CODEC_OK); + aom_img_free(raw); + + ASSERT_EQ(aom_codec_destroy(&codec), AOM_CODEC_OK); +} + } // namespace