Merge "Fix use-after-free in SurfaceFlinger::doDump" (c71a81cd) · Commits · e / os / android_frameworks_native

services/surfaceflinger/SurfaceFlinger.cpp

+23 −14

Original line number	Original line	Diff line number	Diff line
	@@ -4935,6 +4935,25 @@ status_t SurfaceFlinger::doDump(int fd, const DumpArgs& args, bool asProto) {

	const auto flag = args.empty() ? ""s : std::string(String8(args[0]));		const auto flag = args.empty() ? ""s : std::string(String8(args[0]));

			// Traversal of drawing state must happen on the main thread.
			// Otherwise, SortedVector may have shared ownership during concurrent
			// traversals, which can result in use-after-frees.
			std::string compositionLayers;
			mScheduler
			->schedule([&] {
			StringAppendF(&compositionLayers, "Composition layers\n");
			mDrawingState.traverseInZOrder([&](Layer* layer) {
			auto* compositionState = layer->getCompositionState();
			if (!compositionState \|\| !compositionState->isVisible) return;

			android::base::StringAppendF(&compositionLayers, "* Layer %p (%s)\n", layer,
			layer->getDebugName() ? layer->getDebugName()
			: "<unknown>");
			compositionState->dump(compositionLayers);
			});
			})
			.get();

	bool dumpLayers = true;		bool dumpLayers = true;
	{		{
	TimedLock lock(mStateLock, s2ns(1), __func__);		TimedLock lock(mStateLock, s2ns(1), __func__);
	@@ -4947,7 +4966,7 @@ status_t SurfaceFlinger::doDump(int fd, const DumpArgs& args, bool asProto) {
	(it->second)(args, asProto, result);		(it->second)(args, asProto, result);
	dumpLayers = false;		dumpLayers = false;
	} else if (!asProto) {		} else if (!asProto) {
	dumpAllLocked(args, result);		dumpAllLocked(args, compositionLayers, result);
	}		}
	}		}

	@@ -5254,7 +5273,8 @@ void SurfaceFlinger::dumpHwcLayersMinidumpLocked(std::string& result) const {
	}		}
	}		}

	void SurfaceFlinger::dumpAllLocked(const DumpArgs& args, std::string& result) const {		void SurfaceFlinger::dumpAllLocked(const DumpArgs& args, const std::string& compositionLayers,
			std::string& result) const {
	const bool colorize = !args.empty() && args[0] == String16("--color");		const bool colorize = !args.empty() && args[0] == String16("--color");
	Colorizer colorizer(colorize);		Colorizer colorizer(colorize);

	@@ -5302,18 +5322,7 @@ void SurfaceFlinger::dumpAllLocked(const DumpArgs& args, std::string& result) co
	StringAppendF(&result, "Visible layers (count = %zu)\n", mNumLayers.load());		StringAppendF(&result, "Visible layers (count = %zu)\n", mNumLayers.load());
	colorizer.reset(result);		colorizer.reset(result);

	{		result.append(compositionLayers);
	StringAppendF(&result, "Composition layers\n");
	mDrawingState.traverseInZOrder([&](Layer* layer) {
	auto* compositionState = layer->getCompositionState();
	if (!compositionState \|\| !compositionState->isVisible) return;

	android::base::StringAppendF(&result, "* Layer %p (%s)\n", layer,
	layer->getDebugName() ? layer->getDebugName()
	: "<unknown>");
	compositionState->dump(result);
	});
	}

	colorizer.bold(result);		colorizer.bold(result);
	StringAppendF(&result, "Displays (%zu entries)\n", mDisplays.size());		StringAppendF(&result, "Displays (%zu entries)\n", mDisplays.size());

services/surfaceflinger/SurfaceFlinger.h

+2 −1

Original line number	Original line	Diff line number	Diff line
	@@ -1044,7 +1044,8 @@ private:
	/*		/*
	* Debugging & dumpsys		* 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 dumpHwcLayersMinidumpLocked(std::string& result) const REQUIRES(mStateLock);

	void appendSfConfigString(std::string& result) const;		void appendSfConfigString(std::string& result) const;