From: Christian Brauner <christian.brauner@canonical.com>
To: Christian Brauner <christian.brauner@ubuntu.com>
Cc: ebiederm@xmission.com, libc-alpha@sourceware.org
Subject: Re: [PATCH] getpt: use /dev/pts/ptmx as default ptmx master
Date: Thu, 15 Mar 2018 12:08:00 -0000 [thread overview]
Message-ID: <20180315120814.GA15013@gmail.com> (raw)
In-Reply-To: <20180315120651.14107-1-christian.brauner@ubuntu.com>
On Thu, Mar 15, 2018 at 01:06:51PM +0100, Christian Brauner wrote:
> For a long time now Linux has placed the ptmx character device directly
> under the devpts mount at /dev/pts/ptmx. The /dev/ptmx path today is
> usually either a symlink, an additional character device or a bind-mount.
>
> The idea has always been to slowly fade-out /dev/ptmx and switch to using
> /dev/pts/ptmx exclusively. The kernel currently maintains code to retain
> backwards compatibility for anyone going through /dev/ptmx.
>
> Specifically, if the ptmx device is opened through /dev/ptmx the kernel
> will look for a "pts" directory in the same directory where the /dev/ptmx
> device node resides. This implies that the devpts mount at /dev/pts and the
> /dev/ptmx mount need to have a common ancestor directory. This assumption
> is usually fulfilled when a symlink or separate device node is used.
> However, this assumption will be broken when /dev/ptmx is a bind-mount of
> /dev/pts/ptmx because they are located on different devices. For a detailed
> analysis of this problem please refer to my upstream patch [1].
>
> It is time to start switching to using /dev/pts/ptmx and use /dev/ptmx as a
> fallback only. As far as I can tell, we have three cases to reason about:
>
> 1. /dev/ptmx is a symlink to /dev/pts/ptmx
> In this case devpts must have either been mounted with ptmxmode=0666 or
> chmod 0666 /dev/pts/ptmx must have been called.
> So any open() on /dev/pts/ptmx will succeed.
> 2. /dev/ptmx is a bind-mount of /dev/pts/ptmx
> Analogous to 1. devpts must have either been mounted with ptmxmode=0666
> or chmod 0666 /dev/pts/ptmx must have been called.
> So any open() on /dev/pts/ptmx will succeed.
> 3. /dev/ptmx is a separate ptmx device node
> In this case devpts can either be mounted with ptmxmode=0666 or
> ptmxmode=0000. In the latter case privileged opens of /dev/pts/ptmx will
> succeed while unprivileged opens will fail. The unprivileged failure
> case will be unproblematic since we always fallback to opening /dev/ptmx
> which should have permission 0666. If it doesn't then we would fail the
> exact same way we always did.
>
> [1]: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git/commit/?h=tty-next&id=a319b01d9095da6f6c54bd20c1f1300762506255
>
> Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
In addition to the usual consensus I'd like Eric to comment on this as
well.
Thanks!
Christian
> ---
> ChangeLog | 6 ++++++
> sysdeps/unix/sysv/linux/getpt.c | 18 +++++++++++++++---
> 2 files changed, 21 insertions(+), 3 deletions(-)
>
> diff --git a/ChangeLog b/ChangeLog
> index 38154c20ab..01926472cc 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,9 @@
> +2018-03-15 Christian Brauner <christian.brauner@ubuntu.com>
> +
> + * sysdeps/unix/sysv/linux/getpt.c (__posix_openpt): Try to open
> + ptmx device node through /dev/pts/ptmx and use /dev/ptmx as a
> + fallback.
> +
> 2018-03-15 Siddhesh Poyarekar <siddhesh@sourceware.org>
>
> * sysdeps/aarch64/strncmp.S (strncmp): Use lsr instead of
> diff --git a/sysdeps/unix/sysv/linux/getpt.c b/sysdeps/unix/sysv/linux/getpt.c
> index 77aa468d83..c12a984a36 100644
> --- a/sysdeps/unix/sysv/linux/getpt.c
> +++ b/sysdeps/unix/sysv/linux/getpt.c
> @@ -25,11 +25,15 @@
>
> #include "linux_fsinfo.h"
>
> -/* Path to the master pseudo terminal cloning device. */
> -#define _PATH_DEVPTMX _PATH_DEV "ptmx"
> /* Directory containing the UNIX98 pseudo terminals. */
> #define _PATH_DEVPTS _PATH_DEV "pts"
>
> +/* Path to the master pseudo terminal cloning device. */
> +#define _PATH_DEVPTMX _PATH_DEV "ptmx"
> +
> +/* Path to the master pseudo terminal cloning device under devpts mount. */
> +#define _PATH_DEVPTS_PTMX _PATH_DEVPTS "/ptmx"
> +
> /* Prototype for function that opens BSD-style master pseudo-terminals. */
> extern int __bsd_getpt (void) attribute_hidden;
>
> @@ -42,7 +46,15 @@ __posix_openpt (int oflag)
>
> if (!have_no_dev_ptmx)
> {
> - fd = __open (_PATH_DEVPTMX, oflag);
> + /* Try to open ptmx master pseudo terminal cloning device under the
> + * devpts mount.
> + */
> + fd = __open (_PATH_DEVPTS_PTMX, oflag);
> + if (fd == -1)
> + /* Fallback to opening the legacy ptmx master pseudo terminal
> + * cloning device.
> + */
> + fd = __open (_PATH_DEVPTMX, oflag);
> if (fd != -1)
> {
> struct statfs fsbuf;
> --
> 2.15.1
>
next prev parent reply other threads:[~2018-03-15 12:08 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-03-15 12:07 Christian Brauner
2018-03-15 12:08 ` Christian Brauner [this message]
2018-03-15 14:03 ` Zack Weinberg
2018-03-15 14:10 ` Christian Brauner
2018-03-15 14:38 ` Zack Weinberg
2018-03-15 14:49 ` Christian Brauner
[not found] ` <1521124913.18801.73.camel@pbcl.net>
2018-03-15 14:53 ` Christian Brauner
2018-03-15 20:03 ` Eric W. Biederman
2018-03-15 20:12 ` Christian Brauner
2018-03-15 20:28 ` 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=20180315120814.GA15013@gmail.com \
--to=christian.brauner@canonical.com \
--cc=christian.brauner@ubuntu.com \
--cc=ebiederm@xmission.com \
--cc=libc-alpha@sourceware.org \
/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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).