Merge "[security] Make frro iteration thread-safe" into tm-qpr-dev am: ed1e6ae4cb

Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/base/+/20112003

Change-Id: Iae36edf343ee4fb8719cef072a750261d5c9311c
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
This commit is contained in:
TreeHugger Robot
2022-10-04 18:36:47 +00:00
committed by Automerger Merge Worker
4 changed files with 43 additions and 20 deletions

View File

@@ -23,6 +23,7 @@
#include <cstring> #include <cstring>
#include <filesystem> #include <filesystem>
#include <fstream> #include <fstream>
#include <limits>
#include <memory> #include <memory>
#include <ostream> #include <ostream>
#include <string> #include <string>
@@ -295,26 +296,42 @@ Status Idmap2Service::createFabricatedOverlay(
return ok(); return ok();
} }
Status Idmap2Service::acquireFabricatedOverlayIterator() { Status Idmap2Service::acquireFabricatedOverlayIterator(int32_t* _aidl_return) {
std::lock_guard l(frro_iter_mutex_);
if (frro_iter_.has_value()) { if (frro_iter_.has_value()) {
LOG(WARNING) << "active ffro iterator was not previously released"; LOG(WARNING) << "active ffro iterator was not previously released";
} }
frro_iter_ = std::filesystem::directory_iterator(kIdmapCacheDir); 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(); return ok();
} }
Status Idmap2Service::releaseFabricatedOverlayIterator() { Status Idmap2Service::releaseFabricatedOverlayIterator(int32_t iteratorId) {
std::lock_guard l(frro_iter_mutex_);
if (!frro_iter_.has_value()) { if (!frro_iter_.has_value()) {
LOG(WARNING) << "no active ffro iterator to release"; 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(); return ok();
} }
Status Idmap2Service::nextFabricatedOverlayInfos( Status Idmap2Service::nextFabricatedOverlayInfos(int32_t iteratorId,
std::vector<os::FabricatedOverlayInfo>* _aidl_return) { std::vector<os::FabricatedOverlayInfo>* _aidl_return) {
std::lock_guard l(frro_iter_mutex_);
constexpr size_t kMaxEntryCount = 100; constexpr size_t kMaxEntryCount = 100;
if (!frro_iter_.has_value()) { if (!frro_iter_.has_value()) {
return error("no active frro iterator"); 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; size_t count = 0;
@@ -322,22 +339,22 @@ Status Idmap2Service::nextFabricatedOverlayInfos(
auto entry_iter_end = end(*frro_iter_); auto entry_iter_end = end(*frro_iter_);
for (; entry_iter != entry_iter_end && count < kMaxEntryCount; ++entry_iter) { for (; entry_iter != entry_iter_end && count < kMaxEntryCount; ++entry_iter) {
auto& entry = *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; continue;
} }
const auto overlay = FabricatedOverlayContainer::FromPath(entry.path()); const auto overlay = FabricatedOverlayContainer::FromPath(entry.path().native());
if (!overlay) { if (!overlay) {
LOG(WARNING) << "Failed to open '" << entry.path() << "': " << overlay.GetErrorMessage(); LOG(WARNING) << "Failed to open '" << entry.path() << "': " << overlay.GetErrorMessage();
continue; continue;
} }
const auto info = (*overlay)->GetManifestInfo(); auto info = (*overlay)->GetManifestInfo();
os::FabricatedOverlayInfo out_info; os::FabricatedOverlayInfo out_info;
out_info.packageName = info.package_name; out_info.packageName = std::move(info.package_name);
out_info.overlayName = info.name; out_info.overlayName = std::move(info.name);
out_info.targetPackageName = info.target_package; out_info.targetPackageName = std::move(info.target_package);
out_info.targetOverlayable = info.target_name; out_info.targetOverlayable = std::move(info.target_name);
out_info.path = entry.path(); out_info.path = entry.path();
_aidl_return->emplace_back(std::move(out_info)); _aidl_return->emplace_back(std::move(out_info));
count++; count++;

View File

@@ -26,7 +26,10 @@
#include <filesystem> #include <filesystem>
#include <memory> #include <memory>
#include <mutex>
#include <optional>
#include <string> #include <string>
#include <variant>
#include <vector> #include <vector>
namespace android::os { namespace android::os {
@@ -60,11 +63,11 @@ class Idmap2Service : public BinderService<Idmap2Service>, public BnIdmap2 {
binder::Status deleteFabricatedOverlay(const std::string& overlay_path, binder::Status deleteFabricatedOverlay(const std::string& overlay_path,
bool* _aidl_return) override; 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; std::vector<os::FabricatedOverlayInfo>* _aidl_return) override;
binder::Status dumpIdmap(const std::string& overlay_path, std::string* _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. // be able to be recalculated if idmap2 dies and restarts.
std::unique_ptr<idmap2::TargetResourceContainer> framework_apk_cache_; std::unique_ptr<idmap2::TargetResourceContainer> framework_apk_cache_;
int32_t frro_iter_id_ = 0;
std::optional<std::filesystem::directory_iterator> frro_iter_; std::optional<std::filesystem::directory_iterator> frro_iter_;
std::mutex frro_iter_mutex_;
template <typename T> template <typename T>
using MaybeUniquePtr = std::variant<std::unique_ptr<T>, T*>; using MaybeUniquePtr = std::variant<std::unique_ptr<T>, T*>;

View File

@@ -41,9 +41,9 @@ interface IIdmap2 {
@nullable FabricatedOverlayInfo createFabricatedOverlay(in FabricatedOverlayInternal overlay); @nullable FabricatedOverlayInfo createFabricatedOverlay(in FabricatedOverlayInternal overlay);
boolean deleteFabricatedOverlay(@utf8InCpp String path); boolean deleteFabricatedOverlay(@utf8InCpp String path);
void acquireFabricatedOverlayIterator(); int acquireFabricatedOverlayIterator();
void releaseFabricatedOverlayIterator(); void releaseFabricatedOverlayIterator(int iteratorId);
List<FabricatedOverlayInfo> nextFabricatedOverlayInfos(); List<FabricatedOverlayInfo> nextFabricatedOverlayInfos(int iteratorId);
@utf8InCpp String dumpIdmap(@utf8InCpp String overlayApkPath); @utf8InCpp String dumpIdmap(@utf8InCpp String overlayApkPath);
} }

View File

@@ -217,6 +217,7 @@ class IdmapDaemon {
synchronized List<FabricatedOverlayInfo> getFabricatedOverlayInfos() { synchronized List<FabricatedOverlayInfo> getFabricatedOverlayInfos() {
final ArrayList<FabricatedOverlayInfo> allInfos = new ArrayList<>(); final ArrayList<FabricatedOverlayInfo> allInfos = new ArrayList<>();
Connection c = null; Connection c = null;
int iteratorId = -1;
try { try {
c = connect(); c = connect();
final IIdmap2 service = c.getIdmap2(); final IIdmap2 service = c.getIdmap2();
@@ -225,9 +226,9 @@ class IdmapDaemon {
return Collections.emptyList(); return Collections.emptyList();
} }
service.acquireFabricatedOverlayIterator(); iteratorId = service.acquireFabricatedOverlayIterator();
List<FabricatedOverlayInfo> infos; List<FabricatedOverlayInfo> infos;
while (!(infos = service.nextFabricatedOverlayInfos()).isEmpty()) { while (!(infos = service.nextFabricatedOverlayInfos(iteratorId)).isEmpty()) {
allInfos.addAll(infos); allInfos.addAll(infos);
} }
return allInfos; return allInfos;
@@ -235,8 +236,8 @@ class IdmapDaemon {
Slog.wtf(TAG, "failed to get all fabricated overlays", e); Slog.wtf(TAG, "failed to get all fabricated overlays", e);
} finally { } finally {
try { try {
if (c.getIdmap2() != null) { if (c.getIdmap2() != null && iteratorId != -1) {
c.getIdmap2().releaseFabricatedOverlayIterator(); c.getIdmap2().releaseFabricatedOverlayIterator(iteratorId);
} }
} catch (RemoteException e) { } catch (RemoteException e) {
// ignore // ignore