`mount -o private mountpoint` should change the mount propagation type
of mountpoint to MS_PRIVATE. This didn't work because it falls into
the "read /etc/fstab" branch (one argument), and fails when mountpoint
is not found in the fstab, or fstab is not found (Android case).

`mount -o private blah mountpoint` kind of works, as long as `blah`
doesn't look like a directory, otherwise toybox would assume bind mount
and MS_BIND would win over MS_PRIVATE.

Even if `blah mountpoint` looks sufficient unlike a bind mount, the
underlying mount() syscall would be a bit off,
  mount(blah, mountpoint, "ext3", MS_SILENT|MS_PRIVATE, "")

The "ext3" (or whatever comes first in /proc/filesystems) is because
unspecified `-t` defaults to `-t auto`, which means "try mount FS in
/proc/filesystems one-by-one", which is not what we want here. We don't
want to mount anything, but change an existing mountpoint's propagation
type.

This patch adds "mount -o private mountpoint" machinery and unit test.

-- 

Yi-yo Chiang
Software Engineer
[email protected]

I support flexible work schedules, and I’m sending this email now because
it is within the hours I’m working today. Please do not feel obliged to
reply straight away - I understand that you will reply during the hours you
work, which may not match mine.
From ad241856cabbc073047770c44c8340fb94cfb14a Mon Sep 17 00:00:00 2001
From: Yi-Yo Chiang <[email protected]>
Date: Tue, 21 Jun 2022 23:59:54 +0800
Subject: [PATCH] Support mount -o private mountpoint

`mount -o private mountpoint` should change the mount propagation type
of mountpoint to MS_PRIVATE. This didn't work because it falls into
the "read /etc/fstab" branch (one argument), and fails when mountpoint
is not found in the fstab, or fstab is not found (Android case).

`mount -o private blah mountpoint` kind of works, as long as `blah`
doesn't look like a directory, otherwise toybox would assume bind mount
and MS_BIND would win over MS_PRIVATE.

Even if `blah mountpoint` looks sufficient unlike a bind mount, the
underlying mount() syscall would be a bit off,
  mount(blah, mountpoint, "ext3", MS_SILENT|MS_PRIVATE, "")

The "ext3" (or whatever comes first in /proc/filesystems) is because
unspecified `-t` defaults to `-t auto`, which means "try mount FS in
/proc/filesystems one-by-one", which is not what we want here. We don't
want to mount anything, but change an existing mountpoint's propagation
type.

This patch adds "mount -o private mountpoint" machinery and unit test.
---
 tests/mount.test | 26 ++++++++++++++++++++++++++
 toys/lsb/mount.c | 26 +++++++++++++++++---------
 2 files changed, 43 insertions(+), 9 deletions(-)

diff --git a/tests/mount.test b/tests/mount.test
index 4f7a667f..5c867479 100644
--- a/tests/mount.test
+++ b/tests/mount.test
@@ -91,3 +91,29 @@ reCreateTmpFs
 
 umount testDir1
 rm -f $tmp_b_fs
+
+mountpoint="${PWD}/mountpoint"
+mkdir "${mountpoint}"
+
+testing "-o private not_a_mountpoint" \
+  "mount -o private '${mountpoint}' 2>/dev/null || echo error" "error\n" "" ""
+
+testing "-o bind,private dir dir (MS_BIND wins MS_PRIVATE)" \
+  "mount -o bind,private '${mountpoint}' '${mountpoint}' 2>&1 &&
+   umount '${mountpoint}' && echo yes" "yes\n" "" ""
+
+mount --bind "${mountpoint}" "${mountpoint}"
+testing "-o private mountpoint" \
+  "mount -o private '${mountpoint}' 2>&1 &&
+   cat /proc/self/mountinfo | grep '${mountpoint}' | tail -1 | grep -q -v 'shared:' && echo yes" \
+  "yes\n" "" ""
+umount "${mountpoint}"
+
+mount --bind "${mountpoint}" "${mountpoint}"
+testing "-o private mountpoint && mount -o shared mountpoint" \
+  "mount -o private '${mountpoint}' 2>&1 && mount -o shared '${mountpoint}' 2>&1 &&
+   cat /proc/self/mountinfo | grep '${mountpoint}' | tail -1 | grep -q 'shared:' && echo yes" \
+  "yes\n" "" ""
+umount "${mountpoint}"
+
+rmdir "${mountpoint}"
diff --git a/toys/lsb/mount.c b/toys/lsb/mount.c
index f5fdf2b4..4ab6da37 100644
--- a/toys/lsb/mount.c
+++ b/toys/lsb/mount.c
@@ -269,7 +269,7 @@ static void mount_filesystem(char *dev, char *dir, char *type,
 
 void mount_main(void)
 {
-  char *opts = 0, *dev = 0, *dir = 0, **ss;
+  char *opts = 0, *dev = 0, *dir = 0, *more = 0, **ss;
   long flags = MS_SILENT;
   struct arg_list *o;
   struct mtab_list *mtl, *mtl2 = 0, *mm, *remount;
@@ -301,18 +301,29 @@ void mount_main(void)
 
   if (FLAG(a) && dir) error_exit("-a with >1 arg");
 
+  flags = flag_opts(opts, flags, &more);
+
   // For remount we need _last_ match (in case of overmounts), so traverse
   // in reverse order. (Yes I'm using remount as a boolean for a bit here,
-  // the double cast is to get gcc to shut up about it.)
-  remount = (void *)(long)comma_scan(opts, "remount", 0);
+  // the cast is to get gcc to shut up about it.)
+  remount = (void *)(flags & MS_REMOUNT);
   if ((FLAG(a) && !access("/proc/mounts", R_OK)) || remount) {
     mm = dlist_terminate(mtl = mtl2 = xgetmountlist(0));
     if (remount) remount = mm;
   }
 
+  // Changing the mountpoint propagation type?
+  // One of the propagation type flags, not remount, not bind mount, superuser,
+  // one argument.
+  if ((flags & (MS_SHARED | MS_PRIVATE | MS_SLAVE | MS_UNBINDABLE)) &&
+      !(flags & (MS_REMOUNT | MS_BIND)) && !getuid() && dev && !dir) {
+    // When changing the propagation type, only the mountpoint and mount flags
+    // are revelant, ignore the extra options.
+    mount_filesystem("", dev, "", flags, "");
+
   // Do we need to do an /etc/fstab trawl?
   // This covers -a, -o remount, one argument, all user mounts
-  if (FLAG(a) || (dev && (!dir || getuid() || remount))) {
+  } else if (FLAG(a) || (dev && (!dir || getuid() || remount))) {
     if (!remount) dlist_terminate(mtl = xgetmountlist("/etc/fstab"));
 
     for (mm = remount ? remount : mtl; mm; mm = (remount ? mm->prev : mm->next))
@@ -349,7 +360,7 @@ void mount_main(void)
       // user only counts from fstab, not opts.
       if (!mmm) {
         TT.okuser = comma_scan(mm->opts, "user", 1);
-        aflags = flag_opts(mm->opts, flags, &aopts);
+        aflags = flag_opts(mm->opts, MS_SILENT, &aopts);
         aflags = flag_opts(opts, aflags, &aopts);
 
         mount_filesystem(mm->device, mm->dir, mm->type, aflags, aopts);
@@ -380,10 +391,7 @@ void mount_main(void)
 
   // two arguments
   } else {
-    char *more = 0;
-
-    flags = flag_opts(opts, flags, &more);
     mount_filesystem(dev, dir, TT.type, flags, more);
-    if (CFG_TOYBOX_FREE) free(more);
   }
+  if (CFG_TOYBOX_FREE) free(more);
 }
-- 
2.37.0.rc0.104.g0611611a94-goog

_______________________________________________
Toybox mailing list
[email protected]
http://lists.landley.net/listinfo.cgi/toybox-landley.net

Reply via email to