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

Issue 622435 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jul 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 0
Type: Bug

Blocking:
issue 616183



Sign in to add a comment

a % b == static_cast<T>(0) in checks.h

Project Member Reported by ClusterFuzz, Jun 22 2016

Issue description

Detailed report: https://cluster-fuzz.appspot.com/testcase?key=6198277347475456

Fuzzer: phoglund_webrtc_peerconnection
Job Type: linux_cfi_chrome
Platform Id: linux

Crash Type: CHECK failure
Crash Address: 
Crash State:
  a % b == static_cast<T>(0) in checks.h
  rtc::FatalMessage::~FatalMessage
  rtc::CheckedDivExact<>
  webrtc::AudioCodingModuleImpl::Encode
  

Unminimized Testcase: https://cluster-fuzz.appspot.com/download/AMIfv95HxXiM0Eku2avav01GwZSzoTkMmR8iwjJlIofhkbLLMcQDVI5fq2LxL8hZPm7FVLZecg4tvJMWaF70fcuKJfS3SsDN18ciqpjWv42Ssovj7TJiX32V59bhgoKXPOHvJDlcIMaWPmAFP32vMDv_yDbheJdn6WOd3fcEaT6LaKzP21f6JUo?testcase_id=6198277347475456


Additional requirements: Requires HTTP

Filer: inferno

See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information.
 
Project Member

Comment 1 by ClusterFuzz, Jun 22 2016

Detailed report: https://cluster-fuzz.appspot.com/testcase?key=4504027371143168

Fuzzer: phoglund_webrtc_peerconnection
Job Type: linux_cfi_chrome
Platform Id: linux

Crash Type: CHECK failure
Crash Address: 
Crash State:
  a % b == static_cast<T>(0) in checks.h
  [1:2:0622/NUMBER:ERROR:video_frame.cc(383)]
  [1:2:0622/NUMBER:ERROR:video_frame.cc(383)]
  [1:2:0622/NUMBER:ERROR:video_frame.cc(383)]
  

Unminimized Testcase: https://cluster-fuzz.appspot.com/download/AMIfv95WhZNkztzVm-OTTROtmudUZ1bdev__D93f2zpANEFjLYuRFsu8ksLXy0IkXZwBORFjk-jATw2jIz2W7PHv8fdqecVv0rWp25LEFIIpzi9n2jlsvD11rUqH_QoY967t19PFtj2XpzJg5spZSG58YTUya_bV7W4i7cWApEnMOJB20Om6wpE?testcase_id=4504027371143168


Additional requirements: Requires HTTP

Filer: inferno

See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information.
Project Member

Comment 2 by ClusterFuzz, Jun 22 2016

Detailed report: https://cluster-fuzz.appspot.com/testcase?key=4659862273523712

Fuzzer: phoglund_webrtc_peerconnection
Job Type: linux_cfi_chrome
Platform Id: linux

Crash Type: CHECK failure
Crash Address: 
Crash State:
  a % b == static_cast<T>(0) in checks.h
  [1:10:0621/NUMBER:ERROR:datachannel.cc(567)]
  [1:10:0621/NUMBER:ERROR:datachannel.cc(567)]
  [1:10:0621/NUMBER:ERROR:datachannel.cc(567)]
  

Unminimized Testcase: https://cluster-fuzz.appspot.com/download/AMIfv97JtHDfrryecGDbH5KMdCYx4SPfAp4GCUGbDyw4YSpn-LKbP4YWiGS3yXYf5RU1en05h474zyUBMEOkU3Fdv_95ikXR5Gmzdv5hrSu_AqDvSZhiGNbzORkqIQB68UcJ3qvtaTl5_BQZ80niEh8CPSJGUFNy2HahJrKhMsY8hj5U3HxrsUI?testcase_id=4659862273523712


Additional requirements: Requires HTTP

Filer: inferno

See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information.
Project Member

Comment 3 by ClusterFuzz, Jun 22 2016

Detailed report: https://cluster-fuzz.appspot.com/testcase?key=4659862273523712

Fuzzer: phoglund_webrtc_peerconnection
Job Type: linux_cfi_chrome
Platform Id: linux

Crash Type: CHECK failure
Crash Address: 
Crash State:
  a % b == static_cast<T>(0) in checks.h
  [1:10:0621/NUMBER:ERROR:datachannel.cc(567)]
  [1:10:0621/NUMBER:ERROR:datachannel.cc(567)]
  [1:10:0621/NUMBER:ERROR:datachannel.cc(567)]
  

Unminimized Testcase: https://cluster-fuzz.appspot.com/download/AMIfv97JtHDfrryecGDbH5KMdCYx4SPfAp4GCUGbDyw4YSpn-LKbP4YWiGS3yXYf5RU1en05h474zyUBMEOkU3Fdv_95ikXR5Gmzdv5hrSu_AqDvSZhiGNbzORkqIQB68UcJ3qvtaTl5_BQZ80niEh8CPSJGUFNy2HahJrKhMsY8hj5U3HxrsUI?testcase_id=4659862273523712


Additional requirements: Requires HTTP

Filer: inferno

See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information.
Blocking: 616183
Cc: mbarbe...@chromium.org
Components: Blink>WebRTC
Labels: -Pri-1 Pri-0
Owner: phoglund@chromium.org
Status: Assigned (was: Available)
Patrick, can you please help with an owner for this soon. This is crashing on CF a lot and also creating noise since errors are bleeding into stack frame and confuses crash state.

e.g. lines like
#12 0x7f8272aa304d [1:2:0622/122933:ERROR:video_frame.cc(383)] WrapVideoFrame Invalid config.format:PIXEL_FORMAT_I420 storage_type:SHMEM coded_size:320x240 visible_rect:0,0 0x0 natural_size:0x1

Also, can you decrease the logging in webrtc for fuzzers.
Cc: katrielc@chromium.org phoglund@chromium.org hlundin@chromium.org
Owner: kwiberg@chromium.org
kwiberg: you look like a good owner from the blame info. Looks like a checked div fails, so the fuzzer appears able to force you into a state where the encoder stack state or time stamps are inconsistent?
Also re #4: Elaborate on decreasing logging. What logging are you thinking of? I don't see any logging in https://cluster-fuzz.appspot.com/testcase?key=6198277347475456 for instance.
phoglund@: I think inferno@ is referring to the printouts that are intermixed with the stack trace, making it hard to read for humans and probably confusing scripts even more. The link you gave is the best-looking of them, with all the logging first and all the stacktrace afterwards (mostly); the others are much more of a mess.
The fuzzed page appears to be relatively nice. It passes these constraints:

getUserMedia({'audio': {'optional': [{'googTypingNoiseDetection': 'false'}, {'googNoiseSuppression': 'false'}, {'googHighpassFilter': 'false'}, {'echoCancellation': 'false'}, {'googAudioMirroring': 'true'}, {'googAutoGainControl': 'false'}]}, 'fake': 'true'},

There's no reloading, mutations or abrupt restarts. SDP transform functions look like this:

  var gTransformOfferSdp = function(arg) { return arg; };
  var gTransformAnswerSdp = function(sdp) { return forceRandomVideoCodec(forceRandomAudioCodec(sdp)); };

We can't see the SDP in the test, but it could choose any of the available audio codecs here. You can try to go for a local repro to figure out which one (or ones) it chooses.
Cc: ossu@chromium.org
Owner: hlundin@chromium.org
hlundin: please find a new owner for this issue since kwiberg is going on vacation.
Project Member

Comment 10 by ClusterFuzz, Jun 24 2016

ClusterFuzz has detected this testcase as flaky and is unable to reproduce it in the original crash revision. Skipping fixed testing check and marking it as potentially fixed.

Detailed report: https://cluster-fuzz.appspot.com/testcase?key=6198277347475456

Fuzzer: phoglund_webrtc_peerconnection
Job Type: linux_cfi_chrome
Platform Id: linux

Crash Type: CHECK failure
Crash Address: 
Crash State:
  a % b == static_cast<T>(0) in checks.h
  rtc::FatalMessage::~FatalMessage
  rtc::CheckedDivExact<>
  webrtc::AudioCodingModuleImpl::Encode
  

Unminimized Testcase: https://cluster-fuzz.appspot.com/download/AMIfv95HxXiM0Eku2avav01GwZSzoTkMmR8iwjJlIofhkbLLMcQDVI5fq2LxL8hZPm7FVLZecg4tvJMWaF70fcuKJfS3SsDN18ciqpjWv42Ssovj7TJiX32V59bhgoKXPOHvJDlcIMaWPmAFP32vMDv_yDbheJdn6WOd3fcEaT6LaKzP21f6JUo?testcase_id=6198277347475456


Additional requirements: Requires HTTP

See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information.

If you suspect that the result above is incorrect, try re-doing that job on the test case report page.
Ok, #10 means maybe this isn't a problem after all. Abhishek, is this still hitting on CF?
Cc: infe...@chromium.org
(Abhishek, see #11)

Also, Abhishek/Martin, can you elaborate on which logging I should reduce from the WebRTC fuzzer, that you asked for in #4. This report in this bug doesn't contain any logging as far I can see.

Comment 13 by aarya@google.com, Jun 27 2016

It is still hitting, though not as frequently.

I meant frame confusion like
"#1 0x7f8274f6c0d5 [1:2:0622/122932:ERROR:video_frame.cc(383)] WrapVideoFrame Invalid config.format:PIXEL_FORMAT_I420 storage_type:SHMEM coded_size:320x240"

in https://cluster-fuzz.appspot.com/testcase?key=4504027371143168
Ok, filed https://bugs.chromium.org/p/chromium/issues/detail?id=623598 and https://bugs.chromium.org/p/chromium/issues/detail?id=623601. Do you know what level the fuzzer logs at by default?
Cc: grunell@chromium.org pbos@chromium.org
+grunell,pbos for context for the log spam bugs I just filed.

Comment 16 by pbos@chromium.org, Jun 27 2016

For libFuzzer we've turned off all webrtc logging. katrielc@ did that. The logspam is in Chromium code though?

Sounds like a pretty bogus format though: config.format:PIXEL_FORMAT_I420 storage_type:SHMEM coded_size:320x240 visible_rect:0,0 0x0 natural_size:0x1

Not sure at what level this input should be disallowed though?

Comment 17 by pbos@chromium.org, Jun 27 2016

Are these fuzzers running with Chromium logging at an appropriate level? If DFATAL should never happen, is a bug uncovered? Or should we never log DFATAL?
Project Member

Comment 18 by ClusterFuzz, Jun 28 2016

ClusterFuzz has detected this testcase as flaky and is unable to reproduce it in the original crash revision. Skipping fixed testing check and marking it as potentially fixed.

Detailed report: https://cluster-fuzz.appspot.com/testcase?key=4504027371143168

Fuzzer: phoglund_webrtc_peerconnection
Job Type: linux_cfi_chrome
Platform Id: linux

Crash Type: CHECK failure
Crash Address: 
Crash State:
  a % b == static_cast<T>(0) in checks.h
  [1:2:0622/NUMBER:ERROR:video_frame.cc(383)]
  [1:2:0622/NUMBER:ERROR:video_frame.cc(383)]
  [1:2:0622/NUMBER:ERROR:video_frame.cc(383)]
  

Unminimized Testcase: https://cluster-fuzz.appspot.com/download/AMIfv95WhZNkztzVm-OTTROtmudUZ1bdev__D93f2zpANEFjLYuRFsu8ksLXy0IkXZwBORFjk-jATw2jIz2W7PHv8fdqecVv0rWp25LEFIIpzi9n2jlsvD11rUqH_QoY967t19PFtj2XpzJg5spZSG58YTUya_bV7W4i7cWApEnMOJB20Om6wpE?testcase_id=4504027371143168


Additional requirements: Requires HTTP

See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information.

If you suspect that the result above is incorrect, try re-doing that job on the test case report page.
Owner: ossu@chromium.org
ossu@: can you take a look at this? The interesting bit for us is the OP.

Comment 20 by ossu@chromium.org, Jul 4 2016

Alright. I'll try to dig into it!

Comment 21 by ossu@chromium.org, Jul 4 2016

I've been trying to reproduce it locally using the instructions in the doc, but do not seem to be able to. I've altered the config to run 1000 iterations and am repeating that endlessly but all I'm getting is "No crash found". It's not clear to me if I'm just re-iterating the same test thousands of times, or if it is fuzzing with new values making re-running the test at all useful.

I guess if it's a test case that should be reproducible, re-running it should use the same values. Anyone?

Comment 22 by ossu@chromium.org, Jul 4 2016

I take it all back (for now). It seems I might've messed up the command line (relative paths to things). I'll keep investigating.

Comment 23 by ossu@chromium.org, Jul 4 2016

First, two observations:
1. There are two CheckedDivExact in that AudioCodingModule::Encode, one nested in the call to the other. The inner one uses only values from within the code base and essentially returns a 1 or a 2. It can only return a 2 if the codec is G722. The outer call will not fail with this message if the inner call returns 1 (since division by 1 is always exact). 

2. When trying to reproduce this locally, I'm not getting AudioCodingModuleImpl::Encode getting called at all - i.e. no audio channel gets set up. This probably means negotiation most often fails, or at least seldom includes audio.

So it seems we're getting set up with a G722 audio channel that's being fed an odd number of samples. Now, there's a check in AudioCodingModuleImpl::Add10MsDataInternal that checks that the number of samples fed into the AudioCodingModule is exactly 10ms, i.e. equals to the sample rate / 100. After this, the code does resampling and up/downmixing as required.

My conjecture at this point is that we're running the G722 audio codec while recording at 44100 Hz; since 44100 / 100 = 441, which is odd (or at least not even :))

I'll try to replicate these circumstances locally.

Comment 24 by ossu@chromium.org, Jul 5 2016

Yeah, that doesn't seem to be it. There are several places where the audio is resampled to the encoder's sample rate before arriving at Encode(). Forcing G722 and 44100 outside of the ACM still worked fine, with the audio at 16KHz once it reaches Encode.

Back to the drawing board...


Comment 25 by ossu@chromium.org, Jul 5 2016

I've been able to reproduce it locally. The bug happens when changing codecs, in this case from opus at 48KHz to G722 at 16KHz - input sample rate is 44100Hz.
While initially running opus, timestamps are scaled properly, so although they are input as samples at 44.1KHz, they reflect proper 48KHz internally in the ACM. When changing codecs, it seems the last input timestamp is used as a basis for all future timestamps, without scaling, causing two wrong things to happen:
1. The input timestamp moves backwards, in this case from 66240 to 61299. This goes by unnoticed and it probably shouldn't.
2. If this happens after an odd number of 10ms audio blocks, the timestamp is odd, which is caught by the ACM, but only if using G722.

Since the latter depends on a certain number of blocks input, it's very sensitive to timing. After adding a bit of logging, I was unable to reproduce it while outputting to stdout.

I'll look at fixing both issues - the clock should never move backwards.

Comment 26 by ossu@chromium.org, Jul 5 2016

I've put up a CL with a fix here: https://codereview.webrtc.org/2119393002/
I've tested it locally and it runs fine.

As a side-note, I found I was unable to reproduce this locally using the clusterfuzz scripts for local reproduction (i.e. running in some sort of sandbox, I guess). I was, however, able to reproduce it by running the command line in the report manually, although only while I also had the reproduction script running - the webserver it sets up needed to be running. Maybe there's something that can be done to allow the reproduction script to run in the user's local environment as well.
Excellent and thorough work, ossu! Get reviewed and ship it.

Ok, what you did in #26 is what I would expect the repro script to do.

Comment 28 by ossu@chromium.org, Jul 7 2016

I think it runs the command line fine, but maybe in some different environment - or I failed at some part of the setup. When running "manually" I got output through my headphones when the test was running, and got an (albeit black) chrome window to pop up. When just running through the script, I got neither.

Comment 29 by ossu@chromium.org, Jul 8 2016

Status: Fixed (was: Assigned)
I was expecting a message to get generated here automatically, but it mightn't due to the change being in a different project.

The fix landed in WebRTC here>: https://crrev.com/63fb95a68d70cd7036db65c3378e91cb77faa44d

I expect it'll get into chromium with the next roll.
Impressive debugging, ossu@!

It looks like there's decent libfuzzer coverage of each audio decoder specifically, but iiuc this bug is caused by switching between different codecs. Is there an obvious target to hit with a libfuzzer?

I'm thinking something like generating fuzzed AudioFrames and feeding them into the decoder, but I don't really know how the decoding pathway works.

Comment 31 by ossu@chromium.org, Jul 8 2016

Hmm. I guess that should be possible. For testing this exact thing, I expect we could keep a (small) set of valid audio frames for all codecs that get slightly modified to make them reusable (i.e. updating timestamps and sequence numbers manually). We'd then generate a list of entries for which codecs to pick in each instance and run the fuzzer on that. Should be able to generate many strange combinations without too much effort.
Project Member

Comment 32 by sheriffbot@chromium.org, Nov 22 2016

Labels: -Restrict-View-EditIssue
Removing EditIssue view restrictions from ClusterFuzz filed bugs. If you believe that this issue should still be restricted, please reapply the label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Sign in to add a comment