Re: [PATCH 01/16] devpts: Attempting to get it right

[Andy Lutomirski]

On Fri, Apr 15, 2016 at 8:34 AM, Eric W. Biederman
<ebiederm@xxxxxxxxxxxx> wrote:
>
> To recap the situation for those who have not been following closely.
>
> There are programs such as xen-create-image that run as root and setup
> a chroot environment with:
> "mknod dev/ptmx c 5 2"
> "mkdir dev/pts"
> "mount -t devpts none dev/pts"
>
> Which mostly works but stomps the mount options of the system /dev/pts.
> In particular the options of "gid=5,mode=620" are lost resulting in a
> situation where creating a new pty by opening /dev/ptmx results in
> that pty having the wrong permissions.
>
> Some distributions have been working around this problem by continuing
> to install a setuid root pt_chown binary that will be called by glibc
> to fix the permissions.
>
> Maintaining a setuid root pt_chown binary is not too scary until
> multiple instances of devpts are considered at which point it becomes
> possible to trick the setuid root pt_chown binary into operating on the
> wrong files and directories.  Leading to all of the things one might
> fear when a setuid root binary goes wrong.
>
> The following patchset digs us out of that hole by carefully devpts and
> /dev/ptmx in a way that does not introduce any userspace regressions,
> while making each mount of devpts distinct (so pt_chown is unnecessary)
> and arranging things so that enough information is available so
> that a secure pt_chown binary is possible to write if that is ever
> needed.
>
> The approach I have chosen to take is to first enhance the /dev/ptmx
> device node to automount /dev/pts/ptmx on top of it.  This leads to a
> simple high performance solution that allows applications such as
> xen-create-image (that call "mknod ptmx c 5 2" and mount devpts)
> to continue to run as before even when they are given a non-system
> instance of devpts.
>
> Using automountic bind mounts of /dev/pts/ptmx results in no new
> security cases to consider as this can already be done, and actually
> results in a simplification of the analysis of the code.  As now all
> opens of ptmx are of /dev/pts/ptmx.  /dev/ptmx is now just a magic
> mountpoint that does the right thing.

And what happens when someone tries to rm /dev/ptmx or unmount their
pts instance or similar?  What happens if /dev/ptmx is in a mount that
is set to propagate elsewhere but /dev/pts was replaced by an
unprivileged user?  (Can this happen?  I'm not sure.)

This seems much weirder than the previous approach.  I think I'm
starting to come over to Linus' view -- the magic lookup was fine, and
I still can't think of a case where the permissions matter.  If we
care, we can cause the /dev/ptmx magic lookup to fail if the devpts it
finds was created with newinstance.  (After all, devpts instances
created with newinstance *never* worked via /dev/ptmx magic.)

--Andy