diff options
author | 2023-02-22 10:40:30 -0800 | |
---|---|---|
committer | 2023-02-22 10:41:24 -0800 | |
commit | 277af61c83028863dfac4260ad24b3d579f3201b (patch) | |
tree | 243f5a56230e35159b9bd8b0ecf4a534dd79a8cf | |
parent | d6ea4df62db59dc1ebc1fd3b62146700640ef400 (diff) |
Fix out of bounds array access when deadlock detector finds exceptionally large cycles.
PiperOrigin-RevId: 511536497
Change-Id: If70a1c72ef5f7cbb4a80100c4edff459373a5d55
-rw-r--r-- | absl/synchronization/mutex.cc | 5 | ||||
-rw-r--r-- | absl/synchronization/mutex_test.cc | 19 |
2 files changed, 23 insertions, 1 deletions
diff --git a/absl/synchronization/mutex.cc b/absl/synchronization/mutex.cc index f6a8506c..a8911614 100644 --- a/absl/synchronization/mutex.cc +++ b/absl/synchronization/mutex.cc @@ -1403,7 +1403,7 @@ static GraphId DeadlockCheck(Mutex *mu) { ABSL_RAW_LOG(ERROR, "Cycle: "); int path_len = deadlock_graph->FindPath( mu_id, other_node_id, ABSL_ARRAYSIZE(b->path), b->path); - for (int j = 0; j != path_len; j++) { + for (int j = 0; j != path_len && j != ABSL_ARRAYSIZE(b->path); j++) { GraphId id = b->path[j]; Mutex *path_mu = static_cast<Mutex *>(deadlock_graph->Ptr(id)); if (path_mu == nullptr) continue; @@ -1416,6 +1416,9 @@ static GraphId DeadlockCheck(Mutex *mu) { symbolize); ABSL_RAW_LOG(ERROR, "%s", b->buf); } + if (path_len > static_cast<int>(ABSL_ARRAYSIZE(b->path))) { + ABSL_RAW_LOG(ERROR, "(long cycle; list truncated)"); + } if (synch_deadlock_detection.load(std::memory_order_acquire) == OnDeadlockCycle::kAbort) { deadlock_graph_mu.Unlock(); // avoid deadlock in fatal sighandler diff --git a/absl/synchronization/mutex_test.cc b/absl/synchronization/mutex_test.cc index 34751cb1..f76b1e8b 100644 --- a/absl/synchronization/mutex_test.cc +++ b/absl/synchronization/mutex_test.cc @@ -1131,6 +1131,25 @@ TEST(Mutex, DeadlockDetectorBazelWarning) { absl::SetMutexDeadlockDetectionMode(absl::OnDeadlockCycle::kAbort); } +TEST(Mutex, DeadlockDetectorLongCycle) { + absl::SetMutexDeadlockDetectionMode(absl::OnDeadlockCycle::kReport); + + // This test generates a warning if it passes, and crashes otherwise. + // Cause bazel to ignore the warning. + ScopedDisableBazelTestWarnings disable_bazel_test_warnings; + + // Check that we survive a deadlock with a lock cycle. + std::vector<absl::Mutex> mutex(100); + for (size_t i = 0; i != mutex.size(); i++) { + mutex[i].Lock(); + mutex[(i + 1) % mutex.size()].Lock(); + mutex[i].Unlock(); + mutex[(i + 1) % mutex.size()].Unlock(); + } + + absl::SetMutexDeadlockDetectionMode(absl::OnDeadlockCycle::kAbort); +} + // This test is tagged with NO_THREAD_SAFETY_ANALYSIS because the // annotation-based static thread-safety analysis is not currently // predicate-aware and cannot tell if the two for-loops that acquire and |