New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 760052 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Sep 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 2
Type: Bug



Sign in to add a comment

Integer-overflow in media::MovingAverage::AddSample

Project Member Reported by ClusterFuzz, Aug 29 2017

Issue description

Detailed report: https://clusterfuzz.com/testcase?key=6176984877236224

Fuzzer: ochang_search_index_mutator
Job Type: linux_ubsan_chrome
Platform Id: linux

Crash Type: Integer-overflow
Crash Address: 
Crash State:
  media::MovingAverage::AddSample
  media::VideoRendererAlgorithm::UpdateFrameStatistics
  media::VideoRendererAlgorithm::Render
  
Sanitizer: undefined (UBSAN)

Regressed: https://clusterfuzz.com/revisions?job=linux_ubsan_chrome&range=370022:370027

Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=6176984877236224

Issue filed automatically.

See https://github.com/google/clusterfuzz-tools for more information.
 
Cc: msrchandra@chromium.org
Labels: M-62 Test-Predator-Wrong-CLs
Owner: dalecur...@chromium.org
Status: Assigned (was: Untriaged)
Predator and CL could not provide any possible suspects.
Using Code Search for the file, "moving_average.cc" assigning to the concern owner.

Suspecting Commit#
https://chromium.googlesource.com/chromium/src/+/f7dc395ec1310a8d1f288fbff421ca15b94a8a54

@dalecurtis -- Could you please look into the issue, kindly re-assign if this is not related to your changes.
Thank You.
Thanks, not related to that change, but long standing issue. I'll see what we can do here.
Project Member

Comment 3 by bugdroid1@chromium.org, Sep 11 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/798ea62fe80cdf8a6b8d2f11e74391bd4fe3cdaf

commit 798ea62fe80cdf8a6b8d2f11e74391bd4fe3cdaf
Author: Dale Curtis <dalecurtis@chromium.org>
Date: Mon Sep 11 20:28:03 2017

Switch to double for sum of squares calculation with uint64_t.

ubsan found a test case which causes this to overflow, so switch to
using a double value in seconds for the deviation calculation. To avoid
cumulative errors, remove streaming calculations in favor of explicit
ones based on the current sample queue.

Given our window sizes (<32), removal of the streaming calculations
shouldn't have any large effect on performance. Looking through the
use cases we seem to call them about 1/frame. The perf bots will
holler if this materially an issue.

BUG= 760052 
TEST=ubsan passes

Change-Id: Ie1ba3490c260769de3b656666baa8652506a740f
Reviewed-on: https://chromium-review.googlesource.com/642508
Reviewed-by: Qiang Chen <qiangchen@chromium.org>
Reviewed-by: Dan Sanders <sandersd@chromium.org>
Commit-Queue: Dale Curtis <dalecurtis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#501021}
[modify] https://crrev.com/798ea62fe80cdf8a6b8d2f11e74391bd4fe3cdaf/media/base/moving_average.cc
[modify] https://crrev.com/798ea62fe80cdf8a6b8d2f11e74391bd4fe3cdaf/media/base/moving_average.h

Status: Fixed (was: Assigned)
Project Member

Comment 5 by ClusterFuzz, Sep 12 2017

ClusterFuzz has detected this issue as fixed in range 501012:501053.

Detailed report: https://clusterfuzz.com/testcase?key=6176984877236224

Fuzzer: ochang_search_index_mutator
Job Type: linux_ubsan_chrome
Platform Id: linux

Crash Type: Integer-overflow
Crash Address: 
Crash State:
  media::MovingAverage::AddSample
  media::VideoRendererAlgorithm::UpdateFrameStatistics
  media::VideoRendererAlgorithm::Render
  
Sanitizer: undefined (UBSAN)

Regressed: https://clusterfuzz.com/revisions?job=linux_ubsan_chrome&range=370022:370027
Fixed: https://clusterfuzz.com/revisions?job=linux_ubsan_chrome&range=501012:501053

Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=6176984877236224

See https://github.com/google/clusterfuzz-tools for more information.

If you suspect that the result above is incorrect, try re-doing that job on the test case report page.
Project Member

Comment 6 by ClusterFuzz, Sep 12 2017

Labels: ClusterFuzz-Verified
Status: Verified (was: Fixed)
ClusterFuzz testcase 6176984877236224 is verified as fixed, so closing issue as verified.

If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.

Sign in to add a comment