commit 18eb8772467d45a234cd4343a4e9a45cb938d996
author Yurii Zubrytskyi <zyy@google.com> Mon Oct 18 22:33:15 2021 -0700
committer Alex Buynytskyy <alexbuy@google.com> Mon Jan 31 22:13:14 2022 +0000
tree 9d367be630c1eebb27be99c4b9a01fdc6f55c7fd
parent 313e8556a6fe5b23f4cc0e83fbdc2b0933f5d65c

[vold] Check incremental paths before mounting

Vold was trusting system_server too much and allowed for pretty
much any path in mount()/bindMount() calls for incremental.
This CL adds validation to make sure it's only accessing own
directories. This includes enforcing no symlinks in the paths

Bug: 198657657
Bug: 216722132
Test: manual
Change-Id: I6035447f94ef44c4ae3294c3ae47de2d7210683a
Merged-In: I6035447f94ef44c4ae3294c3ae47de2d7210683a

VoldNativeService.cpp

diff --git a/VoldNativeService.cpp b/VoldNativeService.cpp
index 1c94220..1033af9 100644
--- a/VoldNativeService.cpp
+++ b/VoldNativeService.cpp
@@ -966,10 +966,25 @@
         const std::string& sysfsName,
         ::android::os::incremental::IncrementalFileSystemControlParcel* _aidl_return) {
     ENFORCE_SYSTEM_OR_ROOT;
-    CHECK_ARGUMENT_PATH(backingPath);
-    CHECK_ARGUMENT_PATH(targetDir);
+    if (auto status = CheckIncrementalPath(IncrementalPathKind::MountTarget, targetDir);
+        !status.isOk()) {
+        return status;
+    }
+    if (auto status = CheckIncrementalPath(IncrementalPathKind::MountSource, backingPath);
+        !status.isOk()) {
+        return status;
+    }
 
-    auto control = incfs::mount(backingPath, targetDir,
+    auto [backingFd, backingSymlink] = OpenDirInProcfs(backingPath);
+    if (!backingFd.ok()) {
+        return translate(-errno);
+    }
+    auto [targetFd, targetSymlink] = OpenDirInProcfs(targetDir);
+    if (!targetFd.ok()) {
+        return translate(-errno);
+    }
+
+    auto control = incfs::mount(backingSymlink, targetSymlink,
                                 {.flags = IncFsMountFlags(flags),
                                  // Mount with read timeouts.
                                  .defaultReadTimeoutMs = INCFS_DEFAULT_READ_TIMEOUT_MS,
@@ -992,9 +1007,15 @@
 
 binder::Status VoldNativeService::unmountIncFs(const std::string& dir) {
     ENFORCE_SYSTEM_OR_ROOT;
-    CHECK_ARGUMENT_PATH(dir);
+    if (auto status = CheckIncrementalPath(IncrementalPathKind::Any, dir); !status.isOk()) {
+        return status;
+    }
 
-    return translate(incfs::unmount(dir));
+    auto [fd, symLink] = OpenDirInProcfs(dir);
+    if (!fd.ok()) {
+        return translate(-errno);
+    }
+    return translate(incfs::unmount(symLink));
 }
 
 binder::Status VoldNativeService::setIncFsMountOptions(
@@ -1042,10 +1063,22 @@
 binder::Status VoldNativeService::bindMount(const std::string& sourceDir,
                                             const std::string& targetDir) {
     ENFORCE_SYSTEM_OR_ROOT;
-    CHECK_ARGUMENT_PATH(sourceDir);
-    CHECK_ARGUMENT_PATH(targetDir);
+    if (auto status = CheckIncrementalPath(IncrementalPathKind::Any, sourceDir); !status.isOk()) {
+        return status;
+    }
+    if (auto status = CheckIncrementalPath(IncrementalPathKind::Bind, targetDir); !status.isOk()) {
+        return status;
+    }
 
-    return translate(incfs::bindMount(sourceDir, targetDir));
+    auto [sourceFd, sourceSymlink] = OpenDirInProcfs(sourceDir);
+    if (!sourceFd.ok()) {
+        return translate(-errno);
+    }
+    auto [targetFd, targetSymlink] = OpenDirInProcfs(targetDir);
+    if (!targetFd.ok()) {
+        return translate(-errno);
+    }
+    return translate(incfs::bindMount(sourceSymlink, targetSymlink));
 }
 
 binder::Status VoldNativeService::destroyDsuMetadataKey(const std::string& dsuSlot) {