Unmount old postinstall mountpoint from previous runs. am: f411650403 am: ddccea485e
am: b16438bd1a

Change-Id: I781fa3389041ebf4e104fcff585385d8ae5e3689

common/utils.cc

diff --git a/common/utils.cc b/common/utils.cc
index 1cb37e5..ea748c1 100644
--- a/common/utils.cc
+++ b/common/utils.cc
@@ -703,6 +703,32 @@
   return true;
 }
 
+bool IsMountpoint(const std::string& mountpoint) {
+  struct stat stdir, stparent;
+
+  // Check whether the passed mountpoint is a directory and the /.. is in the
+  // same device or not. If mountpoint/.. is in a different device it means that
+  // there is a filesystem mounted there. If it is not, but they both point to
+  // the same inode it basically is the special case of /.. pointing to /. This
+  // test doesn't play well with bind mount but that's out of the scope of what
+  // we want to detect here.
+  if (lstat(mountpoint.c_str(), &stdir) != 0) {
+    PLOG(ERROR) << "Error stat'ing " << mountpoint;
+    return false;
+  }
+  if (!S_ISDIR(stdir.st_mode))
+    return false;
+
+  base::FilePath parent(mountpoint);
+  parent = parent.Append("..");
+  if (lstat(parent.value().c_str(), &stparent) != 0) {
+    PLOG(ERROR) << "Error stat'ing " << parent.value();
+    return false;
+  }
+  return S_ISDIR(stparent.st_mode) &&
+         (stparent.st_dev != stdir.st_dev || stparent.st_ino == stdir.st_ino);
+}
+
 // Tries to parse the header of an ELF file to obtain a human-readable
 // description of it on the |output| string.
 static bool GetFileFormatELF(const uint8_t* buffer, size_t size,

common/utils.h

diff --git a/common/utils.h b/common/utils.h
index d551b7b..8cccc24 100644
--- a/common/utils.h
+++ b/common/utils.h
@@ -192,6 +192,12 @@
                      const std::string& fs_mount_options);
 bool UnmountFilesystem(const std::string& mountpoint);
 
+// Return whether the passed |mountpoint| path is a directory where a filesystem
+// is mounted. Due to detection mechanism limitations, when used on directories
+// where another part of the tree was bind mounted returns true only if bind
+// mounted on top of a different filesystem (not inside the same filesystem).
+bool IsMountpoint(const std::string& mountpoint);
+
 // Returns a human-readable string with the file format based on magic constants
 // on the header of the file.
 std::string GetFileFormat(const std::string& path);

common/utils_unittest.cc

diff --git a/common/utils_unittest.cc b/common/utils_unittest.cc
index 7852910..6e9a911 100644
--- a/common/utils_unittest.cc
+++ b/common/utils_unittest.cc
@@ -478,17 +478,37 @@
   test_utils::ScopedLoopbackDeviceBinder loop_binder(
       tmp_image, true, &loop_dev);
 
+  EXPECT_FALSE(utils::IsMountpoint(mnt_dir.path().value()));
   // This is the actual test part. While we hold a file descriptor open for the
   // mounted filesystem, umount should still succeed.
   EXPECT_TRUE(utils::MountFilesystem(
       loop_dev, mnt_dir.path().value(), MS_RDONLY, "ext4", ""));
+  // Verify the directory is a mount point now.
+  EXPECT_TRUE(utils::IsMountpoint(mnt_dir.path().value()));
+
   string target_file = mnt_dir.path().Append("empty-file").value();
   int fd = HANDLE_EINTR(open(target_file.c_str(), O_RDONLY));
   EXPECT_GE(fd, 0);
   EXPECT_TRUE(utils::UnmountFilesystem(mnt_dir.path().value()));
+  // The filesystem should be already unmounted at this point.
+  EXPECT_FALSE(utils::IsMountpoint(mnt_dir.path().value()));
   IGNORE_EINTR(close(fd));
   // The filesystem was already unmounted so this call should fail.
   EXPECT_FALSE(utils::UnmountFilesystem(mnt_dir.path().value()));
 }
 
+TEST(UtilsTest, IsMountpointTest) {
+  EXPECT_TRUE(utils::IsMountpoint("/"));
+  EXPECT_FALSE(utils::IsMountpoint("/path/to/nowhere"));
+
+  base::ScopedTempDir mnt_dir;
+  EXPECT_TRUE(mnt_dir.CreateUniqueTempDir());
+  EXPECT_FALSE(utils::IsMountpoint(mnt_dir.path().value()));
+
+  base::FilePath file;
+  EXPECT_TRUE(base::CreateTemporaryFile(&file));
+  ScopedPathUnlinker unlinker(file.value());
+  EXPECT_FALSE(utils::IsMountpoint(file.value()));
+}
+
 }  // namespace chromeos_update_engine

payload_consumer/postinstall_runner_action.cc

diff --git a/payload_consumer/postinstall_runner_action.cc b/payload_consumer/postinstall_runner_action.cc
index a1b6f25..11eec34 100644
--- a/payload_consumer/postinstall_runner_action.cc
+++ b/payload_consumer/postinstall_runner_action.cc
@@ -118,6 +118,13 @@
   fs_mount_dir_ = temp_dir.value();
 #endif  // __ANDROID__
 
+  // Double check that the fs_mount_dir is not busy with a previous mounted
+  // filesystem from a previous crashed postinstall step.
+  if (utils::IsMountpoint(fs_mount_dir_)) {
+    LOG(INFO) << "Found previously mounted filesystem at " << fs_mount_dir_;
+    utils::UnmountFilesystem(fs_mount_dir_);
+  }
+
   base::FilePath postinstall_path(partition.postinstall_path);
   if (postinstall_path.IsAbsolute()) {
     LOG(ERROR) << "Invalid absolute path passed to postinstall, use a relative"