From patchwork Wed Nov 15 21:33:14 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Christian Brauner X-Patchwork-Id: 24271 Received: (qmail 18471 invoked by alias); 15 Nov 2017 21:33:29 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 18417 invoked by uid 89); 15 Nov 2017 21:33:28 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.2 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_LAZY_DOMAIN_SECURITY, KB_WAM_FROM_NAME_SINGLEWORD, RCVD_IN_DNSWL_LOW autolearn=ham version=3.3.2 spammy=1955, life X-HELO: mx1.mailbox.org From: Christian Brauner To: libc-alpha@sourceware.org Cc: lukeshu@parabola.nu Subject: [PATCH COMMITTED 5/6] linux ttyname{_r}: Don't bail prematurely [BZ #22145] Date: Wed, 15 Nov 2017 22:33:14 +0100 Message-Id: <20171115213315.7300-5-christian.brauner@ubuntu.com> In-Reply-To: <20171115213315.7300-1-christian.brauner@ubuntu.com> References: <20171115213315.7300-1-christian.brauner@ubuntu.com> From: Luke Shumaker Commit 15e9a4f378c8607c2ae1aa465436af4321db0e23 introduced logic for ttyname() sending back ENODEV to signal that we can't get a name for the TTY because we inherited it from a different mount namespace. However, just because we inherited it from a different mount namespace and it isn't available at its original path, doesn't mean that its name is unknowable; we can still try to find it by allowing the normal fall back on iterating through devices. An example scenario where this happens is with "/dev/console" in containers. It's a common practice among container managers to allocate a PTY master/slave pair in the host's mount namespace (the slave having a path like "/dev/pty/$X"), bind mount the slave to "/dev/console" in the container's mount namespace, and send the slave FD to a process in the container. Inside of the container, the slave-end isn't available at its original path ("/dev/pts/$X"), since the container mount namespace has a separate devpts instance from the host (that path may or may not exist in the container; if it does exist, it's not the same PTY slave device). Currently ttyname{_r} sees that the file at the original "/dev/pts/$X" path doesn't match the FD passed to it, and fails early and gives up, even though if it kept searching it would find the TTY at "/dev/console". Fix that; don't have the ENODEV path force an early return inhibiting the fall-back search. This change is based on the previous patch that adds use of is_mytty in getttyname and getttyname_r. Without that change, this effectively reverts 15e9a4f, which made us disregard the false similarity of file pointed to by "/proc/self/fd/$Y", because if it doesn't bail prematurely then that file ("/dev/pts/$X") will just come up again anyway in the fall-back search. Reviewed-by: Christian Brauner --- ChangeLog | 5 +++++ sysdeps/unix/sysv/linux/ttyname.c | 19 ++++++++++++------- sysdeps/unix/sysv/linux/ttyname_r.c | 20 ++++++++++++-------- 3 files changed, 29 insertions(+), 15 deletions(-) diff --git a/ChangeLog b/ChangeLog index 00f388369f..566591047f 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,10 @@ 2017-11-15 Luke Shumaker + [BZ #22145] + * sysdeps/unix/sysv/linux/ttyname.c (ttyname): + Defer is_pty check until end of the function. + * sysdeps/unix/sysv/linux/ttyname_r.c (__ttyname_r): Likewise. + [BZ #22145] * sysdeps/unix/sysv/linux/ttyname.h (is_mytty): New function. * sysdeps/unix/sysv/linux/ttyname.c (getttyname): Call is_mytty. diff --git a/sysdeps/unix/sysv/linux/ttyname.c b/sysdeps/unix/sysv/linux/ttyname.c index 6e97d2d455..f4c955f25b 100644 --- a/sysdeps/unix/sysv/linux/ttyname.c +++ b/sysdeps/unix/sysv/linux/ttyname.c @@ -115,6 +115,7 @@ ttyname (int fd) char procname[30]; struct stat64 st, st1; int dostat = 0; + int doispty = 0; char *name; int save = errno; struct termios term; @@ -165,13 +166,7 @@ ttyname (int fd) && is_mytty (&st, &st1)) return ttyname_buf; - /* If the link doesn't exist, then it points to a device in another - namespace. */ - if (is_pty (&st)) - { - __set_errno (ENODEV); - return NULL; - } + doispty = 1; } if (__xstat64 (_STAT_VER, "/dev/pts", &st1) == 0 && S_ISDIR (st1.st_mode)) @@ -195,5 +190,15 @@ ttyname (int fd) name = getttyname ("/dev", &st, save, &dostat); } + if (!name && doispty && is_pty (&st)) + { + /* We failed to figure out the TTY's name, but we can at least + signal that we did verify that it really is a PTY slave. + This happens when we have inherited the file descriptor from + a different mount namespace. */ + __set_errno (ENODEV); + return NULL; + } + return name; } diff --git a/sysdeps/unix/sysv/linux/ttyname_r.c b/sysdeps/unix/sysv/linux/ttyname_r.c index 58eb919c3f..00eefc2c5c 100644 --- a/sysdeps/unix/sysv/linux/ttyname_r.c +++ b/sysdeps/unix/sysv/linux/ttyname_r.c @@ -95,6 +95,7 @@ __ttyname_r (int fd, char *buf, size_t buflen) char procname[30]; struct stat64 st, st1; int dostat = 0; + int doispty = 0; int save = errno; /* Test for the absolute minimal size. This makes life easier inside @@ -149,14 +150,7 @@ __ttyname_r (int fd, char *buf, size_t buflen) && is_mytty (&st, &st1)) return 0; - /* If the link doesn't exist, then it points to a device in another - * namespace. - */ - if (is_pty (&st)) - { - __set_errno (ENODEV); - return ENODEV; - } + doispty = 1; } /* Prepare the result buffer. */ @@ -190,6 +184,16 @@ __ttyname_r (int fd, char *buf, size_t buflen) save, &dostat); } + if (ret && doispty && is_pty (&st)) + { + /* We failed to figure out the TTY's name, but we can at least + signal that we did verify that it really is a PTY slave. + This happens when we have inherited the file descriptor from + a different mount namespace. */ + __set_errno (ENODEV); + return ENODEV; + } + return ret; }