diff --git a/sandboxed_api/sandbox2/BUILD.bazel b/sandboxed_api/sandbox2/BUILD.bazel index 860094c..a9c0481 100644 --- a/sandboxed_api/sandbox2/BUILD.bazel +++ b/sandboxed_api/sandbox2/BUILD.bazel @@ -629,7 +629,6 @@ cc_library( deps = [ ":mount_tree_cc_proto", ":mounts", - ":util", ":violation_cc_proto", "//sandboxed_api/util:file_base", "//sandboxed_api/util:fileops", @@ -928,7 +927,6 @@ cc_test( copts = sapi_platform_copts(), deps = [ ":util", - "//sandboxed_api:testing", "@com_google_googletest//:gtest_main", ], ) diff --git a/sandboxed_api/sandbox2/CMakeLists.txt b/sandboxed_api/sandbox2/CMakeLists.txt index 9afa099..ec717bc 100644 --- a/sandboxed_api/sandbox2/CMakeLists.txt +++ b/sandboxed_api/sandbox2/CMakeLists.txt @@ -586,7 +586,6 @@ target_link_libraries(sandbox2_namespace PRIVATE sandbox2::mounts sandbox2::mount_tree_proto sapi::strerror - sandbox2::util sandbox2::violation_proto sapi::base sapi::raw_logging @@ -1035,7 +1034,6 @@ if(BUILD_TESTING AND SAPI_BUILD_TESTING) OUTPUT_NAME util_test ) target_link_libraries(sandbox2_util_test PRIVATE - sapi::testing sandbox2::util sapi::test_main ) diff --git a/sandboxed_api/sandbox2/namespace.cc b/sandboxed_api/sandbox2/namespace.cc index f8ab7de..795ef78 100644 --- a/sandboxed_api/sandbox2/namespace.cc +++ b/sandboxed_api/sandbox2/namespace.cc @@ -35,7 +35,6 @@ #include "absl/strings/str_cat.h" #include "absl/strings/str_format.h" #include "absl/strings/string_view.h" -#include "sandboxed_api/sandbox2/util.h" #include "sandboxed_api/util/fileops.h" #include "sandboxed_api/util/path.h" #include "sandboxed_api/util/raw_logging.h" @@ -67,8 +66,9 @@ int MountFallbackToReadOnly(const char* source, const char* target, void PrepareChroot(const Mounts& mounts) { // Create a tmpfs mount for the new rootfs. - SAPI_RAW_CHECK(util::CreateDirRecursive(kSandbox2ChrootPath, 0700), - "could not create directory for rootfs"); + SAPI_RAW_CHECK( + file_util::fileops::CreateDirectoryRecursively(kSandbox2ChrootPath, 0700), + "could not create directory for rootfs"); SAPI_RAW_PCHECK(mount("none", kSandbox2ChrootPath, "tmpfs", 0, nullptr) == 0, "mounting rootfs failed"); @@ -340,13 +340,15 @@ void Namespace::InitializeNamespaces(uid_t uid, gid_t gid, int32_t clone_flags, void Namespace::InitializeInitialNamespaces(uid_t uid, gid_t gid) { SetupIDMaps(uid, gid); - SAPI_RAW_CHECK(util::CreateDirRecursive(kSandbox2ChrootPath, 0700), - "could not create directory for rootfs"); + SAPI_RAW_CHECK( + file_util::fileops::CreateDirectoryRecursively(kSandbox2ChrootPath, 0700), + "could not create directory for rootfs"); SAPI_RAW_PCHECK(mount("none", kSandbox2ChrootPath, "tmpfs", 0, nullptr) == 0, "mounting rootfs failed"); auto realroot_path = file::JoinPath(kSandbox2ChrootPath, "/realroot"); - SAPI_RAW_CHECK(util::CreateDirRecursive(realroot_path, 0700), - "could not create directory for real root"); + SAPI_RAW_CHECK( + file_util::fileops::CreateDirectoryRecursively(realroot_path, 0700), + "could not create directory for real root"); SAPI_RAW_PCHECK(syscall(__NR_pivot_root, kSandbox2ChrootPath, realroot_path.c_str()) != -1, "pivot root"); diff --git a/sandboxed_api/sandbox2/util.cc b/sandboxed_api/sandbox2/util.cc index e3cafdb..fb1d90c 100644 --- a/sandboxed_api/sandbox2/util.cc +++ b/sandboxed_api/sandbox2/util.cc @@ -19,7 +19,6 @@ #include #include #include -#include #include #include #include @@ -169,34 +168,6 @@ long Syscall(long sys_no, // NOLINT return syscall(sys_no, a1, a2, a3, a4, a5, a6); } -bool CreateDirRecursive(const std::string& path, mode_t mode) { - int error = mkdir(path.c_str(), mode); - - if (error == 0 || errno == EEXIST) { - return true; - } - - // We couldn't create the dir for reasons we can't handle. - if (errno != ENOENT) { - return false; - } - - // The EEXIST case, the parent directory doesn't exist yet. - // Let's create it. - const std::string dir = file_util::fileops::StripBasename(path); - if (dir == "/" || dir.empty()) { - return false; - } - if (!CreateDirRecursive(dir, mode)) { - return false; - } - - // Now the parent dir exists, retry creating the directory. - error = mkdir(path.c_str(), mode); - - return error == 0; -} - namespace { int ChildFunc(void* arg) { diff --git a/sandboxed_api/sandbox2/util.h b/sandboxed_api/sandbox2/util.h index 7062017..f91aa01 100644 --- a/sandboxed_api/sandbox2/util.h +++ b/sandboxed_api/sandbox2/util.h @@ -78,9 +78,6 @@ long Syscall(long sys_no, // NOLINT uintptr_t a1 = 0, uintptr_t a2 = 0, uintptr_t a3 = 0, uintptr_t a4 = 0, uintptr_t a5 = 0, uintptr_t a6 = 0); -// Recursively creates a directory, skipping segments that already exist. -bool CreateDirRecursive(const std::string& path, mode_t mode); - // Fork based on clone() which updates glibc's PID/TID caches - Based on: // https://chromium.googlesource.com/chromium/src/+/9eb564175dbd452196f782da2b28e3e8e79c49a5%5E!/ // diff --git a/sandboxed_api/sandbox2/util_test.cc b/sandboxed_api/sandbox2/util_test.cc index be7318d..0d8b20d 100644 --- a/sandboxed_api/sandbox2/util_test.cc +++ b/sandboxed_api/sandbox2/util_test.cc @@ -16,31 +16,15 @@ #include -#include - #include "gmock/gmock.h" #include "gtest/gtest.h" -#include "sandboxed_api/testing.h" namespace sandbox2::util { namespace { -using ::sapi::GetTestTempPath; using ::testing::Gt; using ::testing::IsTrue; -constexpr char kTestDir[] = "a/b/c"; - -TEST(UtilTest, TestCreateDirSuccess) { - EXPECT_THAT(CreateDirRecursive(GetTestTempPath(kTestDir), 0700), IsTrue()); -} - -TEST(UtilTest, TestCreateDirExistSuccess) { - const std::string test_dir = GetTestTempPath(kTestDir); - EXPECT_THAT(CreateDirRecursive(test_dir, 0700), IsTrue()); - EXPECT_THAT(CreateDirRecursive(test_dir, 0700), IsTrue()); -} - TEST(UtilTest, TestCreateMemFd) { int fd = 0; ASSERT_THAT(CreateMemFd(&fd), IsTrue()); diff --git a/sandboxed_api/util/fileops.cc b/sandboxed_api/util/fileops.cc index 5514c12..e5eb271 100644 --- a/sandboxed_api/util/fileops.cc +++ b/sandboxed_api/util/fileops.cc @@ -220,6 +220,30 @@ bool ListDirectoryEntries(const std::string& directory, return true; } +bool CreateDirectoryRecursively(const std::string& path, int mode) { + if (mkdir(path.c_str(), mode) == 0 || errno == EEXIST) { + return true; + } + + // We couldn't create the dir for reasons we can't handle. + if (errno != ENOENT) { + return false; + } + + // The ENOENT case, the parent directory doesn't exist yet. + // Let's create it. + const std::string dir = StripBasename(path); + if (dir == "/" || dir.empty()) { + return false; + } + if (!CreateDirectoryRecursively(dir, mode)) { + return false; + } + + // Now the parent dir exists, retry creating the directory. + return mkdir(path.c_str(), mode) == 0; +} + bool DeleteRecursively(const std::string& filename) { std::vector to_delete; to_delete.push_back(filename); diff --git a/sandboxed_api/util/fileops.h b/sandboxed_api/util/fileops.h index 6ba4c23..b2fb28f 100644 --- a/sandboxed_api/util/fileops.h +++ b/sandboxed_api/util/fileops.h @@ -89,6 +89,9 @@ bool ListDirectoryEntries(const std::string& directory, std::vector* entries, std::string* error); +// Recursively creates a directory, skipping segments that already exist. +bool CreateDirectoryRecursively(const std::string& path, int mode); + // Deletes the specified file or directory, including any sub-directories. bool DeleteRecursively(const std::string& filename); diff --git a/sandboxed_api/util/fileops_test.cc b/sandboxed_api/util/fileops_test.cc index a1c51e1..fd9f89a 100644 --- a/sandboxed_api/util/fileops_test.cc +++ b/sandboxed_api/util/fileops_test.cc @@ -332,6 +332,12 @@ void SetupDirectory() { ASSERT_THAT(chmod("foo/bar/baz/foo", 0644), Eq(0)); } +TEST_F(FileOpsTest, CreateDirectoryRecursivelyTest) { + constexpr char kTestDir[] = "a/b/c"; + EXPECT_THAT(fileops::CreateDirectoryRecursively(kTestDir, 0700), IsTrue()); + EXPECT_THAT(fileops::CreateDirectoryRecursively(kTestDir, 0700), IsTrue()); +} + TEST_F(FileOpsTest, DeleteRecursivelyTest) { EXPECT_THAT(fileops::DeleteRecursively("foo"), IsTrue()); EXPECT_THAT(fileops::DeleteRecursively("/not_there"), IsTrue());