diff --git a/fs_mgr/libfiemap/image_manager.cpp b/fs_mgr/libfiemap/image_manager.cpp index 3ee742f5e..f32e0ebfc 100644 --- a/fs_mgr/libfiemap/image_manager.cpp +++ b/fs_mgr/libfiemap/image_manager.cpp @@ -136,13 +136,13 @@ bool ImageManager::BackingImageExists(const std::string& name) { return !!FindPartition(*metadata.get(), name); } -static bool IsTestDir(const std::string& path) { - return android::base::StartsWith(path, kTestImageMetadataDir) || - android::base::StartsWith(path, kOtaTestImageMetadataDir); +bool ImageManager::MetadataDirIsTest() const { + return IsSubdir(metadata_dir_, kTestImageMetadataDir) || + IsSubdir(metadata_dir_, kOtaTestImageMetadataDir); } -static bool IsUnreliablePinningAllowed(const std::string& path) { - return android::base::StartsWith(path, "/data/gsi/dsu/") || IsTestDir(path); +bool ImageManager::IsUnreliablePinningAllowed() const { + return IsSubdir(data_dir_, "/data/gsi/dsu/") || MetadataDirIsTest(); } FiemapStatus ImageManager::CreateBackingImage( @@ -159,7 +159,7 @@ FiemapStatus ImageManager::CreateBackingImage( if (!FilesystemHasReliablePinning(data_path, &reliable_pinning)) { return FiemapStatus::Error(); } - if (!reliable_pinning && !IsUnreliablePinningAllowed(data_path)) { + if (!reliable_pinning && !IsUnreliablePinningAllowed()) { // For historical reasons, we allow unreliable pinning for certain use // cases (DSUs, testing) because the ultimate use case is either // developer-oriented or ephemeral (the intent is to boot immediately @@ -178,7 +178,7 @@ FiemapStatus ImageManager::CreateBackingImage( // if device-mapper is stacked in some complex way not supported by // FiemapWriter. auto device_path = GetDevicePathForFile(fw.get()); - if (android::base::StartsWith(device_path, "/dev/block/dm-") && !IsTestDir(metadata_dir_)) { + if (android::base::StartsWith(device_path, "/dev/block/dm-") && !MetadataDirIsTest()) { LOG(ERROR) << "Cannot persist images against device-mapper device: " << device_path; fw = {}; diff --git a/fs_mgr/libfiemap/image_test.cpp b/fs_mgr/libfiemap/image_test.cpp index 66633916a..6d0975150 100644 --- a/fs_mgr/libfiemap/image_test.cpp +++ b/fs_mgr/libfiemap/image_test.cpp @@ -34,10 +34,13 @@ #include #include +#include "utility.h" + using namespace android::dm; using namespace std::literals; using android::base::unique_fd; using android::fiemap::ImageManager; +using android::fiemap::IsSubdir; using android::fs_mgr::BlockDeviceInfo; using android::fs_mgr::PartitionOpener; using android::fs_mgr::WaitForFile; @@ -131,6 +134,51 @@ TEST_F(NativeTest, GetMappedImageDevice) { ASSERT_TRUE(manager_->UnmapImageDevice(base_name_)); } +namespace { + +struct IsSubdirTestParam { + std::string child; + std::string parent; + bool result; +}; + +class IsSubdirTest : public ::testing::TestWithParam {}; + +TEST_P(IsSubdirTest, Test) { + const auto& param = GetParam(); + EXPECT_EQ(param.result, IsSubdir(param.child, param.parent)) + << "IsSubdir(child=\"" << param.child << "\", parent=\"" << param.parent + << "\") != " << (param.result ? "true" : "false"); +} + +std::vector IsSubdirTestValues() { + // clang-format off + std::vector base_cases{ + {"/foo/bar", "/foo", true}, + {"/foo/bar/baz", "/foo", true}, + {"/foo", "/foo", true}, + {"/foo", "/", true}, + {"/", "/", true}, + {"/foo", "/foo/bar", false}, + {"/foo", "/bar", false}, + {"/foo-bar", "/foo", false}, + {"/", "/foo", false}, + }; + // clang-format on + std::vector ret; + for (const auto& e : base_cases) { + ret.push_back(e); + ret.push_back({e.child + "/", e.parent, e.result}); + ret.push_back({e.child, e.parent + "/", e.result}); + ret.push_back({e.child + "/", e.parent + "/", e.result}); + } + return ret; +} + +INSTANTIATE_TEST_SUITE_P(IsSubdirTest, IsSubdirTest, ::testing::ValuesIn(IsSubdirTestValues())); + +} // namespace + bool Mkdir(const std::string& path) { if (mkdir(path.c_str(), 0700) && errno != EEXIST) { std::cerr << "Could not mkdir " << path << ": " << strerror(errno) << std::endl; diff --git a/fs_mgr/libfiemap/include/libfiemap/image_manager.h b/fs_mgr/libfiemap/include/libfiemap/image_manager.h index 60b98dce0..50f4f33c2 100644 --- a/fs_mgr/libfiemap/include/libfiemap/image_manager.h +++ b/fs_mgr/libfiemap/include/libfiemap/image_manager.h @@ -176,6 +176,8 @@ class ImageManager final : public IImageManager { bool MapWithDmLinear(const IPartitionOpener& opener, const std::string& name, const std::chrono::milliseconds& timeout_ms, std::string* path); bool UnmapImageDevice(const std::string& name, bool force); + bool IsUnreliablePinningAllowed() const; + bool MetadataDirIsTest() const; ImageManager(const ImageManager&) = delete; ImageManager& operator=(const ImageManager&) = delete; diff --git a/fs_mgr/libfiemap/utility.cpp b/fs_mgr/libfiemap/utility.cpp index c1898556e..54cf1836b 100644 --- a/fs_mgr/libfiemap/utility.cpp +++ b/fs_mgr/libfiemap/utility.cpp @@ -167,5 +167,30 @@ bool FilesystemHasReliablePinning(const std::string& file, bool* supported) { return F2fsPinBeforeAllocate(fd, supported); } +bool IsSubdir(const std::string& child, const std::string& parent) { + // Precondition: both are absolute paths. + CHECK(android::base::StartsWith(child, "/")) << "Not an absolute path: " << child; + CHECK(android::base::StartsWith(parent, "/")) << "Not an absolute path: " << parent; + + // Remove extraneous "/" at the end. + std::string_view child_sv = child; + while (child_sv != "/" && android::base::ConsumeSuffix(&child_sv, "/")) + ; + + std::string_view parent_sv = parent; + while (parent_sv != "/" && android::base::ConsumeSuffix(&parent_sv, "/")) + ; + + // IsSubdir(anything, "/") => true + if (parent_sv == "/") return true; + + // IsSubdir("/foo", "/foo") => true + if (parent_sv == child_sv) return true; + + // IsSubdir("/foo/bar", "/foo") => true + // IsSubdir("/foo-bar", "/foo") => false + return android::base::StartsWith(child_sv, std::string(parent_sv) + "/"); +} + } // namespace fiemap } // namespace android diff --git a/fs_mgr/libfiemap/utility.h b/fs_mgr/libfiemap/utility.h index 4c0bc2b1c..aa40f79d1 100644 --- a/fs_mgr/libfiemap/utility.h +++ b/fs_mgr/libfiemap/utility.h @@ -51,5 +51,9 @@ bool BlockDeviceToName(uint32_t major, uint32_t minor, std::string* bdev_name); // cases (such as snapshots or adb remount). bool FilesystemHasReliablePinning(const std::string& file, bool* supported); +// Crude implementation to check if |child| is a subdir of |parent|. +// Assume both are absolute paths. +bool IsSubdir(const std::string& child, const std::string& parent); + } // namespace fiemap } // namespace android