Merge remote-tracking branch 'origin/issue462'

* origin/issue462:
  issue #462: docs: update Changelog.
  parent: cope with broken /dev/pts on Linux; closes #462.

docs/changelog.rst

@@ -303,6 +303,12 @@ Core Library
   shut down, preventing new messages being enqueued that will never be sent,
   and subsequently producing a program hang.
 
+* `#462 <https://github.com/dw/mitogen/issues/462>`_: Mitogen could fail to
+  open a PTY on broken Linux systems due to a bad interaction between the glibc
+  :func:`grantpt` function and an incorrectly mounted ``/dev/pts`` filesystem.
+  Since correct group ownership is not required in most scenarios, when this
+  problem is detected, the PTY is allocated and opened directly by the library.
+
 * `16ca111e <https://github.com/dw/mitogen/commit/16ca111e>`_: handle OpenSSH
   7.5 permission denied prompts when ``~/.ssh/config`` rewrites are present.
 
@@ -341,8 +347,10 @@ bug reports, testing, features and fixes in this release contributed by
 `Mehdi <https://github.com/mehdisat7>`_,
 `Michael DeHaan <https://github.com/mpdehaan>`_,
 `Mohammed Naser <https://github.com/mnaser/>`_,
-`@syntonym <https://github.com/syntonym/>`_, and
+`Stéphane <https://github.com/sboisson/>`_,
-`@yodatak <https://github.com/yodatak/>`_.
+`@syntonym <https://github.com/syntonym/>`_,
+`@yodatak <https://github.com/yodatak/>`_, and
+`Younès HAFRI <https://github.com/yhafri>`_.
 
 
 v0.2.3 (2018-10-23)

mitogen/parent.py

@@ -41,6 +41,7 @@ import logging
 import os
 import signal
 import socket
+import struct
 import subprocess
 import sys
 import termios
@@ -97,6 +98,10 @@ SYS_EXECUTABLE_MSG = (
 )
 _sys_executable_warning_logged = False
 
+LINUX_TIOCGPTN = 2147767344  # Get PTY number; asm-generic/ioctls.h
+LINUX_TIOCSPTLCK = 1074025521  # Lock/unlock PTY; asm-generic/ioctls.h
+IS_LINUX = os.uname()[0] == 'Linux'
+
 SIGNAL_BY_NUM = dict(
     (getattr(signal, name), name)
     for name in sorted(vars(signal), reverse=True)
@@ -318,6 +323,48 @@ def _acquire_controlling_tty():
         fcntl.ioctl(2, termios.TIOCSCTTY)
 
 
+def _linux_broken_devpts_openpty():
+    """
+    #462: On broken Linux hosts with mismatched configuration (e.g. old
+    /etc/fstab template installed), /dev/pts may be mounted without the gid=
+    mount option, causing new slave devices to be created with the group ID of
+    the calling process. This upsets glibc, whose openpty() is required by
+    specification to produce a slave owned by a special group ID (which is
+    always the 'tty' group).
+
+    Glibc attempts to use "pt_chown" to fix ownership. If that fails, it
+    chown()s the PTY directly, which fails due to non-root, causing openpty()
+    to fail with EPERM ("Operation not permitted"). Since we don't need the
+    magical TTY group to run sudo and su, open the PTY ourselves in this case.
+    """
+    master_fd = None
+    try:
+        # Opening /dev/ptmx causes a PTY pair to be allocated, and the
+        # corresponding slave /dev/pts/* device to be created, owned by UID/GID
+        # matching this process.
+        master_fd = os.open('/dev/ptmx', os.O_RDWR)
+        # Clear the lock bit from the PTY. This a prehistoric feature from a
+        # time when slave device files were persistent.
+        fcntl.ioctl(master_fd, LINUX_TIOCSPTLCK, struct.pack('i', 0))
+        # Since v4.13 TIOCGPTPEER exists to open the slave in one step, but we
+        # must support older kernels. Ask for the PTY number.
+        pty_num_s = fcntl.ioctl(master_fd, LINUX_TIOCGPTN,
+                                struct.pack('i', 0))
+        pty_num, = struct.unpack('i', pty_num_s)
+        pty_name = '/dev/pts/%d' % (pty_num,)
+        # Now open it with O_NOCTTY to ensure it doesn't change our controlling
+        # TTY. Otherwise when we close the FD we get killed by the kernel, and
+        # the child we spawn that should really attach to it will get EPERM
+        # during _acquire_controlling_tty().
+        slave_fd = os.open(pty_name, os.O_RDWR|os.O_NOCTTY)
+        return master_fd, slave_fd
+    except OSError:
+        if master_fd is not None:
+            os.close(master_fd)
+        e = sys.exc_info()[1]
+        raise mitogen.core.StreamError(OPENPTY_MSG, e)
+
+
 def openpty():
     """
     Call :func:`os.openpty`, raising a descriptive error if the call fails.
@@ -331,6 +378,8 @@ def openpty():
         return os.openpty()
     except OSError:
         e = sys.exc_info()[1]
+        if IS_LINUX and e.args[0] == errno.EPERM:
+            return _linux_broken_devpts_openpty()
         raise mitogen.core.StreamError(OPENPTY_MSG, e)
 
 

tests/parent_test.py

@@ -1,4 +1,5 @@
 import errno
+import fcntl
 import os
 import signal
 import subprocess
@@ -197,6 +198,26 @@ class OpenPtyTest(testlib.TestCase):
         msg = mitogen.parent.OPENPTY_MSG % (openpty.side_effect,)
         self.assertEquals(e.args[0], msg)
 
+    @unittest2.skipIf(condition=(os.uname()[0] != 'Linux'),
+                      reason='Fallback only supported on Linux')
+    @mock.patch('os.openpty')
+    def test_broken_linux_fallback(self, openpty):
+        openpty.side_effect = OSError(errno.EPERM)
+        master_fd, slave_fd = self.func()
+        try:
+            st = os.fstat(master_fd)
+            self.assertEquals(5, os.major(st.st_rdev))
+            flags = fcntl.fcntl(master_fd, fcntl.F_GETFL)
+            self.assertTrue(flags & os.O_RDWR)
+
+            st = os.fstat(slave_fd)
+            self.assertEquals(136, os.major(st.st_rdev))
+            flags = fcntl.fcntl(slave_fd, fcntl.F_GETFL)
+            self.assertTrue(flags & os.O_RDWR)
+        finally:
+            os.close(master_fd)
+            os.close(slave_fd)
+
 
 class TtyCreateChildTest(testlib.TestCase):
     func = staticmethod(mitogen.parent.tty_create_child)