New issue
Advanced search Search tips

Issue 681316 link

Starred by 3 users

Issue metadata

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



Sign in to add a comment

Lock-order-inversion in pthread_mutex_lock

Project Member Reported by ClusterFuzz, Jan 14 2017

Issue description

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

Fuzzer: inferno_twister_custom_bundle
Job Type: linux_tsan_chrome_mp
Platform Id: linux

Crash Type: Lock-order-inversion
Crash Address: 
Crash State:
  pthread_mutex_lock
  blink::BaseAudioContext::releaseFinishedSourceNodes
  blink::BaseAudioContext::handlePostRenderTasks
  
Sanitizer: thread (TSAN)

Regressed: https://cluster-fuzz.appspot.com/revisions?job=linux_tsan_chrome_mp&range=443594:443650

Minimized Testcase (0.33 Kb):
Download: https://cluster-fuzz.appspot.com/download/AMIfv94nsSlzQjJOMwbhRwbwc--fCEi9c64rSNLz8qIRsb1Mery2Fwhm3bP2eKBUchohNhHA0fygaAVUpy0dihs2Jv5khRyIW9sLUubjxO5aGkqxEpq2IkemgRp_EWATjx-gqU9moBfYuqdRyVfA0AU4e8_gyxQe1m5F9CXcJwI63Q2RszTRAxGb4Xjll-a-nKhuvML7xWDsLVP7qgkW1VdOrXCiHCou4Gr6p9UR4v3qKUbVTmo-Ero-8Jnp4yGZm7idt0C_ih3NjQ19E5AmbTo7IQ9d3LNxDMGDfeFncxzsLRdbWSxCuc0GSH3xmVE439rEbtkf2u0sd6j1H-7vXTSFb3YOfCG7jz1oiayd_HtIc_nMYtC-A8I?testcase_id=5344331763548160
<script>
var Context0= new AudioContext()
var BufferSource0=Context0.createBufferSource();
BufferSource0.start();
BufferSource0.buffer=function(){
	var length=86333;
	var Buffer=Context0.createBuffer(1,length,Context0.sampleRate);
	return Buffer;
}();
setTimeout('try{gc()}catch(e){}');
setTimeout("window.location.reload();", 2871);
</script>


Issue filed automatically.

See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information.
 
Cc: hongchan@chromium.org
Labels: Test-Predator-Wrong-CLs
Owner: haraken@chromium.org
Status: Assigned (was: Untriaged)
Find it and CL did not provide any possible suspect.
Using Code Search for the file, "BaseAudioContext.cpp" assigning to the concern owner who might be related.

@haraken -- Could you please look into the issue, kindly re-assign if this is not related to your changes.
Thank You.
Owner: rtoy@chromium.org
Along with the issue 681527, it seems like the new behavior shows up when the context is destroyed. However, I don't recall any change in shutting down the context has landed recently.

Comment 4 by rtoy@chromium.org, Jan 17 2017

Components: Blink>WebAudio
Not sure how this can happen.  The backtrace says releaseFinishedSourceNodes has acquired a lock.  But a quick check of the code shows that this function doesn't acquire any locks.  It's supposed to be running under the lock established by handlePostRenderTasks which calls releaseFinishedSourceNodes.


Comment 5 by rtoy@chromium.org, Jan 18 2017

Perhaps this is what is happening.  handlePostRenderTasks on the audio thread runs and grabs the lock, and calls releaseFinishedSourceNodes.  Some short time later, releaseFinishedSourceNodes which starts removeFinishedSourceNodes on the main thread.  In the mean time the audio thread has continued and gets to handlePostRenderTasks again, grabbing the lock.  removeFinishedSourceNodes happens to grabs the lock right after handlePostRenderTasks gets the lock.  

Perhaps the solution is for removeFinishedSourceNodes to use a trylock.  If it can't acquire it, do nothing.  We'll have to arrange for removeFinishedSourceNodes to clear m_finishedSourceNodes instead of releaseFinishedSourceNodes which does the clearing today.

Comment 6 by rtoy@chromium.org, Jan 20 2017

Status: Started (was: Assigned)
Project Member

Comment 7 by bugdroid1@chromium.org, Jan 23 2017

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

commit b82438a6b9cf9324f9d2b3780ea270b7bff62a93
Author: rtoy <rtoy@chromium.org>
Date: Mon Jan 23 20:28:34 2017

Don't run removeFinishedSourceNodes with the context lock.

releaseFinishedSourceNodes is run from handlePostRenderTask on the
audio thread which has the context lock.  releaseFinishedSourceNodes
can call removeFinishedSourceNodes which runs on the main thread which
will also get the context lock.  Depending on how fast things are,
this second attempt to get the lock may happen while
handlePostRenderTask still has the lock, causing a cycle.

Therefore, move the call to removeFinishedSourceNodes outside the
lock.  This is ok, since removeFinishedSourceNodes doesn't do anything
without acquiring the context lock.

BUG= 681316 
TEST=test case from the bug passes on a tsan build

Review-Url: https://codereview.chromium.org/2638413004
Cr-Commit-Position: refs/heads/master@{#445467}

[modify] https://crrev.com/b82438a6b9cf9324f9d2b3780ea270b7bff62a93/third_party/WebKit/Source/modules/webaudio/BaseAudioContext.cpp
[modify] https://crrev.com/b82438a6b9cf9324f9d2b3780ea270b7bff62a93/third_party/WebKit/Source/modules/webaudio/BaseAudioContext.h
[modify] https://crrev.com/b82438a6b9cf9324f9d2b3780ea270b7bff62a93/third_party/WebKit/Source/modules/webaudio/OfflineAudioContext.cpp

Project Member

Comment 8 by ClusterFuzz, Jan 24 2017

ClusterFuzz has detected this issue as fixed in range 445391:445491.

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

Fuzzer: inferno_twister_custom_bundle
Job Type: linux_tsan_chrome_mp
Platform Id: linux

Crash Type: Lock-order-inversion
Crash Address: 
Crash State:
  pthread_mutex_lock
  blink::BaseAudioContext::releaseFinishedSourceNodes
  blink::BaseAudioContext::handlePostRenderTasks
  
Sanitizer: thread (TSAN)

Regressed: https://cluster-fuzz.appspot.com/revisions?job=linux_tsan_chrome_mp&range=443594:443650
Fixed: https://cluster-fuzz.appspot.com/revisions?job=linux_tsan_chrome_mp&range=445391:445491

Minimized Testcase (0.33 Kb):
Download: https://cluster-fuzz.appspot.com/download/AMIfv94nsSlzQjJOMwbhRwbwc--fCEi9c64rSNLz8qIRsb1Mery2Fwhm3bP2eKBUchohNhHA0fygaAVUpy0dihs2Jv5khRyIW9sLUubjxO5aGkqxEpq2IkemgRp_EWATjx-gqU9moBfYuqdRyVfA0AU4e8_gyxQe1m5F9CXcJwI63Q2RszTRAxGb4Xjll-a-nKhuvML7xWDsLVP7qgkW1VdOrXCiHCou4Gr6p9UR4v3qKUbVTmo-Ero-8Jnp4yGZm7idt0C_ih3NjQ19E5AmbTo7IQ9d3LNxDMGDfeFncxzsLRdbWSxCuc0GSH3xmVE439rEbtkf2u0sd6j1H-7vXTSFb3YOfCG7jz1oiayd_HtIc_nMYtC-A8I?testcase_id=5344331763548160
<script>
var Context0= new AudioContext()
var BufferSource0=Context0.createBufferSource();
BufferSource0.start();
BufferSource0.buffer=function(){
	var length=86333;
	var Buffer=Context0.createBuffer(1,length,Context0.sampleRate);
	return Buffer;
}();
setTimeout('try{gc()}catch(e){}');
setTimeout("window.location.reload();", 2871);
</script>


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.
Project Member

Comment 9 by ClusterFuzz, Jan 24 2017

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

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

Sign in to add a comment