Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   ebiederm@xmission.com (Eric W. Biederman)
To:     "Michael Kerrisk \(man-pages\)" <mtk.manpages@gmail.com>
Cc:     Philipp Wendler <ml@philippwendler.de>,
        "Serge E. Hallyn" <serge@hallyn.com>,
        Christian Brauner <christian@brauner.io>,
        Aleksa Sarai <asarai@suse.de>,
        Reid Priedhorsky <reidpr@lanl.gov>,
        Andy Lutomirski <luto@amacapital.net>,
        Yang Bo <rslovers@yandex.com>, Jakub Wilk <jwilk@jwilk.net>,
        Joseph Sible <josephcsible@gmail.com>,
        Al Viro <viro@ftp.linux.org.uk>, werner@almesberger.net,
        linux-man <linux-man@vger.kernel.org>,
        lkml <linux-kernel@vger.kernel.org>,
        Containers <containers@lists.linux-foundation.org>,
        =?utf-8?Q?St=C3=A9p?= =?utf-8?Q?hane?= Graber 
        <stgraber@ubuntu.com>
References: <620c691a-065e-b894-4f06-7453012bc8d3@gmail.com>
        <d449305b-f87c-f26e-e43f-d193fd8f4332@philippwendler.de>
        <e51e454c-b0e7-e5d1-7810-e8f023574aa2@gmail.com>
Date:   Wed, 09 Oct 2019 03:46:02 -0500
In-Reply-To: <e51e454c-b0e7-e5d1-7810-e8f023574aa2@gmail.com> (Michael
        Kerrisk's message of "Wed, 9 Oct 2019 09:41:34 +0200")
Message-ID: <87y2xu71dh.fsf@x220.int.ebiederm.org>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8BIT
Subject: Re: For review: rewritten pivot_root(2) manual page
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

"Michael Kerrisk (man-pages)" <mtk.manpages@gmail.com> writes:

> Hello Philipp,
>
> My apologies that it has taken a while to reply. (I had been hoping
> and waiting that a few more people might weigh in on this thread.)
>
> On 9/23/19 3:42 PM, Philipp Wendler wrote:
>> Hello Michael,
>> 
>> Am 23.09.19 um 14:04 schrieb Michael Kerrisk (man-pages):
>> 
>>> I'm considering to rewrite these pieces to exactly
>>> describe what the system call does (which I already
>>> do in the third paragraph) and remove the "may or may not"
>>> pieces in the second paragraph. I'd welcome comments
>>> on making that change.
>> 
>> I think that it would make the man page significantly easier to
>> understand if if the vague wording and the meta discussion about it are
>> removed.
>
> It is my inclination to make this change, but I'd love to get more
> feedback on this point.
>
>>> DESCRIPTION
>> [...]>        pivot_root()  changes  the
>>>        root  directory  and the current working directory of each process
>>>        or thread in the same mount namespace to new_root if they point to
>>>        the  old  root  directory.   (See also NOTES.)  On the other hand,
>>>        pivot_root() does not change the caller's current  working  direc‐
>>>        tory  (unless it is on the old root directory), and thus it should
>>>        be followed by a chdir("/") call.
>> 
>> There is a contradiction here with the NOTES (cf. below).
>
> See below.
>
>>>        The following restrictions apply:
>>>
>>>        -  new_root and put_old must be directories.
>>>
>>>        -  new_root and put_old must not be on the same filesystem as  the
>>>           current root.  In particular, new_root can't be "/" (but can be
>>>           a bind mounted directory on the current root filesystem).
>> 
>> Wouldn't "must not be on the same mountpoint" or something similar be
>> more clear, at least for new_root? The note in parentheses indicates
>> that new_root can actually be on the same filesystem as the current
>> note. However, ...
>
> For 'put_old', it really is "filesystem".

If we are going to be pedantic "filesystem" is really the wrong concept
here.  The section about bind mount clarifies it, but I wonder if there
is a better term.

I think I would say: "new_root and put_old must not be on the same mount
as the current root."

I think using "mount" instead of "filesystem" keeps the concepts less
confusing.

As I am reading through this email and seeing text that is trying to be
precise and clear then hitting the term "filesystem" is a bit jarring.
pivot_root doesn't care a thing for file systems.  pivot_root only cares
about mounts.

And by a "mount" I mean the thing that you get when you create a bind
mount or you call mount normally.

Michael do you have man pages for the new mount api yet?

> For 'new_root', see below.
>
>>>        -  put_old must be at or underneath new_root; that  is,  adding  a
>>>           nonnegative  number  of /.. to the string pointed to by put_old
>>>           must yield the same directory as new_root.
>>>
>>>        -  new_root must be a mount point.  (If  it  is  not  otherwise  a
>>>           mount  point,  it  suffices  to  bind  mount new_root on top of
>>>           itself.)
>> 
>> ... this item actually makes the above item almost redundant regarding
>> new_root (except for the "/") case. So one could replace this item with
>> something like this:
>> 
>> - new_root must be a mount point different from "/". (If it is not
>>   otherwise a mount point, it suffices  to bind mount new_root on top
>>   of itself.)
>> 
>> The above item would then only mention put_old (and maybe use clarified
>> wording on whether actually a different file system is necessary for
>> put_old or whether a different mount point is enough).
>
> Thanks. That's a good suggestion. I simplified the earlier bullet
> point as you suggested, and changed the text here to say:
>
>        -  new_root must be a mount point, but can't be "/".  If it is not
>           otherwise  a mount point, it suffices to bind mount new_root on
>           top of itself.  (new_root can be a bind  mounted  directory  on
>           the current root filesystem.)

How about:
          - new_root must be the path to a mount, but can't be "/".  Any
          path that is not already a mount can be converted into one by
          bind mounting the path onto itself.
          
>>> NOTES
>> [...]
>>>        pivot_root() allows the caller to switch to a new root  filesystem
>>>        while  at  the  same time placing the old root mount at a location
>>>        under new_root from where it can subsequently be unmounted.   (The
>>>        fact  that  it  moves  all processes that have a root directory or
>>>        current working directory on the old root filesystem  to  the  new
>>>        root  filesystem  frees the old root filesystem of users, allowing
>>>        it to be unmounted more easily.)
>> 
>> Here is the contradiction:
>> The DESCRIPTION says that root and current working dir are only changed
>> "if they point to the old root directory". Here in the NOTES it says
>> that any root or working directories on the old root file system (i.e.,
>> even if somewhere below the root) are changed.
>> 
>> Which is correct?
>
> The first text is correct. I must have accidentally inserted
> "filesystem" into the paragraph just here during a global edit.
> Thanks for catching that.
>
>> If it indeed affects all processes with root and/or current working
>> directory below the old root, the text here does not clearly state what
>> the new root/current working directory of theses processes is.
>> E.g., if a process is at /foo and we pivot to /bar, will the process be
>> moved to /bar (i.e., at / after pivot_root), or will the kernel attempt
>> to move it to some location like /bar/foo? Because the latter might not
>> even exist, I suspect that everything is just moved to new_root, but
>> this could be stated explicitly by replacing "to the new root
>> filesystem" in the above paragraph with "to the new root directory"
>> (after checking whether this is true).
>
> The text here now reads:
>
>        pivot_root() allows the caller to switch to a new root  filesystem
>        while  at  the  same time placing the old root mount at a location
>        under new_root from where it can subsequently be unmounted.   (The
>        fact  that  it  moves  all processes that have a root directory or
>        current working directory on the old root  directory  to  the  new
>        root  frees the old root directory of users, allowing the old root
>        filesystem to be unmounted more easily.)


Please "mount" instead of "filesystem".

>>> EXAMPLE>        The program below demonstrates the use of  pivot_root()  inside  a
>>>        mount namespace that is created using clone(2).  After pivoting to
>>>        the root directory named in the program's first command-line argu‐
>>>        ment,  the  child  created  by  clone(2) then executes the program
>>>        named in the remaining command-line arguments.
>> 
>> Why not use the pivot_root(".", ".") in the example program?
>> It would make the example shorter, and also works if the process cannot
>> write to new_root (e..g., in a user namespace).
>
> I'm not sure. Some people have a bit of trouble to wrap their head
> around the pivot_root(".", ".") idea. (I possibly am one of them.)
> I'd be quite keen to hear other opinions on this. Unfortunately,
> few people have commented on this manual page rewrite.

I am happy as long as it is pivot_root(".", ".") is documented
somewhere.  There is real code that uses it so it is not going away.
Plus pivot_root(".", ".") is really what is desired in a lot of
situations where the caller of pivot_root is an intermediary and
does not control the new root filesystem.  At which point the only
path you can be guaranteed to exit on the new root filesystem is "/".

Eric