This is the mail archive of the glibc-bugs@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[Bug libc/22145] ttyname() gives up too early in the face of namespaces


https://sourceware.org/bugzilla/show_bug.cgi?id=22145

--- Comment #12 from Christian Brauner <christian.brauner at mailbox dot org> ---
Comment on attachment 10416
  --> https://sourceware.org/bugzilla/attachment.cgi?id=10416
proposed fix part 2

>From f9209704ded0240b39c7b1157d9294d88aab3591 Mon Sep 17 00:00:00 2001
>From: Luke Shumaker <lukeshu@lukeshu.com>
>Date: Sun, 17 Sep 2017 03:02:07 -0400
>Subject: [PATCH 2/2] linux ttyname() and ttyname_r(): Fix namespace check
>
>In commit 15e9a4f Christian Brauner 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 find it by allowing the normal fall back on
>iterating through devices.
>
>  This is the case for systemd-nspawn, which arranges for the TTY to be at
>  /dev/console in the new mount namespace.  Christian's check meant that
>  Bash has broken job control when run directly in systemd-nspawn.

I know this is on the verge of bikeshedding but please drop the reference
to systemd-nspawn and provide a generic example for the /dev/console case.
This makes it clearer that this is a general problem and avoids mentioning
tools that might change behavior at any point. Maybe something along the
lines of:

"A common scenario where this happens is with /dev/console in containers.
Usually container runtimes/managers will call openpty() on a ptmx device in the
host's mount namespace to safely allocate a {p,t}ty master-slave pair since
they
can't trust the container's devpts mount after the container's init binary has
started (potentially malicious fuse mounts and what not). The slave {p,t}ty fd
will then usually be sent to the container and bind-mounted over the
container's
/dev/console which in this scenario is simply a regular file. This is
especially
common with unprivileged containers where mknod() syscalls are not possible.
This
patch enables ttyname{_r}() to correctly report that /dev/console does in fact
refer to a {p,t}ty device whose path exists in the current mount namespace but
whose origin is a devpts mount in a different mount namespace."

>---
> sysdeps/unix/sysv/linux/ttyname.c   | 19 ++++++++++++-------
> sysdeps/unix/sysv/linux/ttyname_r.c | 20 ++++++++++++--------
> 2 files changed, 24 insertions(+), 15 deletions(-)
>
>diff --git a/sysdeps/unix/sysv/linux/ttyname.c b/sysdeps/unix/sysv/linux/ttyname.c
>index ba046205ae..509009bdda 100644
>--- a/sysdeps/unix/sysv/linux/ttyname.c
>+++ b/sysdeps/unix/sysv/linux/ttyname.c
>@@ -118,6 +118,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;
>@@ -168,13 +169,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))
>@@ -198,5 +193,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.  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 8ff8653a2d..9dde9f7443 100644
>--- a/sysdeps/unix/sysv/linux/ttyname_r.c
>+++ b/sysdeps/unix/sysv/linux/ttyname_r.c
>@@ -96,6 +96,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
>@@ -150,14 +151,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.  */
>@@ -191,6 +185,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.  This
>+       * happens when we have inherited the file descriptor from a
>+       * different mount namespace. */
>+      __set_errno (ENODEV);
>+      return ENODEV;
>+    }
>+
>   return ret;
> }
> 
>-- 
>2.14.1
>

-- 
You are receiving this mail because:
You are on the CC list for the bug.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]