diff options
-rw-r--r-- | services/incremental/IncrementalService.cpp | 68 | ||||
-rw-r--r-- | services/incremental/IncrementalService.h | 7 | ||||
-rw-r--r-- | services/incremental/test/IncrementalServiceTest.cpp | 55 |
3 files changed, 73 insertions, 57 deletions
diff --git a/services/incremental/IncrementalService.cpp b/services/incremental/IncrementalService.cpp index e4a37dde7758..ac45d7695ab9 100644 --- a/services/incremental/IncrementalService.cpp +++ b/services/incremental/IncrementalService.cpp @@ -617,15 +617,7 @@ int IncrementalService::bind(StorageId storage, std::string_view source, std::st if (storageInfo == ifs->storages.end()) { return -EINVAL; } - std::string normSource; - if (path::isAbsolute(source)) { - normSource = path::normalize(source); - } else { - normSource = path::normalize(path::join(storageInfo->second.name, source)); - } - if (!path::startsWith(normSource, storageInfo->second.name)) { - return -EINVAL; - } + std::string normSource = normalizePathToStorage(ifs, storage, source); l.unlock(); std::unique_lock l2(mLock, std::defer_lock); return addBindMount(*ifs, storage, storageInfo->second.name, std::move(normSource), @@ -674,20 +666,29 @@ int IncrementalService::unbind(StorageId storage, std::string_view target) { return 0; } +std::string IncrementalService::normalizePathToStorage(const IncrementalService::IfsMountPtr ifs, + StorageId storage, std::string_view path) { + const auto storageInfo = ifs->storages.find(storage); + if (storageInfo == ifs->storages.end()) { + return {}; + } + std::string normPath; + if (path::isAbsolute(path)) { + normPath = path::normalize(path); + } else { + normPath = path::normalize(path::join(storageInfo->second.name, path)); + } + if (!path::startsWith(normPath, storageInfo->second.name)) { + return {}; + } + return normPath; +} + int IncrementalService::makeFile(StorageId storage, std::string_view path, int mode, FileId id, incfs::NewFileParams params) { if (auto ifs = getIfs(storage)) { - const auto storageInfo = ifs->storages.find(storage); - if (storageInfo == ifs->storages.end()) { - return -EINVAL; - } - std::string normPath; - if (path::isAbsolute(path)) { - normPath = path::normalize(path); - } else { - normPath = path::normalize(path::join(storageInfo->second.name, path)); - } - if (!path::startsWith(normPath, storageInfo->second.name)) { + std::string normPath = normalizePathToStorage(ifs, storage, path); + if (normPath.empty()) { return -EINVAL; } auto err = mIncFs->makeFile(ifs->control, normPath, mode, id, params); @@ -706,7 +707,11 @@ int IncrementalService::makeFile(StorageId storage, std::string_view path, int m int IncrementalService::makeDir(StorageId storageId, std::string_view path, int mode) { if (auto ifs = getIfs(storageId)) { - return mIncFs->makeDir(ifs->control, path, mode); + std::string normPath = normalizePathToStorage(ifs, storageId, path); + if (normPath.empty()) { + return -EINVAL; + } + return mIncFs->makeDir(ifs->control, normPath, mode); } return -EINVAL; } @@ -716,31 +721,40 @@ int IncrementalService::makeDirs(StorageId storageId, std::string_view path, int if (!ifs) { return -EINVAL; } - - auto err = mIncFs->makeDir(ifs->control, path, mode); + std::string normPath = normalizePathToStorage(ifs, storageId, path); + if (normPath.empty()) { + return -EINVAL; + } + auto err = mIncFs->makeDir(ifs->control, normPath, mode); if (err == -EEXIST) { return 0; } else if (err != -ENOENT) { return err; } - if (auto err = makeDirs(storageId, path::dirname(path), mode)) { + if (auto err = makeDirs(storageId, path::dirname(normPath), mode)) { return err; } - return mIncFs->makeDir(ifs->control, path, mode); + return mIncFs->makeDir(ifs->control, normPath, mode); } int IncrementalService::link(StorageId sourceStorageId, std::string_view oldPath, StorageId destStorageId, std::string_view newPath) { if (auto ifsSrc = getIfs(sourceStorageId), ifsDest = getIfs(destStorageId); ifsSrc && ifsSrc == ifsDest) { - return mIncFs->link(ifsSrc->control, oldPath, newPath); + std::string normOldPath = normalizePathToStorage(ifsSrc, sourceStorageId, oldPath); + std::string normNewPath = normalizePathToStorage(ifsDest, destStorageId, newPath); + if (normOldPath.empty() || normNewPath.empty()) { + return -EINVAL; + } + return mIncFs->link(ifsSrc->control, normOldPath, normNewPath); } return -EINVAL; } int IncrementalService::unlink(StorageId storage, std::string_view path) { if (auto ifs = getIfs(storage)) { - return mIncFs->unlink(ifs->control, path); + std::string normOldPath = normalizePathToStorage(ifs, storage, path); + return mIncFs->unlink(ifs->control, normOldPath); } return -EINVAL; } diff --git a/services/incremental/IncrementalService.h b/services/incremental/IncrementalService.h index ca5e4dbd9231..f8518fd58679 100644 --- a/services/incremental/IncrementalService.h +++ b/services/incremental/IncrementalService.h @@ -18,8 +18,8 @@ #include <android-base/strings.h> #include <android-base/unique_fd.h> -#include <android/os/incremental/IIncrementalManager.h> #include <android/content/pm/DataLoaderParamsParcel.h> +#include <android/os/incremental/IIncrementalManager.h> #include <binder/IServiceManager.h> #include <utils/String16.h> #include <utils/StrongPointer.h> @@ -92,8 +92,7 @@ public: std::optional<std::future<void>> onSystemReady(); - StorageId createStorage(std::string_view mountPoint, - DataLoaderParamsParcel&& dataLoaderParams, + StorageId createStorage(std::string_view mountPoint, DataLoaderParamsParcel&& dataLoaderParams, CreateOptions options = CreateOptions::Default); StorageId createLinkedStorage(std::string_view mountPoint, StorageId linkedStorage, CreateOptions options = CreateOptions::Default); @@ -207,6 +206,8 @@ private: void deleteStorage(IncFsMount& ifs); void deleteStorageLocked(IncFsMount& ifs, std::unique_lock<std::mutex>&& ifsLock); MountMap::iterator getStorageSlotLocked(); + std::string normalizePathToStorage(const IfsMountPtr incfs, StorageId storage, + std::string_view path); // Member variables std::unique_ptr<VoldServiceWrapper> mVold; diff --git a/services/incremental/test/IncrementalServiceTest.cpp b/services/incremental/test/IncrementalServiceTest.cpp index 28268181f173..9cdc83e75055 100644 --- a/services/incremental/test/IncrementalServiceTest.cpp +++ b/services/incremental/test/IncrementalServiceTest.cpp @@ -406,29 +406,16 @@ TEST_F(IncrementalServiceTest, testMakeDirectory) { int storageId = mIncrementalService->createStorage(tempDir.path, std::move(mDataLoaderParcel), IncrementalService::CreateOptions::CreateNew); - std::string_view dir_path("test"); - EXPECT_CALL(*mIncFs, makeDir(_, dir_path, _)); - auto res = mIncrementalService->makeDir(storageId, dir_path, 0555); - ASSERT_EQ(res, 0); -} + std::string dir_path("test"); -TEST_F(IncrementalServiceTest, testMakeDirectoryNested) { - mVold->mountIncFsSuccess(); - mIncFs->makeFileSuccess(); - mVold->bindMountSuccess(); - mIncrementalManager->prepareDataLoaderSuccess(); - mIncrementalManager->startDataLoaderSuccess(); - TemporaryDir tempDir; - int storageId = - mIncrementalService->createStorage(tempDir.path, std::move(mDataLoaderParcel), - IncrementalService::CreateOptions::CreateNew); - auto first = "first"sv; - auto second = "second"sv; - std::string dir_path = std::string(first) + "/" + std::string(second); - EXPECT_CALL(*mIncFs, makeDir(_, first, _)).Times(0); - EXPECT_CALL(*mIncFs, makeDir(_, second, _)).Times(0); - EXPECT_CALL(*mIncFs, makeDir(_, std::string_view(dir_path), _)).Times(1); + std::string tempPath(tempDir.path); + std::replace(tempPath.begin(), tempPath.end(), '/', '_'); + std::string mount_dir = std::string(mRootDir.path) + "/" + tempPath.substr(1); + std::string normalized_dir_path = mount_dir + "/mount/st_1_0/" + dir_path; + // Expecting incfs to call makeDir on a path like: + // /data/local/tmp/TemporaryDir-06yixG/data_local_tmp_TemporaryDir-xwdFhT/mount/st_1_0/test + EXPECT_CALL(*mIncFs, makeDir(_, std::string_view(normalized_dir_path), _)); auto res = mIncrementalService->makeDir(storageId, dir_path, 0555); ASSERT_EQ(res, 0); } @@ -446,15 +433,29 @@ TEST_F(IncrementalServiceTest, testMakeDirectories) { auto first = "first"sv; auto second = "second"sv; auto third = "third"sv; + + std::string tempPath(tempDir.path); + std::replace(tempPath.begin(), tempPath.end(), '/', '_'); + std::string mount_dir = std::string(mRootDir.path) + "/" + tempPath.substr(1); + InSequence seq; auto parent_path = std::string(first) + "/" + std::string(second); auto dir_path = parent_path + "/" + std::string(third); - EXPECT_CALL(*mIncFs, makeDir(_, std::string_view(dir_path), _)).WillOnce(Return(-ENOENT)); - EXPECT_CALL(*mIncFs, makeDir(_, std::string_view(parent_path), _)).WillOnce(Return(-ENOENT)); - EXPECT_CALL(*mIncFs, makeDir(_, first, _)).WillOnce(Return(0)); - EXPECT_CALL(*mIncFs, makeDir(_, std::string_view(parent_path), _)).WillOnce(Return(0)); - EXPECT_CALL(*mIncFs, makeDir(_, std::string_view(dir_path), _)).WillOnce(Return(0)); - auto res = mIncrementalService->makeDirs(storageId, dir_path, 0555); + + std::string normalized_first_path = mount_dir + "/mount/st_1_0/" + std::string(first); + std::string normalized_parent_path = mount_dir + "/mount/st_1_0/" + parent_path; + std::string normalized_dir_path = mount_dir + "/mount/st_1_0/" + dir_path; + + EXPECT_CALL(*mIncFs, makeDir(_, std::string_view(normalized_dir_path), _)) + .WillOnce(Return(-ENOENT)); + EXPECT_CALL(*mIncFs, makeDir(_, std::string_view(normalized_parent_path), _)) + .WillOnce(Return(-ENOENT)); + EXPECT_CALL(*mIncFs, makeDir(_, std::string_view(normalized_first_path), _)) + .WillOnce(Return(0)); + EXPECT_CALL(*mIncFs, makeDir(_, std::string_view(normalized_parent_path), _)) + .WillOnce(Return(0)); + EXPECT_CALL(*mIncFs, makeDir(_, std::string_view(normalized_dir_path), _)).WillOnce(Return(0)); + auto res = mIncrementalService->makeDirs(storageId, normalized_dir_path, 0555); ASSERT_EQ(res, 0); } } // namespace android::os::incremental |