mesos-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ji...@apache.org
Subject [2/3] mesos git commit: Overwriting Directories with Files in Copy Provisioner.
Date Tue, 18 Apr 2017 06:30:43 GMT
Overwriting Directories with Files in Copy Provisioner.

When a layer overwrites a directory with a regular file or symbolic
link (or vice versa), the old dir/file need to be removed before
copying the layer into the rootfs. This is processed together with
whiteout: The copy provisioner find all files to remove, including
files marked as whiteout and the files described above, and remove
them before the copy process.

Review: https://reviews.apache.org/r/58408/


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/994df2bb
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/994df2bb
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/994df2bb

Branch: refs/heads/1.2.x
Commit: 994df2bbfd9ff40aff796594948a625abd56b804
Parents: cb94741
Author: Chun-Hung Hsiao <chhsiao@mesosphere.io>
Authored: Tue Apr 18 14:18:09 2017 +0800
Committer: Jie Yu <yujie.jay@gmail.com>
Committed: Tue Apr 18 14:28:04 2017 +0800

----------------------------------------------------------------------
 .../mesos/provisioner/backends/copy.cpp         | 119 +++++++++++--------
 1 file changed, 71 insertions(+), 48 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/994df2bb/src/slave/containerizer/mesos/provisioner/backends/copy.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/provisioner/backends/copy.cpp b/src/slave/containerizer/mesos/provisioner/backends/copy.cpp
index 0ce3e1e..a5cc38d 100644
--- a/src/slave/containerizer/mesos/provisioner/backends/copy.cpp
+++ b/src/slave/containerizer/mesos/provisioner/backends/copy.cpp
@@ -145,60 +145,83 @@ Future<Nothing> CopyBackendProcess::_provision(
   vector<string> whiteouts;
   for (FTSENT *node = ::fts_read(tree);
        node != nullptr; node = ::fts_read(tree)) {
-    if (node->fts_info != FTS_F) {
+    string ftsPath = string(node->fts_path);
+
+    if (node->fts_info == FTS_DNR ||
+        node->fts_info == FTS_ERR ||
+        node->fts_info == FTS_NS) {
+      return Failure(
+          "Failed to read '" + ftsPath + "': " + os::strerror(node->fts_errno));
+    }
+
+    // Skip the postorder visit of a directory.
+    // See the manpage of fts_read in the following link:
+    //   http://man7.org/linux/man-pages/man3/fts_read.3.html
+    if (node->fts_info == FTS_DP) {
       continue;
     }
 
-    if (!strings::startsWith(node->fts_name, docker::spec::WHITEOUT_PREFIX)) {
+    if (ftsPath == layer) {
       continue;
     }
 
-    string ftsPath = string(node->fts_path);
-    Path whiteout = Path(ftsPath.substr(layer.length() + 1));
-
-    // Keep the relative paths of the whiteout files, we will
-    // remove them from rootfs after layer is copied to rootfs.
-    whiteouts.push_back(whiteout.string());
-
-    if (node->fts_name == string(docker::spec::WHITEOUT_OPAQUE_PREFIX)) {
-      const string path = path::join(rootfs, Path(whiteout).dirname());
-
-      // Remove the entries under the directory labeled
-      // as opaque whiteout from rootfs.
-      Try<Nothing> rmdir = os::rmdir(path, true, false);
-      if (rmdir.isError()) {
-        ::fts_close(tree);
-        return Failure(
-            "Failed to remove the entries under the directory labeled as"
-            " opaque whiteout '" + path + "': " + rmdir.error());
+    string layerPath = ftsPath.substr(layer.length() + 1);
+    string rootfsPath = path::join(rootfs, layerPath);
+    Option<string> removePath;
+
+    // Handle whiteout files.
+    if (node->fts_info == FTS_F &&
+        strings::startsWith(node->fts_name, docker::spec::WHITEOUT_PREFIX)) {
+      Path whiteout = Path(layerPath);
+
+      // Keep the absolute paths of the whiteout files, we will
+      // remove them from rootfs after layer is copied to rootfs.
+      whiteouts.push_back(rootfsPath);
+
+      if (node->fts_name == string(docker::spec::WHITEOUT_OPAQUE_PREFIX)) {
+        removePath = path::join(rootfs, whiteout.dirname());
+      } else {
+        removePath = path::join(
+            rootfs,
+            whiteout.dirname(),
+            whiteout.basename().substr(strlen(docker::spec::WHITEOUT_PREFIX)));
       }
-    } else {
-      const string path = path::join(
-          rootfs,
-          whiteout.dirname(),
-          whiteout.basename().substr(strlen(docker::spec::WHITEOUT_PREFIX)));
-
-      // The file/directory labeled as whiteout may have already been
-      // removed with the code above due to its parent directory labeled
-      // as opaque whiteout, so here we need to check if it still exists
-      // before trying to remove it.
-      if (os::exists(path)) {
-        if (os::stat::isdir(path)) {
-          Try<Nothing> rmdir = os::rmdir(path);
-          if (rmdir.isError()) {
-            ::fts_close(tree);
-            return Failure(
-                "Failed to remove the directory labeled as whiteout '" +
-                path + "': " + rmdir.error());
-          }
-        } else {
-          Try<Nothing> rm = os::rm(path);
-          if (rm.isError()) {
-            ::fts_close(tree);
-            return Failure(
-                "Failed to remove the file labeled as whiteout '" +
-                path + "': " + rm.error());
-          }
+    }
+
+    // Handle overwriting between directories and non-directories.
+    // Note: If a symbolic link is overwritten by a directory, the
+    // symbolic link must be removed before the directory is traversed
+    // so the following case won't cause a security issue:
+    //   ROOTFS: /bad@ -> /usr
+    //   LAYER:  /bad/bin/.wh.wh.opq
+    bool ftsIsDir = node->fts_info == FTS_D || node->fts_info == FTS_DC;
+    if (os::exists(rootfsPath) && os::stat::isdir(rootfsPath) != ftsIsDir) {
+      removePath = rootfsPath;
+    }
+
+    // The file/directory referred to by removePath may be empty or have
+    // already been removed because its parent directory is labeled as
+    // opaque whiteout or overwritten by a file, so here we need to
+    // check if it exists before trying to remove it.
+    if (removePath.isSome() && os::exists(removePath.get())) {
+      if (os::stat::isdir(removePath.get())) {
+        // It is OK to remove the entire directory labeled as opaque
+        // whiteout, since the same directory exists in this layer and
+        // will be copied back to rootfs.
+        Try<Nothing> rmdir = os::rmdir(removePath.get());
+        if (rmdir.isError()) {
+          ::fts_close(tree);
+          return Failure(
+              "Failed to remove directory '" +
+              removePath.get() + "': " + rmdir.error());
+        }
+      } else {
+        Try<Nothing> rm = os::rm(removePath.get());
+        if (rm.isError()) {
+          ::fts_close(tree);
+          return Failure(
+              "Failed to remove file '" +
+              removePath.get() + "': " + rm.error());
         }
       }
     }
@@ -256,7 +279,7 @@ Future<Nothing> CopyBackendProcess::_provision(
 
       // Remove the whiteout files from rootfs.
       foreach (const string whiteout, whiteouts) {
-        Try<Nothing> rm = os::rm(path::join(rootfs, whiteout));
+        Try<Nothing> rm = os::rm(whiteout);
         if (rm.isError()) {
           return Failure(
               "Failed to remove whiteout file '" +


Mime
View raw message