[Toybox] [PATCH] Support mount -o private mountpoint

[Rob Landley]

On 6/21/22 11:30, Yi-yo Chiang via Toybox wrote:
> `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).

I think you want to do "mount -o remount,private mountpoint"?

> `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.

Is mountpoint an existing mountpoint? Because you should either get an overmount
or an "is already mounted" error, unless MS_PRIVATE is being _REALLY_ magic in
the kernel behind the scenes and doing an implicit -o remount?

> 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, "")

Yes, it would. Among other things, no MS_REMOUNT flag...

> 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.

That's what -o remount does. Are you saying you want -o remount to be implicit here?

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

Not looking like it makes MS_REMOUNT implicit when
MS_PRIVATE/SHARED/SLAVE/UNBINDABLE is giving with one argument...

+testing "-o private not_a_mountpoint" \
+  "mount -o private '${mountpoint}' 2>/dev/null || echo error" "error\n" "" ""

Because you didn't tell it WHAT to mount, so yes it should error if this isn't a
remount. (Did your patch change this behavior?)

+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" "" ""

You're bind mounting a directory on itself and adding the private flag: did that
not already work? (If it wasn't _already_ a mount point, I mean.)

I admit I haven't tested the private/shared/slave stuff much because it ties
into namespaces/containers which I have a big todo hairball for. but I think I
tested this at one point? I note that mount.tests is one of the big things I
want to get tests running under mkroot for, because "run as root, leave the
system in a bad state if it didn't finish cleanly" is not something I want to do
on my development laptop a lot. Getting that to work soonish has been the focus
of my recent sh.c commits...

Rob