This issue is a security regression. If you are not able to fix this quickly, please revert the change that introduced it.

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

This issue is a security regression. If you are not able to fix this quickly, please revert the change that introduced it.

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

capn, perhaps your commit dc7759ccc3151a1aefefa9f86610e66f6fe9311c is related? Also, does this apply to all POSIX platforms or just Linux, do you think?

Thanks for notifying me. My change has indeed triggered this, but it's a symptom of a pre-existing condition which a revert won't fix. Fortunately it's only happening on teardown of the GPU process, and in practice SwiftShader only kicks in when using WebGL when the GPU is blacklisted. Only desktop Linux is affected. So in all it's not very significant.

> So in all it's not very significant.

It's important to distinguish between the severity of the bug and the likelihood of triggering it. In this case, the mitigating factors don't make the bug less severe when it does occur, so we should still treat it as important.

Can you suggest someone who would be able to fix this, or triage it further?

Understood. I didn't mean to imply this isn't important and/or shouldn't be fixed. I was just trying to assess the priority of this among the other things I'm working on.

I don't know anyone else who would be better suited to fix this than myself, or my colleague who will return from vacation on May 15.

Started working on this now. I was able to reproduce it easily (but only after opening the testcase from a local server).

It's a destruction order issue. I was able to avoid the TSAN error by not releasing the egl::Image's parentTexture in its destructor, but that would cause a leak. A proper fix will require releasing the parent texture after the Image's Surface base class destructor, which won't be possible without changing the inheritance to composition, or cleaning up the Surface before its actual destructor, or making the Surface responsible for releasing the parent texture. Evaluating the tradeoffs of these designs now...

The following revision refers to this bug:
  https://swiftshader.googlesource.com/SwiftShader.git/+/bf7a8145635e7dc6625596be127733ac7418cf21

commit bf7a8145635e7dc6625596be127733ac7418cf21
Author: Nicolas Capens <capn@google.com>
Date: Fri May 19 21:00:56 2017

Fix locking a destroyed mutex.

Surface should not lock the resource of a parent texture at destruction, because
it can already have been destroyed. For example when a texture's image was bound
as a render target, and the texture is deleted by the app, then the image holds
the last reference to the texture. When the render target image gets deleted, it
first releases its parent texture, and then the underlying surface gets
destroyed.

This is fixed by synchronizing, by locking and unlocking the (parent) resource,
earlier. The derived class is responsible for calling Surface::sync() before
releasing the parent resource.

 Bug chromium:716803 

Change-Id: Ifc3685dcf9e25e8419000af65d4bb7407f26bbcb
Reviewed-on: https://swiftshader-review.googlesource.com/9750
Reviewed-by: Nicolas Capens <capn@google.com>
Tested-by: Nicolas Capens <capn@google.com>

[modify] https://crrev.com/bf7a8145635e7dc6625596be127733ac7418cf21/src/Common/Resource.hpp
[modify] https://crrev.com/bf7a8145635e7dc6625596be127733ac7418cf21/src/D3D8/Direct3DCubeTexture8.cpp
[modify] https://crrev.com/bf7a8145635e7dc6625596be127733ac7418cf21/src/D3D8/Direct3DTexture8.cpp
[modify] https://crrev.com/bf7a8145635e7dc6625596be127733ac7418cf21/src/D3D8/Direct3DVolumeTexture8.cpp
[modify] https://crrev.com/bf7a8145635e7dc6625596be127733ac7418cf21/src/D3D9/Direct3DCubeTexture9.cpp
[modify] https://crrev.com/bf7a8145635e7dc6625596be127733ac7418cf21/src/D3D9/Direct3DTexture9.cpp
[modify] https://crrev.com/bf7a8145635e7dc6625596be127733ac7418cf21/src/D3D9/Direct3DVolumeTexture9.cpp
[modify] https://crrev.com/bf7a8145635e7dc6625596be127733ac7418cf21/src/OpenGL/common/Image.cpp
[modify] https://crrev.com/bf7a8145635e7dc6625596be127733ac7418cf21/src/OpenGL/common/Image.hpp
[modify] https://crrev.com/bf7a8145635e7dc6625596be127733ac7418cf21/src/OpenGL/libGL/Texture.cpp
[modify] https://crrev.com/bf7a8145635e7dc6625596be127733ac7418cf21/src/OpenGL/libGLES_CM/Texture.cpp
[modify] https://crrev.com/bf7a8145635e7dc6625596be127733ac7418cf21/src/OpenGL/libGLESv2/Texture.cpp
[modify] https://crrev.com/bf7a8145635e7dc6625596be127733ac7418cf21/src/Renderer/Surface.cpp
[modify] https://crrev.com/bf7a8145635e7dc6625596be127733ac7418cf21/src/Renderer/Surface.hpp

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

commit 38dae1d492a4f53e5d5928a6c8be4a0f08d6e083
Author: capn <capn@chromium.org>
Date: Fri May 19 23:58:35 2017

Roll SwiftShader 9ed48ba..bf7a814

https://swiftshader.googlesource.com/SwiftShader.git/+log/9ed48ba..bf7a814

BUG= 716803 

TBR=kbr@chromium.org

TEST=bots

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel

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

[modify] https://crrev.com/38dae1d492a4f53e5d5928a6c8be4a0f08d6e083/DEPS

ClusterFuzz has detected this issue as fixed in range 473261:473393.

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

Fuzzer: ochang_domfuzzer
Job Type: linux_tsan_chrome_mp
Platform Id: linux

Crash Type: Use of an invalid mutex
Crash Address: 
Crash State:
  pthread_mutex_unlock
  sw::Resource::unlock
  sw::Surface::~Surface
  
Sanitizer: thread (TSAN)

Regressed: https://clusterfuzz.com/revisions?job=linux_tsan_chrome_mp&range=468129:468154
Fixed: https://clusterfuzz.com/revisions?job=linux_tsan_chrome_mp&range=473261:473393

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


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.

ClusterFuzz testcase 4547308976078848 is verified as fixed, so closing issue.

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

Note that while this invalid pthread mutex issue has been fixed, M59 (currently in Beta) has a similar issue from before we switched to pthread mutexes. Windows is also affected by this underlying issue.

Merging this fix would carry some risk itself, and considering that this bug has existed for many years while SwiftShader was used as a component, I think we can leave things as-is and let the fix roll into M60. Let me know if the security risk is deemed significant and we should attempt to merge the fix to M59. Thanks.

This bug has been closed for more than 14 weeks. Removing security view restrictions.

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