From owner-freebsd-arch@freebsd.org Fri Sep 15 09:20:13 2017 Return-Path: Delivered-To: freebsd-arch@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 29CB1E092B9; Fri, 15 Sep 2017 09:20:13 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from kib.kiev.ua (kib.kiev.ua [IPv6:2001:470:d5e7:1::1]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id C2FE58378C; Fri, 15 Sep 2017 09:20:12 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from tom.home (kib@localhost [127.0.0.1]) by kib.kiev.ua (8.15.2/8.15.2) with ESMTPS id v8F9K1Ai048801 (version=TLSv1.2 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Fri, 15 Sep 2017 12:20:01 +0300 (EEST) (envelope-from kostikbel@gmail.com) DKIM-Filter: OpenDKIM Filter v2.10.3 kib.kiev.ua v8F9K1Ai048801 Received: (from kostik@localhost) by tom.home (8.15.2/8.15.2/Submit) id v8F9K1Ar048800; Fri, 15 Sep 2017 12:20:01 +0300 (EEST) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: tom.home: kostik set sender to kostikbel@gmail.com using -f Date: Fri, 15 Sep 2017 12:20:01 +0300 From: Konstantin Belousov To: Kirk McKusick Cc: Andriy Gapon , freebsd-fs , freebsd-arch@FreeBSD.org Subject: Re: mount / unmount and mountcheckdirs() Message-ID: <20170915092001.GK78693@kib.kiev.ua> References: <134c7c6e-f4f1-ef38-cc50-0e56c27c9fb8@FreeBSD.org> <201709150314.v8F3Ea6B085072@chez.mckusick.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <201709150314.v8F3Ea6B085072@chez.mckusick.com> User-Agent: Mutt/1.9.0 (2017-09-02) X-Spam-Status: No, score=-2.0 required=5.0 tests=ALL_TRUSTED,BAYES_00, DKIM_ADSP_CUSTOM_MED,FREEMAIL_FROM,NML_ADSP_CUSTOM_MED autolearn=no autolearn_force=no version=3.4.1 X-Spam-Checker-Version: SpamAssassin 3.4.1 (2015-04-28) on tom.home X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 15 Sep 2017 09:20:13 -0000 On Thu, Sep 14, 2017 at 08:14:36PM -0700, Kirk McKusick wrote: > > To: Kirk McKusick > > Subject: Re: mount / unmount and mountcheckdirs() > > Cc: freebsd-arch@FreeBSD.org, freebsd-fs > > From: Andriy Gapon > > 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. In fact, the behavior implemented by mountcheckdirs() is surprising as well. For instance, I did expected that the system would operate as if mountcheckdirs() does not exist, and it caused me some head-scratching when I see it first time.