On Tue, Mar 04, 2025 at 01:38:48AM +0000, Naohiro Aota wrote:
> On Mon Mar 3, 2025 at 10:29 PM JST, Naohiro Aota wrote:
> > On Mon Mar 3, 2025 at 7:10 PM JST, Zorro Lang wrote:
> >> On Mon, Mar 03, 2025 at 03:42:09PM +0900, Naohiro Aota wrote:
> >>> While /tmp is mounted with tmpfs in the most setup, it still can be a non-mount
> >>> point. For example, I'm running the fstests in a container, which does not
> >>> mount /tmp inside the container.
> >>>
> >>> Running any test case on such system results in having the following error
> >>> printed, which leads to all the test cases fail due to the output difference.
> >>>
> >>> mount: /tmp: not mount point or bad option.
> >>> dmesg(1) may have more information after failed mount system call.
> >>>
> >>> These lines are printed by the "mount --make-private" command. So, fix that by
> >>> using mountpoint command to check if the directory is a mount point or not.
> >>>
> >>> Fixes: 247ab01fa227 ("check: run tests in a private pid/mount namespace")
> >>> Signed-off-by: Naohiro Aota <naohiro.aota@xxxxxxx>
> >>> ---
> >>> tools/run_privatens | 2 +-
> >>> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/tools/run_privatens b/tools/run_privatens
> >>> index df94974ab30c..c52e0128b8f9 100755
> >>> --- a/tools/run_privatens
> >>> +++ b/tools/run_privatens
> >>> @@ -8,7 +8,7 @@
> >>>
> >>> if [ -n "${FSTESTS_ISOL}" ]; then
> >>> for path in /proc /tmp; do
> >>> - mount --make-private "$path"
> >>> + mountpoint "$path" >/dev/null && mount --make-private "$path"
> >>
> >> Oh, if /tmp isn't a mountpoint on your system, don't you need to think about ...
> >>
> >>> done
> >>> mount -t proc proc /proc
> >>> mount -t tmpfs tmpfs /tmp
> >> ^^^^^^^^^^^^^^^^^^^^^^^^^
> >>
> >> ... this step? I think the first run_privatens process will remain a "mounted"
> >> /tmp on your system.
> >>
> >> And then later tests will use this tmpfs. That's nearly equal to "We always
> >> make sure there's a tmpfs mount on /tmp at the beginning of fstests running".
> >
> > That does not happen because we are already in a mount namespace created
> > by nsexec. The mounted "/tmp" is local to each test, and each test
> > showed the error above.
> >
> >>
> >> But if we don't run that "mount tmpfs" step, I think it's not what this script
> >> wants (to isolate the data in /tmp). Right?
> >
> > I guess we can do these instead?
> >
> > mount --make-private -t proc proc /proc
> > mount --make-private -t tmpfs tmpfs /tmp
> >
> > Actually, I'm not quite confindent that we really need "--make-private"
> > here. As we mount new instance of proc and tmpfs and we don't bind them,
> > I don't see much point making them private.
>
> No, I was wrong. We still need to make them private, not to propagate
> "mount tmpfs" or "mount proc" to the outside. If not, we will see new
> instance of tmpfs mounted on /tmp in the PID 1 environment, which
> breaks several applications :(.
>
> So, in the end, I think my patch did it correctly, no?
Yes. The "mount --make-private" prohibits propagation of changes to
this mount outside of the mount namespace. The "mount -t tmpfs tmpfs
/tmp" creates a new tmpfs that is not exposed to the outside world.
This should've been documented better, Zorro could you please add a few
comments on commit:
if [ -n "${FSTESTS_ISOL}" ]; then
# Don't allow changes to these mountpoints to propagate
# outside of our mount namespace...
for path in /proc /tmp; do
mountpoint "$path" >/dev/null && \
mount --make-private "$path"
done
# ...because here we create new mounts that are private
# to this mount namespace that we don't want to escape.
mount -t proc proc /proc
mount -t tmpfs tmpfs /tmp
fi
Reviewed-by: "Darrick J. Wong" <djwong@xxxxxxxxxx>
--D
> >
> >>
> >> Thanks,
> >> Zorro
> >>
> >>
> >>
> >>> --
> >>> 2.48.1
> >>>
> >>>
