linux-kernel - Re: [PATCH 0/1] devpts: use dynamic

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87y3qjoty5.fsf@xmission.com>
Date:   Wed, 16 Aug 2017 20:37:38 -0500
From:   ebiederm@...ssion.com (Eric W. Biederman)
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     Christian Brauner <christian.brauner@...onical.com>,
        Christian Brauner <christian.brauner@...ntu.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        "Serge E. Hallyn" <serge@...lyn.com>,
        Al Viro <viro@...iv.linux.org.uk>
Subject: Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

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

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