Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 14 Sep 2017 15:45:07 +0300
From:      Andriy Gapon <avg@FreeBSD.org>
To:        Kirk McKusick <mckusick@chez.mckusick.com>
Cc:        freebsd-arch@FreeBSD.org, freebsd-fs <freebsd-fs@FreeBSD.org>
Subject:   Re: mount / unmount and mountcheckdirs()
Message-ID:  <134c7c6e-f4f1-ef38-cc50-0e56c27c9fb8@FreeBSD.org>
In-Reply-To: <201605220640.u4M6enEo017327@chez.mckusick.com>
References:  <201605220640.u4M6enEo017327@chez.mckusick.com>

next in thread | previous in thread | raw e-mail | index | archive | help
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



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?134c7c6e-f4f1-ef38-cc50-0e56c27c9fb8>