Fix use-after-free in SurfaceFlinger::doDump
SurfaceFlinger::doDump previously contained two layer traversals, one on
the main thread and one off the main thread. During concurrent dumpsys
commands, the layer traversals may race with each other, which causes
shared ownership of the underlying storage of a SortedVector containing
the z-ordered list of layers. Because the implementation of
SortedVector's STL iterators assumes that the underlying storage may be
edited, this can cause the storage to be copied whenever SortedVector::begin
and SortedVector::end are called, which means that SortedVector::begin()
+ SortedVector::size() == SortedVector::end() is not always true, which
causes invalid iteration.
In general, this use-after-free can happen as long as the off-main
thread traversal exists in doDump(), because the traversal can run in
parallel with any workload on the main thread that executes a layer
traversal. So, this patch moves the traversal for dumping out the list
of composition layers into the main thread.
A future patch could explore either fixing SortedVector to fix judicious
iterator invalidation, or building LayerVector on top of std::set, but
either option is an invasive data structure change.
Bug: 237291506
Test: Test script that calls dumpsys SurfaceFlinger on many threads
Change-Id: I0748396519c924dc1b84113d44259f22d0d7ebd6
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index fdaacf0..3c92d56 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -1044,7 +1044,8 @@
/*
* Debugging & dumpsys
*/
- void dumpAllLocked(const DumpArgs& args, std::string& result) const REQUIRES(mStateLock);
+ void dumpAllLocked(const DumpArgs& args, const std::string& compositionLayers,
+ std::string& result) const REQUIRES(mStateLock);
void dumpHwcLayersMinidumpLocked(std::string& result) const REQUIRES(mStateLock);
void appendSfConfigString(std::string& result) const;