Date: Fri, 15 Sep 2017 08:19:15 -0700 (PDT) From: "Rodney W. Grimes" <freebsd-rwg@pdx.rh.CN85.dnsmgr.net> To: Edward Napierala <trasz@freebsd.org> Cc: Konstantin Belousov <kostikbel@gmail.com>, 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: <201709151519.v8FFJFHK065086@pdx.rh.CN85.dnsmgr.net> In-Reply-To: <CAFLM3-rqg9Pso636Y3Vm6JT0WxWossEfvnsMmmAbC-jEkvyS7Q@mail.gmail.com>
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
This process should be blocked before the cd is completed and
resumed once the automouter has done its job. Even per the
autofs man page that is what should be happening:
DESCRIPTION
The autofs driver is the kernel component of the automounter
infrastructure. Its job is to pass mount requests to the automountd(8)
daemon, and pause the processes trying to access the automounted
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
filesystem until the mount is completed. It is mounted by the
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
automount(8).
> [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
This looks like a fail of the autofs code to block the
process in cd before it changes the cwd.
> [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.
--
Rod Grimes rgrimes@freebsd.org
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201709151519.v8FFJFHK065086>