Merge "[security] Make frro iteration thread-safe" into tm-qpr-dev (ed1e6ae4) · Commits · e / os / android_frameworks_base

cmds/idmap2/idmap2d/Idmap2Service.cpp

+27 −10

Original line number	Diff line number	Diff line
		@@ -23,6 +23,7 @@
		#include <cstring>
		#include <filesystem>
		#include <fstream>
		#include <limits>
		#include <memory>
		#include <ostream>
		#include <string>
		@@ -295,26 +296,42 @@ Status Idmap2Service::createFabricatedOverlay(
		return ok();
		}

		Status Idmap2Service::acquireFabricatedOverlayIterator() {
		Status Idmap2Service::acquireFabricatedOverlayIterator(int32_t* _aidl_return) {
		std::lock_guard l(frro_iter_mutex_);
		if (frro_iter_.has_value()) {
		LOG(WARNING) << "active ffro iterator was not previously released";
		}
		frro_iter_ = std::filesystem::directory_iterator(kIdmapCacheDir);
		if (frro_iter_id_ == std::numeric_limits<int32_t>::max()) {
		frro_iter_id_ = 0;
		} else {
		++frro_iter_id_;
		}
		*_aidl_return = frro_iter_id_;
		return ok();
		}

		Status Idmap2Service::releaseFabricatedOverlayIterator() {
		Status Idmap2Service::releaseFabricatedOverlayIterator(int32_t iteratorId) {
		std::lock_guard l(frro_iter_mutex_);
		if (!frro_iter_.has_value()) {
		LOG(WARNING) << "no active ffro iterator to release";
		} else if (frro_iter_id_ != iteratorId) {
		LOG(WARNING) << "incorrect iterator id in a call to release";
		} else {
		frro_iter_.reset();
		}
		return ok();
		}

		Status Idmap2Service::nextFabricatedOverlayInfos(
		Status Idmap2Service::nextFabricatedOverlayInfos(int32_t iteratorId,
		std::vector<os::FabricatedOverlayInfo>* _aidl_return) {
		std::lock_guard l(frro_iter_mutex_);

		constexpr size_t kMaxEntryCount = 100;
		if (!frro_iter_.has_value()) {
		return error("no active frro iterator");
		} else if (frro_iter_id_ != iteratorId) {
		return error("incorrect iterator id in a call to next");
		}

		size_t count = 0;
		@@ -322,22 +339,22 @@ Status Idmap2Service::nextFabricatedOverlayInfos(
		auto entry_iter_end = end(*frro_iter_);
		for (; entry_iter != entry_iter_end && count < kMaxEntryCount; ++entry_iter) {
		auto& entry = *entry_iter;
		if (!entry.is_regular_file() \|\| !android::IsFabricatedOverlay(entry.path())) {
		if (!entry.is_regular_file() \|\| !android::IsFabricatedOverlay(entry.path().native())) {
		continue;
		}

		const auto overlay = FabricatedOverlayContainer::FromPath(entry.path());
		const auto overlay = FabricatedOverlayContainer::FromPath(entry.path().native());
		if (!overlay) {
		LOG(WARNING) << "Failed to open '" << entry.path() << "': " << overlay.GetErrorMessage();
		continue;
		}

		const auto info = (*overlay)->GetManifestInfo();
		auto info = (*overlay)->GetManifestInfo();
		os::FabricatedOverlayInfo out_info;
		out_info.packageName = info.package_name;
		out_info.overlayName = info.name;
		out_info.targetPackageName = info.target_package;
		out_info.targetOverlayable = info.target_name;
		out_info.packageName = std::move(info.package_name);
		out_info.overlayName = std::move(info.name);
		out_info.targetPackageName = std::move(info.target_package);
		out_info.targetOverlayable = std::move(info.target_name);
		out_info.path = entry.path();
		_aidl_return->emplace_back(std::move(out_info));
		count++;

cmds/idmap2/idmap2d/Idmap2Service.h

+8 −3

Original line number	Diff line number	Diff line
		@@ -26,7 +26,10 @@

		#include <filesystem>
		#include <memory>
		#include <mutex>
		#include <optional>
		#include <string>
		#include <variant>
		#include <vector>

		namespace android::os {
		@@ -60,11 +63,11 @@ class Idmap2Service : public BinderService<Idmap2Service>, public BnIdmap2 {
		binder::Status deleteFabricatedOverlay(const std::string& overlay_path,
		bool* _aidl_return) override;

		binder::Status acquireFabricatedOverlayIterator() override;
		binder::Status acquireFabricatedOverlayIterator(int32_t* _aidl_return) override;

		binder::Status releaseFabricatedOverlayIterator() override;
		binder::Status releaseFabricatedOverlayIterator(int32_t iteratorId) override;

		binder::Status nextFabricatedOverlayInfos(
		binder::Status nextFabricatedOverlayInfos(int32_t iteratorId,
		std::vector<os::FabricatedOverlayInfo>* _aidl_return) override;

		binder::Status dumpIdmap(const std::string& overlay_path, std::string* _aidl_return) override;
		@@ -74,7 +77,9 @@ class Idmap2Service : public BinderService<Idmap2Service>, public BnIdmap2 {
		// be able to be recalculated if idmap2 dies and restarts.
		std::unique_ptr<idmap2::TargetResourceContainer> framework_apk_cache_;

		int32_t frro_iter_id_ = 0;
		std::optional<std::filesystem::directory_iterator> frro_iter_;
		std::mutex frro_iter_mutex_;

		template <typename T>
		using MaybeUniquePtr = std::variant<std::unique_ptr<T>, T*>;

cmds/idmap2/idmap2d/aidl/services/android/os/IIdmap2.aidl

+3 −3

Original line number	Diff line number	Diff line
		@@ -41,9 +41,9 @@ interface IIdmap2 {
		@nullable FabricatedOverlayInfo createFabricatedOverlay(in FabricatedOverlayInternal overlay);
		boolean deleteFabricatedOverlay(@utf8InCpp String path);

		void acquireFabricatedOverlayIterator();
		void releaseFabricatedOverlayIterator();
		List<FabricatedOverlayInfo> nextFabricatedOverlayInfos();
		int acquireFabricatedOverlayIterator();
		void releaseFabricatedOverlayIterator(int iteratorId);
		List<FabricatedOverlayInfo> nextFabricatedOverlayInfos(int iteratorId);

		@utf8InCpp String dumpIdmap(@utf8InCpp String overlayApkPath);
		}

services/core/java/com/android/server/om/IdmapDaemon.java

+5 −4

Original line number	Diff line number	Diff line
		@@ -217,6 +217,7 @@ class IdmapDaemon {
		synchronized List<FabricatedOverlayInfo> getFabricatedOverlayInfos() {
		final ArrayList<FabricatedOverlayInfo> allInfos = new ArrayList<>();
		Connection c = null;
		int iteratorId = -1;
		try {
		c = connect();
		final IIdmap2 service = c.getIdmap2();
		@@ -225,9 +226,9 @@ class IdmapDaemon {
		return Collections.emptyList();
		}

		service.acquireFabricatedOverlayIterator();
		iteratorId = service.acquireFabricatedOverlayIterator();
		List<FabricatedOverlayInfo> infos;
		while (!(infos = service.nextFabricatedOverlayInfos()).isEmpty()) {
		while (!(infos = service.nextFabricatedOverlayInfos(iteratorId)).isEmpty()) {
		allInfos.addAll(infos);
		}
		return allInfos;
		@@ -235,8 +236,8 @@ class IdmapDaemon {
		Slog.wtf(TAG, "failed to get all fabricated overlays", e);
		} finally {
		try {
		if (c.getIdmap2() != null) {
		c.getIdmap2().releaseFabricatedOverlayIterator();
		if (c.getIdmap2() != null && iteratorId != -1) {
		c.getIdmap2().releaseFabricatedOverlayIterator(iteratorId);
		}
		} catch (RemoteException e) {
		// ignore