From: ebiederm@xmission.com (Eric W. Biederman)
To: Peter Hurley <peter@hurleysoftware.com>
Cc: Greg KH <greg@kroah.com>, Jiri Slaby <jslaby@suse.com>,
        "H. Peter Anvin" <hpa@zytor.com>,
        Linus Torvalds <torvalds@linux-foundation.org>,
        Aurelien Jarno <aurelien@aurel32.net>,
        Andy Lutomirski <luto@amacapital.net>,
        Florian Weimer <fw@deneb.enyo.de>, Al Viro <viro@zeniv.linux.org.uk>,
        Serge Hallyn <serge.hallyn@ubuntu.com>, Jann Horn <jann@thejh.net>,
        "security\@kernel.org" <security@kernel.org>,
        "security\@ubuntu.com \>\> security" <security@ubuntu.com>,
        security@debian.org, Willy Tarreau <w@1wt.eu>,
        linux-kernel@vger.kernel.org
References: <43AD2BA7-B594-4299-95F3-D86FD38AF21B@zytor.com>
	<87egexpf4o.fsf@x220.int.ebiederm.org>
	<CA+55aFw9Bg+Zh_T4zP487n3ieaxoMHgZ_nNJVdpSR4kQK9gQ9w@mail.gmail.com>
	<1CB621EF-1647-463B-A144-D611DB150E15@zytor.com>
	<20151208223135.GA8352@kroah.com>
	<87oae0h2bo.fsf@x220.int.ebiederm.org> <56677DE3.5040705@zytor.com>
	<20151209012311.GA11794@kroah.com>
	<84B136DF-55E4-476A-9CB2-062B15677EE5@zytor.com>
	<20151209013859.GA12442@kroah.com> <20151209083225.GA30452@1wt.eu>
	<87wpskyds7.fsf_-_@x220.int.ebiederm.org>
	<566F1CD7.20502@hurleysoftware.com>
Date: Sat, 19 Dec 2015 15:13:28 -0600
In-Reply-To: <566F1CD7.20502@hurleysoftware.com> (Peter Hurley's message of
	"Mon, 14 Dec 2015 11:47:35 -0800")
Message-ID: <87y4cq6t1j.fsf@x220.int.ebiederm.org>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain
Subject: Re: [PATCH] devpts: Sensible /dev/ptmx & force newinstance
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 4148
Lines: 85

Peter Hurley <peter@hurleysoftware.com> writes:

> On 12/11/2015 11:40 AM, Eric W. Biederman wrote:
>> Forcing newinstance for every mount of the devpts filesystem actually
>> requires the association between /dev/ptmx and the currently mounted
>> instance of devpts at /dev/pts.  Simply remembering the first mount of
>> the devpts filesystem and associating that with /dev/ptmx is not
>> enough.  I am aware of at least one instance where an initramfs mounts
>> devpts before the main system instance of devpts is mounted.
>
> Can you point me to that usage please?

I have found that the Dracut versions in CentOS5 and CentOS6 generates
initial ramdisks that mount devpts before the primary OS mount of devpts
on /dev/pts.  I have also found that openwrt-15.05 without an initial
ramdisk does something strange during startup and boots devpts twice as
well.

I have looked but I haven't seen that pattern elsewhere but my search
space of 15ish distros is small compared to what is out there.  Given
that mounting devpts multiple times has been implemented at least twice
independently I would not be surprised if mounting devpts multiple times
during boot shows up somewhere else.

> I ask because there's a patch to move devpts init from module initcall
> up to fs initcall (neither devpts nor the pty driver is actually built
> as a module anyway), and I'd like to look at what the consequences
> might be for that userspace configuration.

I don't expect there are any.  As all of this happens before userspace
initializes anyway.    We have enough variation in the kernel anyway
that the device number the first devpts is mounted on varies between
kernels already.

>> In that system ptys simply did not work after boot when I tested
>> associating /dev/ptmx with the first mount of the devpts filesystem.
>
> Assuming userspace isn't broken by that patch, is a fixed association
> with first mount otherwise an acceptable solution for magic /dev/ptmx
> (where /dev/ptmx is not a symlink to /dev/pts/ptmx)?

I do not believe a fixed association with the first mount is an
acceptable solution for implementing  /dev/ptmx in association with
a change to cause mount of devpts to be an independent filesystem.
Such an association fails to be backwards compatible with existing
userspace, and it is extremely fragile.

If the association between the device node and the filesystem in the
mount namespace is insufficient for backwards compatibility I do not
believe full backwards compatibility is acheivable with a magic version
of /dev/ptmx.

On the flip side the consequences of a ptmx symlink in devpts pointing
to pts/ptmx look extremely minor.  Of my test cases only openwrt-15.05
and CentOS5 fail, as they don't use devtmpfs.  While debian-6.0.2,
debian-7.9, debian-8.2, CentOS6, CentOS7, fedora32, magia-5, mint-17.3,
opensuse-42.1, slackware-14.1, unbuntu-14.04.3 and ubuntu-15.10 all
work.

By making the change in behavior controlled by a kernel command line
option (devpts.newinstance is what I have been testing with) that allows
me to build a single kernel that works on everything.  Which is enough
backwards compatibility for me.

I still have not quite reached the point of testing what the real world
consequences for programs such as lxc that currently use the newinstance
option are.  There is a possibility that if someone is bind mounting
/dev/pts/ptmx over /dev/ptmx they might break.  Similarly there may be a
few cases do "mknod ptmx c 5 2" and that will start failing.

I don't expect running into weird userspace cases that fail will change
my opinion on a path forward, but it will be good to know what the
consequences are of flipping the option.  As so far everything thing
looks like it will just work.

Right now having a nano-flag day and putting a symlink in devtmps looks
a whole lot cleaner in both implementation, maintenance and use than a
magic /dev/ptmx.

Eric
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/