Lock-order-inversion in pthread_mutex_lock |
||||||||||
Issue descriptionDetailed report: https://clusterfuzz.com/testcase?key=5691647966052352 Fuzzer: lcamtuf_cross_fuzz Job Type: linux_tsan_chrome_mp Platform Id: linux Crash Type: Lock-order-inversion Crash Address: Crash State: pthread_mutex_lock base::internal::LockImpl::Lock midi::TaskService::RunTask Sanitizer: thread (TSAN) Regressed: https://clusterfuzz.com/revisions?job=linux_tsan_chrome_mp&range=479362:479374 Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=5691647966052352 Issue filed automatically. See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information.
,
Jun 15 2017
My CL that you reference isn't in the "regressed range". Probably better to randomly pick one of the CLs in that range that assign it against one 10 months old. Clusterfuzz output lists MIDI so assigning against this: c73f9dc095133071e0d3dd918a9135443aeb8032 Web MIDI: use midi::TaskService in MidiManagerAlsa This patch replaces existing MidiManagerAlsa implementation to support asynchronous method posts with midi::TaskService that should be used on all platforms now to support such functionalities. BUG=672793 Review-Url: https://codereview.chromium.org/2923163003 Cr-Commit-Position: refs/heads/master@{#479374}
,
Jun 16 2017
I feel this is a false alert of https://github.com/google/sanitizers/issues/488 because three locks are carefully managed, and taking order is clearly defined as |instance_lock_| -> |lock_| -> |(one of) thread_task_locks_|. I would check again next week.
,
Jun 23 2017
ClusterFuzz has detected this issue as fixed in range 481537:481544. Detailed report: https://clusterfuzz.com/testcase?key=5691647966052352 Fuzzer: lcamtuf_cross_fuzz Job Type: linux_tsan_chrome_mp Platform Id: linux Crash Type: Lock-order-inversion Crash Address: Crash State: pthread_mutex_lock base::internal::LockImpl::Lock midi::TaskService::RunTask Sanitizer: thread (TSAN) Regressed: https://clusterfuzz.com/revisions?job=linux_tsan_chrome_mp&range=479362:479374 Fixed: https://clusterfuzz.com/revisions?job=linux_tsan_chrome_mp&range=481537:481544 Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=5691647966052352 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.
,
Jun 23 2017
We haven't make any related change, and reported revision range seems not containing any relevant fix. Let me keep this open for a while.
,
Jun 23 2017
ClusterFuzz testcase 5691647966052352 is verified as fixed, so closing issue. If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.
,
Jun 23 2017
,
Jun 23 2017
In this report, M85140734437680512 can be identified as |thread_task_locks_| from callstacks because it was taken in TaskService::RunTask, and kept on holding in MidiManagerAlsa::EventLoop. On the other hand, M1311 is also obtained in TaskService::RunTask. So this could be instance_lock_ or lock_. But, they can not be hold when MidiManagerAlsa::EventLoop runs via TaskService::RunTask because the AutoLock is always out of scope at the point to run the task.
,
Jun 23 2017
,
Jun 23 2017
msrchandra@, can I have a chance to get advice from thread experts on this?
,
Jun 23 2017
I'll take ownership back. Probably, I could make it simple by using ReadWriteLock so that tsan becomes happy.
,
Jul 4 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c0c38f15e6afa44b2a3b9aa6b1e4de2da71a4cbc commit c0c38f15e6afa44b2a3b9aa6b1e4de2da71a4cbc Author: Takashi Toyoshima <toyoshim@chromium.org> Date: Tue Jul 04 17:05:09 2017 Web MIDI: use ReadWriteLock to make TaskService simple Use ReadWriteLock instead of std::vector<std::unique_ptr<base::Lock>> to remove complicated lock acquisition. This makes TaskService implementation much simpler. BUG= 733384 Change-Id: I3ac62974fa6b03cf992b47bb4b737d890e8a28b0 Reviewed-on: https://chromium-review.googlesource.com/544371 Commit-Queue: Takashi Toyoshima <toyoshim@chromium.org> Reviewed-by: Yutaka Hirano <yhirano@chromium.org> Cr-Commit-Position: refs/heads/master@{#484133} [modify] https://crrev.com/c0c38f15e6afa44b2a3b9aa6b1e4de2da71a4cbc/media/midi/task_service.cc [modify] https://crrev.com/c0c38f15e6afa44b2a3b9aa6b1e4de2da71a4cbc/media/midi/task_service.h [modify] https://crrev.com/c0c38f15e6afa44b2a3b9aa6b1e4de2da71a4cbc/media/midi/task_service_unittest.cc
,
Jul 5 2017
ClusterFuzz has detected this issue as fixed in range 481537:481544. Detailed report: https://clusterfuzz.com/testcase?key=5691647966052352 Fuzzer: lcamtuf_cross_fuzz Job Type: linux_tsan_chrome_mp Platform Id: linux Crash Type: Lock-order-inversion Crash Address: Crash State: pthread_mutex_lock base::internal::LockImpl::Lock midi::TaskService::RunTask Sanitizer: thread (TSAN) Regressed: https://clusterfuzz.com/revisions?job=linux_tsan_chrome_mp&range=479362:479374 Fixed: https://clusterfuzz.com/revisions?job=linux_tsan_chrome_mp&range=481537:481544 Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=5691647966052352 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.
,
Jul 5 2017
484025 is the last revision I can see the error in the fuzzer's timeline. I'd monitor it in another day, and will close if there is no error after 484133.
,
Jul 7 2017
,
Sep 18 2017
We have made a bunch of changes on ClusterFuzz side, so resetting ClusterFuzz-Wrong label. |
||||||||||
►
Sign in to add a comment |
||||||||||
Comment 1 by msrchandra@chromium.org
, Jun 15 2017Labels: Test-Predator-Wrong-CLs M-61
Owner: bcwh...@chromium.org
Status: Assigned (was: Untriaged)