From owner-freebsd-arch@freebsd.org Fri Sep 15 10:08:32 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 ECB19E0B8A8; Fri, 15 Sep 2017 10:08:32 +0000 (UTC) (envelope-from etnapierala@gmail.com) Received: from mail-ua0-x22e.google.com (mail-ua0-x22e.google.com [IPv6:2607:f8b0:400c:c08::22e]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id A1CB613CB; Fri, 15 Sep 2017 10:08:32 +0000 (UTC) (envelope-from etnapierala@gmail.com) Received: by mail-ua0-x22e.google.com with SMTP id k63so989334uak.12; Fri, 15 Sep 2017 03:08:32 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:sender:in-reply-to:references:from:date:message-id :subject:to:cc; bh=WvemsmAcbVVWcb6anF3UZZ+QeMxdOBfA88FTxcKo+ws=; b=VZWi7kTq2IAgUQE+gA0YYcj9cdAMsVCfx1cDT1J8zY8k3U6U3Ja43x4dQADan18dE7 c2Tfl9wUktOd1EbBgArhs1b7jsI2F5PUu61chWeBzMxgu5Qju4/7sZptGmlKJS+eXoJ3 lLQ3wJ1IDvVNQhES1oA0NctUW+wyC0IRCxhinpfQMlcylg4kQJUI64S07UIoQcEGk+e1 07SkwVvhJ6qhPw3oPe9pRdFYvuTl4exFJ5mIe/LSsW8tgXoqkFcrqX4Uu++JVh03PkiX AM0paIIQ5et3rAMav01kUNEgkkiRF5qQgcSDV0oPvm1UrRVpWBF1GmaMtqaWoUfyqmzw QOcw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:sender:in-reply-to:references:from :date:message-id:subject:to:cc; bh=WvemsmAcbVVWcb6anF3UZZ+QeMxdOBfA88FTxcKo+ws=; b=kOSaB8SMIZXmB6iz4d0pCoJHwzuFFRpa54LJILC96AvRjr90PdHwjIO0hqZXZm4zTQ Mz1mOBG2RVj3HyYJr4NkdgINqkpacatNxsMG7czaeiswE3ujdtDU2E0s8YbQaCAMSH+2 +CMhdTNWoRGO7LkyNiDQ6D2d/Uo8ojPYl4VsSOONo2EVzMAq+qdFG+XDOwATByQJD5b0 BDni/6fU9DA6zNEDVpkh76/vKmtK1cPvx/tRteqNvef5ZHyeOFWAGOlw+ZsmkYB4YOBm 01aaWv6I3dRRvxJ1CCJwelrz1J+4Xeyv29eNWMQIqWPEfrsbA3iLrd1AZNAIHOVTjgOr lCfw== X-Gm-Message-State: AHPjjUhyxv4vqZQrdVzzBepZ+KLzfhr9mS8sySZKYcG6P5EvOTwWDlRd WhZFeTIB8rrKPPww8p4O8fEz4mZUKi1bp3pe278= X-Google-Smtp-Source: ADKCNb5R0j+c+YzO6Bucq8sonl/3eRgkdBN5qOndAXN9iO1TjCIxwv02Qlx/4fY9BcOqHCn/6mX3lR7JqVL1Kyeh2E0= X-Received: by 10.176.21.109 with SMTP id p42mr12368078uae.127.1505470111627; Fri, 15 Sep 2017 03:08:31 -0700 (PDT) MIME-Version: 1.0 Sender: etnapierala@gmail.com Received: by 10.159.51.232 with HTTP; Fri, 15 Sep 2017 03:08:31 -0700 (PDT) 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> From: Edward Napierala Date: Fri, 15 Sep 2017 11:08:31 +0100 X-Google-Sender-Auth: q1_4RPx-uknuYeYqYQVra7HLIj8 Message-ID: Subject: Re: mount / unmount and mountcheckdirs() To: Konstantin Belousov Cc: Kirk McKusick , freebsd-fs , Andriy Gapon , "freebsd-arch@freebsd.org" Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.23 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 10:08:33 -0000 2017-09-15 10:20 GMT+01:00 Konstantin Belousov : > 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. > 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.