commit 211ca09f1d6ed58be703d9dd01fe43e623f74129
author Joel Hockey <joelhockey@chromium.org> Tue Dec 04 16:16:39 2018 -0800
committer Commit Bot <commit-bot@chromium.org> Sat Aug 08 00:26:35 2020 +0000
tree ab603f3e1e05e398a9460f672e95930db4bce953
parent 3526f6d330874b1e3f955c71af3ea6e84421fc07

vm_tools: seneschal: Force unmount MNT_DETACH for MyFiles if EBUSY

MyFiles has Downloads bind-mounted within it.  When MyFiles is shared
and mounted, the Downloads mount propagates.

When we unshare MyFiles, we first try to unmount MyFiles/Downloads, but
it fails with EINVAL.  If we then continue and unmount MyFiles, it fails
with EBUSY regardless of whether any files are open.

This fixes by ignoring the first error for MyFiles/Downloads, and then
reattempting to unmount with MNT_DETACH set if unmounting MyFiles fails
with EBUSY.

BUG=chromium:1112190
TEST=share then unshare MyFiles root

Change-Id: Ib3995d8f867928c451eab3ff987b391cce795f84
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform2/+/1404454
Tested-by: Joel Hockey <joelhockey@chromium.org>
Reviewed-by: Chirantan Ekbote <chirantan@chromium.org>
Commit-Queue: Joel Hockey <joelhockey@chromium.org>

vm_tools/seneschal/service.cc

diff --git a/vm_tools/seneschal/service.cc b/vm_tools/seneschal/service.cc
index 0e61b59..111aba0 100644
--- a/vm_tools/seneschal/service.cc
+++ b/vm_tools/seneschal/service.cc
@@ -977,6 +977,8 @@
   base::FilePath server_root =
       iter->second.root_dir().GetPath().Append(&kServerRoot[1]);
   base::FilePath dst = server_root.Append(path);
+  base::FilePath my_files = server_root.Append("MyFiles");
+  base::FilePath my_files_downloads = my_files.Append("Downloads");
   // Ensure path exists.
   if (!base::PathExists(dst)) {
     LOG(ERROR) << "Unshare path does not exist";
@@ -1065,6 +1067,24 @@
   for (auto iter = mount_points.rbegin(), end = mount_points.rend();
        iter != end; ++iter) {
     if (umount(iter->value().c_str()) != 0) {
+      // When MyFiles is shared, its MyFiles/Downloads mount propagates. It
+      // seems that the kernel does not allow us to unmount MyFiles/Downloads
+      // with EINVAL, and then also fails to unmount MyFiles with EBUSY even
+      // when no files are open.
+      if (errno == EINVAL && dst == my_files &&
+          iter->value() == my_files_downloads.value()) {
+        // Ignore EINVAL when unsharing MyFiles and MyFiles/Downloads fails.
+        PLOG(WARNING)
+            << "Unmount MyFiles/Downloads failed with EINVAL, ignoring";
+        continue;
+      } else if (errno == EBUSY && iter->value() == my_files.value()) {
+        // If/when unmount MyFiles fails with EBUSY, we retry with MNT_DETACH.
+        PLOG(WARNING)
+            << "Unmount MyFiles failed with EBUSY, attempting MNT_DETACH";
+        if (umount2(iter->value().c_str(), MNT_DETACH) == 0) {
+          continue;
+        }
+      }
       PLOG(ERROR) << "Failed to unmount";
       response.set_failure_reason("Failed to unmount");
       writer.AppendProtoAsArrayOfBytes(response);