public inbox for libc-stable@sourceware.org
 help / color / mirror / Atom feed
* [2.26 COMMITTED] linux ttyname{_r}: Don't bail prematurely [BZ #22145]
@ 2017-01-01  0:00 Florian Weimer
  0 siblings, 0 replies; only message in thread
From: Florian Weimer @ 2017-01-01  0:00 UTC (permalink / raw)
  To: libc-stable

From: Luke Shumaker <lukeshu@parabola.nu>

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 <christian.brauner@ubuntu.com>
(cherry picked from commit a09dfc19edcbac3f96d5410529b724db0a583879)

2017-11-15  Luke Shumaker  <lukeshu@parabola.nu>

	[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.

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;
 }
 

^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2017-12-22 15:06 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-01  0:00 [2.26 COMMITTED] linux ttyname{_r}: Don't bail prematurely [BZ #22145] Florian Weimer

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).