Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 15 Sep 2017 11:08:31 +0100
From:      Edward Napierala <trasz@freebsd.org>
To:        Konstantin Belousov <kostikbel@gmail.com>
Cc:        Kirk McKusick <mckusick@mckusick.com>, freebsd-fs <freebsd-fs@freebsd.org>, Andriy Gapon <avg@freebsd.org>, "freebsd-arch@freebsd.org" <freebsd-arch@freebsd.org>
Subject:   Re: mount / unmount and mountcheckdirs()
Message-ID:  <CAFLM3-rqg9Pso636Y3Vm6JT0WxWossEfvnsMmmAbC-jEkvyS7Q@mail.gmail.com>
In-Reply-To: <20170915092001.GK78693@kib.kiev.ua>
References:  <134c7c6e-f4f1-ef38-cc50-0e56c27c9fb8@FreeBSD.org> <201709150314.v8F3Ea6B085072@chez.mckusick.com> <20170915092001.GK78693@kib.kiev.ua>

next in thread | previous in thread | raw e-mail | index | archive | help
2017-09-15 10:20 GMT+01:00 Konstantin Belousov <kostikbel@gmail.com>:

> On Thu, Sep 14, 2017 at 08:14:36PM -0700, Kirk McKusick wrote:
> > > To: Kirk McKusick <mckusick@mckusick.com>
> > > Subject: Re: mount / unmount and mountcheckdirs()
> > > Cc: freebsd-arch@FreeBSD.org, freebsd-fs <freebsd-fs@FreeBSD.org>
> > > From: Andriy Gapon <avg@FreeBSD.org>
> > > Date: Thu, 14 Sep 2017 15:45:07 +0300
> > >
> > > On 22/05/2016 09:40, Kirk McKusick wrote:
> > >> I added the checkdirs functionality in the mount direction only
> > >> (I actually did it in 4.4BSD-Lite and it got swept in with commit
> > >> 22521). The reason is that when a directory that is not empty is
> > >> mounted on, the expectation is that the entries in that directory
> > >> should no longer be present; rather they should be replaced by the
> > >> entries in the newly mounted directory. Thus all processes sitting
> > >> in the mounted on directory should see the newly mounted directory
> > >> as if they had come to it using a lookup after the mount had been
> > >> done. If a process had proceeded through the mounted on directory
> > >> into one of its other entries, then they are left alone until such
> > >> time as they chdir back into the mount point directory through ".."
> > >> at which time they will be passed up to the mounted directory using
> > >> the same mechanism that would put them there if they traversed into
> > >> the mount point from above it in the tree. I believe this is the
> > >> correct behavior, is not a security threat, and should be left alone.
> > >
> > > I almost dropped a ball on this issue, but I am now picking it up
> again.
> > > At the moment I am moving forward with the dounmount change as it
> seems to be
> > > non-contentious and rather simple to do and test.
> > >
> > > Regarding the mount part, I am not sure that I completely agree with
> you.
> > > Even if mountcheckdirs() does not cause any problems in the mount
> path, I still
> > > fail to see its usefulness.  Specifically, I still do not see any
> significant
> > > difference between the covered directory and any directory below it.
> So, if we
> > > leave the lower directories alone, while bother with the covered
> directory...
> > >
> > > The covered directory:
> > > - absolute paths work correctly
> > > - relative paths with enough ".." (one) can access the actual namespace
> > > - other relative paths operate on the shadowed sub-tree of the original
> > >   filesystem
> > >
> > >
> > > The lower directories:
> > > - absolute paths work correctly
> > > - relative paths with enough ".." (> 1) can access the actual namespace
> > > - other relative paths operate on the shadowed sub-tree of the original
> > >   filesystem
> > >
> > > The only difference I can think of is that the root of the mounted
> filesystem
> > > cannot be reached with just ".."-s from the covered directory.  But is
> this
> > > difference of any significance?
> > >
> > > Mateusz also raised some interesting points.
> > >
> > > On the other hand, it seems that illumos and probably Solaris has
> > > interesting parallels to the FreeBSD behavior.  It does not allow
> > > to mount over a directory that is a current directory for any process
> > > ("Device busy"), but does not object against processes in directories
> > > below the mount point.
> > >
> > > So, probably it's just I who misses something about that scenario :-)
> > >
> > >> I was not aware that the functionality had been added at unmount
> > >> time, and I do not believe that it should have been done. Normally
> > >> an unmount will not succeed if any vnodes are busy (for example, if
> > >> any directory in the filesystem is a current directory). The only
> > >> way that it can succeed in such a case is if a forcible unmount is
> > >> done. The forcible unmount will effectively do a revoke(2) on all
> > >> current directory vnodes in the unmounted filesystem. Further attempts
> > >> to access them will fail with "." not found errors. The only way to
> > >> get a valid current directory is to chdir to an absolute pathname.
> > >> Gratuitously fixing this if you happen to be in the former root of
> > >> the filesystem is wrong. And as you note can lead to unintensionally
> > >> giving an escape path from a prison. So I concur with your removing
> > >> this added functionality.
> > > --
> > > Andriy Gapon
> >
> > I had to dig back through some *really* old emails to find out what
> > triggered the addition of mountcheckdirs(). The problem that it was
> > specifically solving was that as part of the startup script a minimal
> > root directory was replaced by the real root directory. The shell
> > running the startup script needed to be moved to the new mounted-on
> > root so that the rest of the script would not fail.
> If the mountcheckdirs() code not going away, please add your spelunking
> results as a comment before the function. This theme is recurring, and
> it would be highly beneficial to not loose the non-trivial reasoning
> behind the code existence.
>
> >
> > That disaster of a hack has been replaced with the much more functional
> > code that deals with setting up the root and the devfs filesystem on
> > /dev. So the need for which it was designed no longer exists. But I
> > still believe that it is the correct thing to do. For example, if you
> > are using automount code and chdir into your home directory triggering
> > an auto-mount, you should just be in your home directory after the
> > mount rather than having to do cd ../$USER to get there.
> I believe that the current autofs does not allow a process to get into
> this situation at all.
>

It does.  For example:

[trasz@v2:~]% cd /media/md0
[trasz@v2:/media/md0]% mount
/dev/ada0s1a on / (ufs, local, noatime, journaled soft-updates)
devfs on /dev (devfs, local, multilabel)
map -hosts on /net (autofs)
map -media on /media (autofs)
[trasz@v2:/media/md0]% ls
[trasz@v2:/media/md0]% mount
/dev/ada0s1a on / (ufs, local, noatime, journaled soft-updates)
devfs on /dev (devfs, local, multilabel)
map -hosts on /net (autofs)
map -media on /media (autofs)
/dev/md0 on /media/md0 (ufs, local, noatime, nosuid, automounted)

Getting rid of mountcheckdirs() in the unmount path should be fine, I think.



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAFLM3-rqg9Pso636Y3Vm6JT0WxWossEfvnsMmmAbC-jEkvyS7Q>