Merge "Fix allowlist for unreliable pinning."

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) {
+bool ImageManager::MetadataDirIsTest() const {
-    return android::base::StartsWith(path, kTestImageMetadataDir) ||
+    return IsSubdir(metadata_dir_, kTestImageMetadataDir) ||
-           android::base::StartsWith(path, kOtaTestImageMetadataDir);
+           IsSubdir(metadata_dir_, kOtaTestImageMetadataDir);
 }
 
-static bool IsUnreliablePinningAllowed(const std::string& path) {
+bool ImageManager::IsUnreliablePinningAllowed() const {
-    return android::base::StartsWith(path, "/data/gsi/dsu/") || IsTestDir(path);
+    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 = {};

fs_mgr/libfiemap/image_test.cpp

@@ -34,10 +34,13 @@
 #include <libdm/dm.h>
 #include <libfiemap/image_manager.h>
 
+#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<IsSubdirTestParam> {};
+
+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<IsSubdirTestParam> IsSubdirTestValues() {
+    // clang-format off
+    std::vector<IsSubdirTestParam> 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<IsSubdirTestParam> 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;

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;

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

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