Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

[Eric W. Biederman]

Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> writes:

> On Wed, Aug 16, 2017 at 12:56 PM, Linus Torvalds
> <torvalds@xxxxxxxxxxxxxxxxxxxx> 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