From 42e79138c0bfb0ac31bed572c3cedaab18e6e6f8 Mon Sep 17 00:00:00 2001 From: Daniele Di Proietto Date: Fri, 10 Nov 2023 12:42:31 +0000 Subject: FenceTime: Fix undefined behavior std::unordered_map::erase() invalidates iterators to the erased elements. Using erase() inside a range based for loop can lead to undefined behavior, because the loop holds the same iterator that are invalidated. Fix the problem by using an interator directly and incrementng only when it makes sense. This was found by surfaceflinger_frametracer_fuzzer running with asan (not hwasan!). TESTED=only fuzzer Bug: 307601836 Change-Id: Id99feaec21300dbd55d35acba67801b2483dd144 --- libs/ui/FenceTime.cpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'libs') diff --git a/libs/ui/FenceTime.cpp b/libs/ui/FenceTime.cpp index 538c1d2a42..4246c40f64 100644 --- a/libs/ui/FenceTime.cpp +++ b/libs/ui/FenceTime.cpp @@ -363,9 +363,9 @@ void FenceToFenceTimeMap::signalAllForTest( } void FenceToFenceTimeMap::garbageCollectLocked() { - for (auto& it : mMap) { + for (auto it = mMap.begin(); it != mMap.end();) { // Erase all expired weak pointers from the vector. - auto& vect = it.second; + auto& vect = it->second; vect.erase( std::remove_if(vect.begin(), vect.end(), [](const std::weak_ptr& ft) { @@ -375,7 +375,9 @@ void FenceToFenceTimeMap::garbageCollectLocked() { // Also erase the map entry if the vector is now empty. if (vect.empty()) { - mMap.erase(it.first); + it = mMap.erase(it); + } else { + it++; } } } -- cgit v1.2.3-59-g8ed1b