From: ebiederm@xmission.com (Eric W. Biederman)
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Christian Brauner <christian.brauner@canonical.com>,
        Christian Brauner <christian.brauner@ubuntu.com>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        "Serge E. Hallyn" <serge@hallyn.com>,
        Al Viro <viro@zeniv.linux.org.uk>
References: <20170816171211.4021-1-christian.brauner@ubuntu.com>
        <CA+55aFzqedKjjq+FDF3AEgyQfqQRHyR-6PTR_BTvyyQLqnPwPw@mail.gmail.com>
        <CA+55aFyy1RU2emO6giU9gNVb_doYP+K_y61ij2cumgs1_VMMmA@mail.gmail.com>
        <20170816194805.hnof3aqiqykwki7p@gmail.com>
        <CA+55aFzmcydKsuwXpocd0hYMrACEXt+UUuaqa7j3L5LG7SjajQ@mail.gmail.com>
        <CA+55aFxMnEQVq8zRG1JEsW+6x23qkBV=KKXksgVHmBegPuepMg@mail.gmail.com>
Date: Wed, 16 Aug 2017 20:37:38 -0500
In-Reply-To: <CA+55aFxMnEQVq8zRG1JEsW+6x23qkBV=KKXksgVHmBegPuepMg@mail.gmail.com>
        (Linus Torvalds's message of "Wed, 16 Aug 2017 13:19:01 -0700")
Message-ID: <87y3qjoty5.fsf@xmission.com>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.1 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain
Subject: Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2589
Lines: 62

Linus Torvalds <torvalds@linux-foundation.org> writes:

> On Wed, Aug 16, 2017 at 12:56 PM, Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
>>
>> So the fact that we _don't_ get the right pathname for the pts entry
>> here means that something got screwed up in setting filp->f_path to
>> the right thing. We have all the code in place that _tries_ to do it,
>> but it clearly has a bug somewhere.
>
> Ok, I think I see what the bug is, although I don't have a fix for it yet.
>
> We generate the path largely correctly: the path has a nice dentry
> that contains the right pts number, and has the right parent pointer
> that points to the root of the pts mount.
>
> And we also fill in the path 'mnt' field. Everything should be fine.
>
> Except when we actually hit that root dentry of the pts mount, the
> code in prepend_path() hits this condition:
>
>                 if (dentry == vfsmnt->mnt_root || IS_ROOT(dentry)) {
>                         struct mount *parent = ACCESS_ONCE(mnt->mnt_parent);
>                         /* Escaped? */
>                         if (dentry != vfsmnt->mnt_root) {
>
> and we break out, and reset the path to '/' because we think it
> somehow escaped out of the user namespace.

Escaped it's bind mount actually.  There should always be a path
from mnt_root to a dentry under that mount point.  In some rare caseses
involving bind mounts a rename that moves a dentry from one directory to
another can result in dentries that are not reachable from mnt_root.

As those entries do not have a path in a meaningful sense setting the
path to '/' is the best we can do.

This condition should be limited to bind mounts as any dentry on a
filesystem is descendent from the filesystems root directory.

The rest of your analysis below is correct.

My apologies for the pendantic reply.  I am repling just so that someone
doesn't find this in an email archive 20 years from now and become
impossibly confused.


> So it looks like we filled in the path with the *wrong* mount information.
>
> And THAT in turn is because we fill the path with the mount
> information for the "/dev/ptmx" field - which is *not* in the
> /dev/pts/ mount - that's the mount for '/dev'.
>
> So we have a dentry and a mnt, but they simply aren't paired up correctly.
>
> And you can see this with your test program: if you open /dev/pts/ptmx
> for the master, it actually works correctly (but you need to make sure
> the permissions for that ptmx node allow that).
>
> Anyway, I know what's wrong, next step is to figure out what the fix is.
>
>                    Linus