]> git.ipfire.org Git - thirdparty/kernel/stable.git/commitdiff
tty: make n_tty_read() always abort if hangup is in progress
authorTejun Heo <tj@kernel.org>
Tue, 13 Feb 2018 15:38:08 +0000 (07:38 -0800)
committerSasha Levin <alexander.levin@microsoft.com>
Wed, 23 May 2018 01:36:32 +0000 (21:36 -0400)
[ Upstream commit 28b0f8a6962a24ed21737578f3b1b07424635c9e ]

A tty is hung up by __tty_hangup() setting file->f_op to
hung_up_tty_fops, which is skipped on ttys whose write operation isn't
tty_write().  This means that, for example, /dev/console whose write
op is redirected_tty_write() is never actually marked hung up.

Because n_tty_read() uses the hung up status to decide whether to
abort the waiting readers, the lack of hung-up marking can lead to the
following scenario.

 1. A session contains two processes.  The leader and its child.  The
    child ignores SIGHUP.

 2. The leader exits and starts disassociating from the controlling
    terminal (/dev/console).

 3. __tty_hangup() skips setting f_op to hung_up_tty_fops.

 4. SIGHUP is delivered and ignored.

 5. tty_ldisc_hangup() is invoked.  It wakes up the waits which should
    clear the read lockers of tty->ldisc_sem.

 6. The reader wakes up but because tty_hung_up_p() is false, it
    doesn't abort and goes back to sleep while read-holding
    tty->ldisc_sem.

 7. The leader progresses to tty_ldisc_lock() in tty_ldisc_hangup()
    and is now stuck in D sleep indefinitely waiting for
    tty->ldisc_sem.

The following is Alan's explanation on why some ttys aren't hung up.

 http://lkml.kernel.org/r/20171101170908.6ad08580@alans-desktop

 1. It broke the serial consoles because they would hang up and close
    down the hardware. With tty_port that *should* be fixable properly
    for any cases remaining.

 2. The console layer was (and still is) completely broken and doens't
    refcount properly. So if you turn on console hangups it breaks (as
    indeed does freeing consoles and half a dozen other things).

As neither can be fixed quickly, this patch works around the problem
by introducing a new flag, TTY_HUPPING, which is used solely to tell
n_tty_read() that hang-up is in progress for the console and the
readers should be aborted regardless of the hung-up status of the
device.

The following is a sample hung task warning caused by this issue.

  INFO: task agetty:2662 blocked for more than 120 seconds.
        Not tainted 4.11.3-dbg-tty-lockup-02478-gfd6c7ee-dirty #28
  "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
      0  2662      1 0x00000086
  Call Trace:
   __schedule+0x267/0x890
   schedule+0x36/0x80
   schedule_timeout+0x23c/0x2e0
   ldsem_down_write+0xce/0x1f6
   tty_ldisc_lock+0x16/0x30
   tty_ldisc_hangup+0xb3/0x1b0
   __tty_hangup+0x300/0x410
   disassociate_ctty+0x6c/0x290
   do_exit+0x7ef/0xb00
   do_group_exit+0x3f/0xa0
   get_signal+0x1b3/0x5d0
   do_signal+0x28/0x660
   exit_to_usermode_loop+0x46/0x86
   do_syscall_64+0x9c/0xb0
   entry_SYSCALL64_slow_path+0x25/0x25

The following is the repro.  Run "$PROG /dev/console".  The parent
process hangs in D state.

  #include <sys/types.h>
  #include <sys/stat.h>
  #include <sys/wait.h>
  #include <sys/ioctl.h>
  #include <fcntl.h>
  #include <unistd.h>
  #include <stdio.h>
  #include <stdlib.h>
  #include <errno.h>
  #include <signal.h>
  #include <time.h>
  #include <termios.h>

  int main(int argc, char **argv)
  {
  struct sigaction sact = { .sa_handler = SIG_IGN };
  struct timespec ts1s = { .tv_sec = 1 };
  pid_t pid;
  int fd;

  if (argc < 2) {
  fprintf(stderr, "test-hung-tty /dev/$TTY\n");
  return 1;
  }

  /* fork a child to ensure that it isn't already the session leader */
  pid = fork();
  if (pid < 0) {
  perror("fork");
  return 1;
  }

  if (pid > 0) {
  /* top parent, wait for everyone */
  while (waitpid(-1, NULL, 0) >= 0)
  ;
  if (errno != ECHILD)
  perror("waitpid");
  return 0;
  }

  /* new session, start a new session and set the controlling tty */
  if (setsid() < 0) {
  perror("setsid");
  return 1;
  }

  fd = open(argv[1], O_RDWR);
  if (fd < 0) {
  perror("open");
  return 1;
  }

  if (ioctl(fd, TIOCSCTTY, 1) < 0) {
  perror("ioctl");
  return 1;
  }

  /* fork a child, sleep a bit and exit */
  pid = fork();
  if (pid < 0) {
  perror("fork");
  return 1;
  }

  if (pid > 0) {
  nanosleep(&ts1s, NULL);
  printf("Session leader exiting\n");
  exit(0);
  }

  /*
   * The child ignores SIGHUP and keeps reading from the controlling
   * tty.  Because SIGHUP is ignored, the child doesn't get killed on
   * parent exit and the bug in n_tty makes the read(2) block the
   * parent's control terminal hangup attempt.  The parent ends up in
   * D sleep until the child is explicitly killed.
   */
  sigaction(SIGHUP, &sact, NULL);
  printf("Child reading tty\n");
  while (1) {
  char buf[1024];

  if (read(fd, buf, sizeof(buf)) < 0) {
  perror("read");
  return 1;
  }
  }

  return 0;
  }

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Alan Cox <alan@llwyncelyn.cymru>
Cc: stable@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Sasha Levin <alexander.levin@microsoft.com>
drivers/tty/n_tty.c
drivers/tty/tty_io.c
include/linux/tty.h

index 66e257b5a5b75fd4b7d465bd04aaaa81f2fc60e8..4693a1d0151f131045684e55d0b1479a4cd3b7d7 100644 (file)
@@ -2259,6 +2259,12 @@ static ssize_t n_tty_read(struct tty_struct *tty, struct file *file,
                                }
                                if (tty_hung_up_p(file))
                                        break;
+                               /*
+                                * Abort readers for ttys which never actually
+                                * get hung up.  See __tty_hangup().
+                                */
+                               if (test_bit(TTY_HUPPING, &tty->flags))
+                                       break;
                                if (!timeout)
                                        break;
                                if (file->f_flags & O_NONBLOCK) {
index be96970646a966dd432170a4e7df5e36db71a9b3..483757d7505e757e8a675455287f2b7695ca7c89 100644 (file)
@@ -690,6 +690,14 @@ static void __tty_hangup(struct tty_struct *tty, int exit_session)
                return;
        }
 
+       /*
+        * Some console devices aren't actually hung up for technical and
+        * historical reasons, which can lead to indefinite interruptible
+        * sleep in n_tty_read().  The following explicitly tells
+        * n_tty_read() to abort readers.
+        */
+       set_bit(TTY_HUPPING, &tty->flags);
+
        /* inuse_filps is protected by the single tty lock,
           this really needs to change if we want to flush the
           workqueue with the lock held */
@@ -745,6 +753,7 @@ static void __tty_hangup(struct tty_struct *tty, int exit_session)
         * can't yet guarantee all that.
         */
        set_bit(TTY_HUPPED, &tty->flags);
+       clear_bit(TTY_HUPPING, &tty->flags);
        tty_unlock(tty);
 
        if (f)
index 52baf4089bd26c89ab99864de7270e3830d3a1a2..5ed0055088483532ed3248efb9db1d6f2eec6936 100644 (file)
@@ -343,6 +343,7 @@ struct tty_file_private {
 #define TTY_PTY_LOCK           16      /* pty private */
 #define TTY_NO_WRITE_SPLIT     17      /* Preserve write boundaries to driver */
 #define TTY_HUPPED             18      /* Post driver->hangup() */
+#define TTY_HUPPING            19      /* Hangup in progress */
 #define TTY_LDISC_HALTED       22      /* Line discipline is halted */
 
 #define TTY_WRITE_FLUSH(tty) tty_write_flush((tty))