From ffecbbf5d0602147d644c94ee9a8f25e6c25733d Mon Sep 17 00:00:00 2001 From: ivoc Date: Fri, 16 Dec 2016 05:51:49 -0800 Subject: [PATCH] Fix for integer overflow in NetEq. BUG=chromium:668736 Review-Url: https://codereview.webrtc.org/2571483002 Cr-Commit-Position: refs/heads/master@{#15654} --- .../acm2/audio_coding_module_unittest.cc | 40 +++++++++---------- webrtc/modules/audio_coding/neteq/expand.cc | 20 +++++++--- .../audio_coding/neteq/neteq_unittest.cc | 8 ++-- 3 files changed, 38 insertions(+), 30 deletions(-) diff --git a/webrtc/modules/audio_coding/acm2/audio_coding_module_unittest.cc b/webrtc/modules/audio_coding/acm2/audio_coding_module_unittest.cc index 863dcd1177..be5e2c56dd 100644 --- a/webrtc/modules/audio_coding/acm2/audio_coding_module_unittest.cc +++ b/webrtc/modules/audio_coding/acm2/audio_coding_module_unittest.cc @@ -977,31 +977,31 @@ class AcmReceiverBitExactnessOldApi : public ::testing::Test { #if (defined(WEBRTC_CODEC_ISAC) || defined(WEBRTC_CODEC_ISACFX)) && \ defined(WEBRTC_CODEC_ILBC) && defined(WEBRTC_CODEC_G722) TEST_F(AcmReceiverBitExactnessOldApi, 8kHzOutput) { - Run(8000, PlatformChecksum("dce4890259e9ded50f472455aa470a6f", - "1c4ada78b12612147f3026920f8dcc14", - "d804791edf2d00be2bc31c81a47368d4", - "b2611f7323ab1209d5056399d0babbf5")); + Run(8000, PlatformChecksum("25cda36a1b967e75c0eb580924247681", + "bbfe6a07f8bca872b5370885825ee061", + "d5b9ae44d03dbd7c921dd9c228e03cc5", + "4d851d1f2e4b8a2f1727fac8fba4b1e1")); } TEST_F(AcmReceiverBitExactnessOldApi, 16kHzOutput) { - Run(16000, PlatformChecksum("27356bddffaa42b5c841b49aa3a070c5", - "5667d1872fc351244092ae995e5a5b32", - "53f5dc8088148479ca112c4c6d0e91cb", - "4061a876d64d6cec5a38450acf4f245d")); + Run(16000, PlatformChecksum("9c7b6f586c4b9d6d0195372660991353", + "1ab45baa674e681ec394e0d3824d8605", + "dd4e7f2521b5f47c0016b12f06c08695", + "5401b64b6dbe7f090f846e89b0d858ce")); } TEST_F(AcmReceiverBitExactnessOldApi, 32kHzOutput) { - Run(32000, PlatformChecksum("eb326547e83292305423b0a7ea57fea1", - "be7fc3140e6b5188c2e5fae0a394543b", - "eab9a0bff17320d6457d04f4c56563c6", - "b60241ef0bac4a75f66eead04e71bb12")); + Run(32000, PlatformChecksum("599b9484ca89615641ebd767cccb149f", + "9f7d51569647eff38026dd815d43ca91", + "78d00d2a3f8f307fc3835ca588a18f3a", + "d335eebc72f4d087aa397a9cf8f4967b")); } TEST_F(AcmReceiverBitExactnessOldApi, 48kHzOutput) { - Run(48000, PlatformChecksum("7eb79ea39b68472a5b04cf9a56e49cda", - "f8cdd6e018688b2fff25c9b865bebdbb", - "2d18f0f06e7e2fc63b74d06e3c58067f", - "81c3e4d24ebec23ca48f42fbaec4aba0")); + Run(48000, PlatformChecksum("5d3b4357c9044264bb4a601b6548bd55", + "8607778183d7ad02b8ce37eeeba4f37c", + "fd71398d336b88cbd4fb5002846e91c6", + "8ce7e0e1c381d920ee7b57751b257de8")); } TEST_F(AcmReceiverBitExactnessOldApi, 48kHzOutputExternalDecoder) { @@ -1077,10 +1077,10 @@ TEST_F(AcmReceiverBitExactnessOldApi, 48kHzOutputExternalDecoder) { rtc::scoped_refptr> factory( new rtc::RefCountedObject); - Run(48000, PlatformChecksum("7eb79ea39b68472a5b04cf9a56e49cda", - "f8cdd6e018688b2fff25c9b865bebdbb", - "2d18f0f06e7e2fc63b74d06e3c58067f", - "81c3e4d24ebec23ca48f42fbaec4aba0"), + Run(48000, PlatformChecksum("5d3b4357c9044264bb4a601b6548bd55", + "8607778183d7ad02b8ce37eeeba4f37c", + "fd71398d336b88cbd4fb5002846e91c6", + "8ce7e0e1c381d920ee7b57751b257de8"), factory, [](AudioCodingModule* acm) { acm->RegisterReceiveCodec(0, {"MockPCMu", 8000, 1}); }); diff --git a/webrtc/modules/audio_coding/neteq/expand.cc b/webrtc/modules/audio_coding/neteq/expand.cc index ffe4370e4e..da870d7706 100644 --- a/webrtc/modules/audio_coding/neteq/expand.cc +++ b/webrtc/modules/audio_coding/neteq/expand.cc @@ -675,12 +675,20 @@ void Expand::AnalyzeSignal(int16_t* random_vector) { parameters.ar_filter, kUnvoicedLpcOrder + 1, 128); - int16_t unvoiced_prescale; - if (WebRtcSpl_MaxAbsValueW16(unvoiced_vector, 128) > 4000) { - unvoiced_prescale = 4; - } else { - unvoiced_prescale = 0; - } + const int unvoiced_max_abs = [&] { + const int16_t max_abs = WebRtcSpl_MaxAbsValueW16(unvoiced_vector, 128); + // Since WebRtcSpl_MaxAbsValueW16 returns 2^15 - 1 when the input contains + // -2^15, we have to conservatively bump the return value by 1 + // if it is 2^15 - 1. + return max_abs == WEBRTC_SPL_WORD16_MAX ? max_abs + 1 : max_abs; + }(); + // Pick the smallest n such that 2^n > unvoiced_max_abs; then the maximum + // value of the dot product is less than 2^7 * 2^(2*n) = 2^(2*n + 7), so to + // prevent overflows we want 2n + 7 <= 31, which means we should shift by + // 2n + 7 - 31 bits, if this value is greater than zero. + int unvoiced_prescale = + std::max(0, 2 * WebRtcSpl_GetSizeInBits(unvoiced_max_abs) - 24); + int32_t unvoiced_energy = WebRtcSpl_DotProductWithScale(unvoiced_vector, unvoiced_vector, 128, diff --git a/webrtc/modules/audio_coding/neteq/neteq_unittest.cc b/webrtc/modules/audio_coding/neteq/neteq_unittest.cc index b958e27a64..ecefc45db3 100644 --- a/webrtc/modules/audio_coding/neteq/neteq_unittest.cc +++ b/webrtc/modules/audio_coding/neteq/neteq_unittest.cc @@ -442,10 +442,10 @@ TEST_F(NetEqDecodingTest, MAYBE_TestBitExactness) { webrtc::test::ResourcePath("audio_coding/neteq_universal_new", "rtp"); const std::string output_checksum = PlatformChecksum( - "acd33f5c73625c1529c412ad59b5565132826f1b", - "1a2e82a0410421c1d1d3eb0615334db5e2c63784", - "acd33f5c73625c1529c412ad59b5565132826f1b", - "52797b781758a1d2303140b80b9c5030c9093d6b"); + "5a8184bc60c0d7dddb50af8966360675476a8d8b", + "be982d2c5685dd1ca4ea5d352283df50e8e5b46d", + "5a8184bc60c0d7dddb50af8966360675476a8d8b", + "c86aec95439748f4949de95b50c94be291118615"); const std::string network_stats_checksum = PlatformChecksum( "f59b3dfdb9b1b8bbb61abedd7c8cf3fc47c21f5f",