From: Christian Brauner <christian.brauner@canonical.com>
To: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
Al Viro <viro@zeniv.linux.org.uk>,
Serge Hallyn <serge@hallyn.com>,
Stefan Lippers-Hollmann <s.l-h@gmx.de>,
Christian Brauner <christian.brauner@ubuntu.com>,
Thorsten Leemhuis <regressions@leemhuis.info>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name
Date: Fri, 25 Aug 2017 01:37:38 +0200 [thread overview]
Message-ID: <20170824233737.umna3s7vsbuv6oi6@gmail.com> (raw)
In-Reply-To: <877exs7epb.fsf@xmission.com>
On Thu, Aug 24, 2017 at 06:01:36PM -0500, Eric W. Biederman wrote:
> Linus Torvalds <torvalds@linux-foundation.org> writes:
>
> > On Thu, Aug 24, 2017 at 1:43 PM, Eric W. Biederman
> > <ebiederm@xmission.com> wrote:
> >>
> >> There are just enough weird one off scripts like xen image builder (I
> >> think that was the nasty test case that broke in debian) that I can't
> >> imagine ever being able to responsibly remove the path based lookups in
> >> /dev/ptmx. I do dream of it sometimes.
> >
> > Not going to happen.
>
> Which is what I said.
>
> > The fact is, /dev/ptmx is the simply the standard location.
> > /dev/pts/ptmx simply is *not*.
>
> The standard is posix_openpt(). That is a syscall on the bsds.
> Opening something called ptmx at this point is a Linuxism.
>
> There are a lot of programs that are going to be calling posix_openpt()
> simply because /dev/ptmx can not be counted on to exist.
>
> > So pretty much every single user that ever uses pty's will use
> > /dev/ptmx, it's just how it has always worked.
> >
> > Trying to change it to anything else is just stupid. There's no
> > upside, there is only downsides - mainly the "we'll have to support
> > the standard way anyway, that newfangled way doesn't add anything".
>
> Except the new fangled way does add quite a bit. Not everyone who
> mounts devpts has permission to call mknod. So /dev/ptmx frequently
> winds up either being a bind mount or a symlink to /dev/pts/ptmx in
> containers.
In fact, /dev/ptmx being a symlink or bind-mount is the *standard* in containers
even for non-user namespaced containers or containers that do not retain
CAP_MKNOD.
>
> It is going to take a long time but device nodes like one of those
> filesystem features thare are very slowly on their way out.
This related to the point above: The fact that we can mount a devpts at its
standard location but are unable to also have/create an additional device node
at the *standard location* is usually quite irritating for people who do not
know about this "legacy" behaviour. But yeah, it's probably going away but
that's going to be a long long time. I agree that userspace is the place to
slowly make the transition though. :)
>
> > Our "pts" lookup isn't expensive.
> >
> > So quite frankly, we should discourage people from using the
> > non-standard place. It really has no real advantages, and it's simply
> > not worth it.
>
> The "pts" lookup admitted isn't runtime expensive. I could propbably
> measure a cost but anyone who is creating ptys fast enough to care
> likely has other issues.
>
> The "pts" lookup does have some real maintenance costs as it takes
> someone with a pretty deep understanding of things to figure out what is
> going on. I hope things have finally been abstracted well enough, and
> the code is used heavily enough we don't have to worry about a
> regression there. I still worry.
>
> As for non-standard locations. Anything that isn't /dev/ptmx and
> /dev/pts/NNN simply won't work for anything isn't very specialized.
I was mainly asking about non-standard locations because I experienced weird
behaviour when trying to open("/mnt/<slave-idx", O_RDWR | O_NOCTTY). Mind you I
did all the steps that grantpt() + unlockpt() usually do purely file descriptor
based. But I think this was due to the faulty TIOCGPTPEER implemenation before
which should now be fixed.
> At which point I don't think there is any reason to skip using the ptmx
> node on the devpts filesystem as you have already given up compatibility
> with everything else.
>
> But I agree it doesn't look worth it to change glibc to deal with an
> alternate location for /dev/ptmx. I see a huge point in changing glibc
> to use the new TIOCGPTPEER ioctl when available as that is really the
> functionality the glibc internals are after.
That's a patch I've been looking into. But TIOCGPTPEER alone won't be enough. A
couple of other function such as grantpt() need to switch from path-based
operation to file descriptor based operations too (Something I tried to point
out in one of my previous mails.). The whole user-space api could do - imho -
with a redo. The kernel is doing the right thing and exposing the right bits
mostly; TIOCGPTPEER being a good step. But user-space wise it's actually a
little security nightmare as soon as namespaces and - sorry for the buzzword -
*containers* come into play. @Eric, are you going to be at Plumbers again this
year? That's maybe a good chance to discuss some of this if there's still
interest.
Christian
>
> Eric
next prev parent reply other threads:[~2017-08-24 23:37 UTC|newest]
Thread overview: 55+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-08-16 17:12 [PATCH 0/1] devpts: use dynamic_dname() to generate proc name Christian Brauner
2017-08-16 17:12 ` [PATCH 1/1] " Christian Brauner
2017-08-16 18:26 ` [PATCH 0/1] " Linus Torvalds
2017-08-16 18:48 ` Linus Torvalds
2017-08-16 19:48 ` Christian Brauner
2017-08-16 19:56 ` Linus Torvalds
2017-08-16 20:19 ` Linus Torvalds
2017-08-16 20:30 ` Linus Torvalds
2017-08-16 21:03 ` Linus Torvalds
2017-08-16 21:37 ` Christian Brauner
2017-08-16 21:45 ` Linus Torvalds
2017-08-16 21:55 ` Linus Torvalds
2017-08-16 22:05 ` Christian Brauner
2017-08-16 22:28 ` Christian Brauner
2017-08-23 15:31 ` Eric W. Biederman
2017-08-23 21:15 ` Christian Brauner
2017-08-16 22:46 ` Eric W. Biederman
2017-08-16 22:58 ` Linus Torvalds
2017-08-16 23:51 ` Eric W. Biederman
2017-08-17 0:08 ` Linus Torvalds
2017-08-17 1:24 ` Eric W. Biederman
2017-08-24 0:24 ` Stefan Lippers-Hollmann
2017-08-24 0:42 ` Linus Torvalds
2017-08-24 1:16 ` Linus Torvalds
2017-08-24 1:25 ` Eric W. Biederman
2017-08-24 1:32 ` Linus Torvalds
2017-08-24 1:49 ` Linus Torvalds
2017-08-24 2:01 ` Linus Torvalds
2017-08-24 3:11 ` Eric W. Biederman
2017-08-24 3:24 ` Linus Torvalds
2017-08-24 15:51 ` Eric W. Biederman
2017-08-24 4:24 ` Stefan Lippers-Hollmann
2017-08-24 15:54 ` Eric W. Biederman
2017-08-24 17:52 ` Linus Torvalds
2017-08-24 18:06 ` Linus Torvalds
2017-08-24 18:13 ` Linus Torvalds
2017-08-24 18:31 ` Linus Torvalds
2017-08-24 18:36 ` Linus Torvalds
2017-08-24 20:24 ` Stefan Lippers-Hollmann
2017-08-24 20:27 ` Linus Torvalds
2017-08-24 18:40 ` Eric W. Biederman
2017-08-24 18:51 ` Linus Torvalds
2017-08-24 19:23 ` Eric W. Biederman
2017-08-24 20:13 ` [PATCH v3] pty: Repair TIOCGPTPEER Eric W. Biederman
2017-08-24 21:01 ` Stefan Lippers-Hollmann
[not found] ` <CAPP7u0WHqDfxTW6hmc=DsmHuoALZcrWdU-Odu=FfoTX26SGHQg@mail.gmail.com>
2017-08-24 19:22 ` [PATCH 0/1] devpts: use dynamic_dname() to generate proc name Linus Torvalds
2017-08-24 19:25 ` Linus Torvalds
2017-08-24 20:43 ` Eric W. Biederman
2017-08-24 21:07 ` Linus Torvalds
2017-08-24 23:01 ` Eric W. Biederman
2017-08-24 23:27 ` Linus Torvalds
2017-08-24 23:37 ` Christian Brauner [this message]
2017-08-26 1:00 ` Linus Torvalds
2017-08-24 19:48 ` Eric W. Biederman
2017-08-17 1:37 ` Eric W. Biederman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170824233737.umna3s7vsbuv6oi6@gmail.com \
--to=christian.brauner@canonical.com \
--cc=christian.brauner@ubuntu.com \
--cc=ebiederm@xmission.com \
--cc=linux-kernel@vger.kernel.org \
--cc=regressions@leemhuis.info \
--cc=s.l-h@gmx.de \
--cc=serge@hallyn.com \
--cc=torvalds@linux-foundation.org \
--cc=viro@zeniv.linux.org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox