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

Issue 754500 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Aug 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Mac
Pri: 1
Type: Bug



Sign in to add a comment

Timeout in media_pipeline_integration_fuzzer

Project Member Reported by ClusterFuzz, Aug 10 2017

Issue description

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

Fuzzer: libFuzzer_media_pipeline_integration_fuzzer
Job Type: libfuzzer_chrome_ubsan
Platform Id: linux

Crash Type: Timeout (exceeds 25 secs)
Crash Address: 
Crash State:
  media_pipeline_integration_fuzzer
  
Sanitizer: undefined (UBSAN)

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

Issue filed automatically.

See https://chromium.googlesource.com/chromium/src/+/master/testing/libfuzzer/reproducing.md for more information.

Note: This crash might not be reproducible with the provided testcase. That said, for the past 14 days we've been seeing this crash frequently. If you are unable to reproduce this, please try a speculative fix based on the crash stacktrace in the report. The fix can be verified by looking at the crash statistics in the report, a day after the fix is deployed. If the fix resolved the issue, please close the bug by marking as Fixed.


 
Project Member

Comment 1 by ClusterFuzz, Aug 11 2017

Labels: OS-Mac
Cc: msrchandra@chromium.org
Labels: Test-Predator-Wrong
Owner: wolenetz@chromium.org
Status: Assigned (was: Untriaged)
Predator and CL could not provide any possible suspects.
Using Code Search for the file, "media_pipeline_integration_fuzzer", assigning to the concern owner.

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

@wolenetz -- Could you please look into the issue, kindly re-assign if this is not related to your changes.
Thank You.
@#2: I don't think that commit did this at all:
In the detailed report's statistics section, if you expand the period from 7 to 45 days, it shows this crash has been going on long before that commit.
The recent drop in crash rate ~Aug 1-Aug 10 was temporary, and is explained by  bug 751011  causing overall high crash rates.
Looking further at the last 1000 days, it looks like the crash really started on Feb 2, 2017.

It looks like PipelineIntegrationTestBase::WaitUntilEndedOrError() times out waiting.

Is this truly P1?
I also don't see any obvious culprits in:
git log --oneline --after=2017-01-31 --before=2017-02-03 -- media
Neither does there appear to be any FFmpeg DEPS change in that git history range.

Cc: dalecur...@chromium.org
Components: Internals>Media
This timeout repros on a fuzzer build from before Feb 3 (Jan 20).

Perf profiling points to repeating AudioRendererImpl::Render(0, 0, *Bus with frames()==960), and |play_delay| is really huge (> 5e12 seconds), so it's just rendering (memsetting) silence, resulting in timeout even in clockless path.

I have definite P1 work to do on other items, so must set this aside for now.

cc+=dalecurtis@ as FYI

Hmm, this is working as intended, there's a video track and then an audio track which starts way later. Not sure what the best way to workaround for the fuzzer is. 

Comment 7 by mmoroz@chromium.org, Aug 11 2017

Cc: mmoroz@chromium.org kcc@chromium.org
Can we do a quick check and reject such inputs in the fuzz target: https://cs.chromium.org/chromium/src/media/test/pipeline_integration_fuzzertest.cc ?

If no, we might consider skipping such things in fuzzing builds via "#ifdef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION" (e.g. https://cs.chromium.org/search/?q=FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION&sq=package:chromium&type=cs), but it is not a very good solution...
We could plumb |first_packet_timestamp_| out of AudioRendererImpl into PipelineIntegrationTest and if |first_packet_timestamp_| >> 0 skip the test.
@#8 that sounds reasonable. I'll take a stab at doing that.
Hmm. Plumbing is looking icky (AudioRendererImpl (also AudioRenderer)) <- ((RendererWrapper <- PipelineImpl) or perhaps a copy of audio_renderer* saved in PipelineIntegrationTestBase::CreateRenderer(...)) <- PipelineIntegrationTestBase.  

All that, plus the audio renderer won't immediately know first_packet_timestamp until after the demuxer has done some parsing and renderers have some info.

A stab at this won't be quick; I'm laying this aside for now.
Seems reasonable, FTR I was suggesting keeping a copy of the audion_renderer* so you didn't need to plumb through any non-test layers.
Labels: M-62
Status: Started (was: Assigned)
Patch in review @ https://chromium-review.googlesource.com/c/614318
Project Member

Comment 13 by bugdroid1@chromium.org, Aug 14 2017

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

commit 219c750098c408d24ef26241a0d598493172ed07
Author: Matt Wolenetz <wolenetz@chromium.org>
Date: Mon Aug 14 22:42:59 2017

Prevent one type of media_pipeline_integration_fuzzer timeout

Aborts media_pipeline_integration_fuzzer early if there is a very large
audio renderer |first_timestamp_packet_| following
PipelineIntegrationTestBase::StartInternal, preventing timeout of fuzzer
while rendering that large amount of initial silence (even in clockless
pipeline case).

Note, there may be other causes of timeouts remaining.

BUG= 754500 

Change-Id: I1d273da47600e9aad3950fa09d18824054bbcf9a
Reviewed-on: https://chromium-review.googlesource.com/614318
Commit-Queue: Matthew Wolenetz <wolenetz@chromium.org>
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#494220}
[modify] https://crrev.com/219c750098c408d24ef26241a0d598493172ed07/media/renderers/audio_renderer_impl.h
[modify] https://crrev.com/219c750098c408d24ef26241a0d598493172ed07/media/test/pipeline_integration_fuzzertest.cc
[modify] https://crrev.com/219c750098c408d24ef26241a0d598493172ed07/media/test/pipeline_integration_test_base.cc
[modify] https://crrev.com/219c750098c408d24ef26241a0d598493172ed07/media/test/pipeline_integration_test_base.h

Status: Fixed (was: Started)
#13 should fix this particular timeout. There may be others.
CF should verify this is fixed soon...
 Issue 754959  has been merged into this issue.
Project Member

Comment 16 by ClusterFuzz, Aug 16 2017

ClusterFuzz has detected this issue as fixed in range 494161:494246.

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

Fuzzer: libFuzzer_media_pipeline_integration_fuzzer
Job Type: libfuzzer_chrome_ubsan
Platform Id: linux

Crash Type: Timeout (exceeds 25 secs)
Crash Address: 
Crash State:
  NULL
Sanitizer: undefined (UBSAN)

Fixed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_ubsan&range=494161:494246

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

See https://chromium.googlesource.com/chromium/src/+/master/testing/libfuzzer/reproducing.md 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 17 by ClusterFuzz, Aug 16 2017

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

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

Comment 18 by bugdroid1@chromium.org, Aug 18 2017

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

commit b40b2ee2aae490a7c9daff046e130901dafc8fa6
Author: Matt Wolenetz <wolenetz@chromium.org>
Date: Fri Aug 18 01:38:18 2017

Fix test-only UAF/uninitialized value/bad-cast from invalid vptr in fuzzer timeout mitigation

Fixes PipelineIntegrationTestBase's UAF and bad-cast from invalid vptr
of |audio_renderer_| and similar use of uninitialized
audio_renderer_->first_packet_timestamp_ value in the
media_pipeline_integration_fuzzer timeout mitigation code from 219c7500.

Instead of waiting for (thread-trampolined) buffering state change
notifications to inspect the audio renderer's |first_packet_timestamp_|
(risking race of its destruction), this change adds a test-only callback
for directly telling the test about any positive |play_delay| in
AudioRendererImpl::Render(). (Using play_delay instead of
|first_packet_timestamp_| will also help a future MSE pipeline fuzzer
abort early after seeking hits a large audio play delay.) This new
test-only callback also must trampoline from audio thread to the
fuzzer's main thread, but it precludes the need to then ask the
(potentially already destructed) AudioRendererImpl for info.

BUG= 755501 , 755499 , 755619 , 754500 , 756412 

Change-Id: I9ab2987aa120c21a30463951c75e51838614d62f
Reviewed-on: https://chromium-review.googlesource.com/616064
Commit-Queue: Matthew Wolenetz <wolenetz@chromium.org>
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#495414}
[modify] https://crrev.com/b40b2ee2aae490a7c9daff046e130901dafc8fa6/media/renderers/audio_renderer_impl.cc
[modify] https://crrev.com/b40b2ee2aae490a7c9daff046e130901dafc8fa6/media/renderers/audio_renderer_impl.h
[modify] https://crrev.com/b40b2ee2aae490a7c9daff046e130901dafc8fa6/media/test/pipeline_integration_fuzzertest.cc
[modify] https://crrev.com/b40b2ee2aae490a7c9daff046e130901dafc8fa6/media/test/pipeline_integration_test_base.cc
[modify] https://crrev.com/b40b2ee2aae490a7c9daff046e130901dafc8fa6/media/test/pipeline_integration_test_base.h

Sign in to add a comment