[BUG] tty: Userland can create hung tasks

[Tejun Heo <tj@kernel.org>]

Hello,

tty hangup code doesn't mark the console as being HUPed for, e.g.,
/dev/console and that can put the session leader trying to
disassociate from the controlling terminal in an indefinite D sleep.

Looking at the code, I have no idea why some tty devices are never
marked being hung up.  It *looks* intentional and dates back to the
git origin but I couldn't find any clue.  The following patch is a
workaround which fixes the observed problem but definitely isn't the
proper fix.

For details, please read the patch description.  If you scroll down,
there's a reproducer too.

Thanks.

------ 8< ------
Subject: [PATCH] tty: make n_tty_read() always abort if hangup is in progress

__tty_hangup() sets file->f_op to hung_up_tty_fops iff the write
operation is tty_write(), which means that, for example, hanging up
/dev/console doesn't clear its f_op as its write op is
redirected_tty_write().

tty_hung_up_p() tests f_op for hung_up_tty_fops to determine whether
the terminal is (being) hung up.  In turn, n_tty_read() uses this test
to decide whether readers should abort due to hangup.

Combined, this means that n_tty_read() can't tell whether /dev/console
is being hung up or not.  This 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.

This leads to hung task warnings like the following.

  [  492.713289] INFO: task agetty:2662 blocked for more than 120 seconds.
  [  492.726170]       Not tainted 4.11.3-dbg-tty-lockup-02478-gfd6c7ee-dirty #28
  [  492.740264] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
  [  492.755919]     0  2662      1 0x00000086
  [  492.763940] Call Trace:
  [  492.768834]  __schedule+0x267/0x890
  [  492.775816]  schedule+0x36/0x80
  [  492.782094]  schedule_timeout+0x23c/0x2e0
  [  492.790120]  ldsem_down_write+0xce/0x1f6
  [  492.797974]  tty_ldisc_lock+0x16/0x30
  [  492.805300]  tty_ldisc_hangup+0xb3/0x1b0
  [  492.813143]  __tty_hangup+0x300/0x410
  [  492.820470]  disassociate_ctty+0x6c/0x290
  [  492.828486]  do_exit+0x7ef/0xb00
  [  492.834946]  do_group_exit+0x3f/0xa0
  [  492.842092]  get_signal+0x1b3/0x5d0
  [  492.849077]  do_signal+0x28/0x660
  [  492.855720]  ? __fput+0x174/0x1e0
  [  492.862353]  ? __audit_syscall_exit+0x1f3/0x280
  [  492.871402]  exit_to_usermode_loop+0x46/0x86
  [  492.879926]  do_syscall_64+0x9c/0xb0
  [  492.887073]  entry_SYSCALL64_slow_path+0x25/0x25
  [  492.896295] RIP: 0033:0x7f69b3e7f783
  [  492.903438] RSP: 002b:00007ffdcb249ca8 EFLAGS: 00000246
  [  492.913868]  ORIG_RAX: 0000000000000017
  [  492.921536] RAX: fffffffffffffdfe RBX: 00007ffdcb249cd0 RCX: 00007f69b3e7f783
  [  492.935786] RDX: 0000000000000000 RSI: 00007ffdcb249da0 RDI: 0000000000000005
  [  492.950034] RBP: 00007ffdcb249e20 R08: 0000000000000000 R09: 00007ffdcb249c60
  [  492.964284] R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
  [  492.964285] R13: 0000000000000004 R14: 0000000000000100 R15: 00007ffdcb24b750

This patch works around the issue by marking that hangup is in
progress in tty->flags regardless of the tty type and make
n_tty_read() abort accordingly.  This isn't a proper fix but does work
around the observed problem.

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

WORKAROUND_NOT_SIGNED_OFF
---
 drivers/tty/n_tty.c  | 3 ++-
 drivers/tty/tty_io.c | 3 +++
 include/linux/tty.h  | 1 +
 3 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index bdf0e6e..cb1e356 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -2180,7 +2180,8 @@ static ssize_t n_tty_read(struct tty_struct *tty, struct file *file,
 					retval = -EIO;
 					break;
 				}
-				if (tty_hung_up_p(file))
+				if (tty_hung_up_p(file) ||
+				    test_bit(TTY_HUPPING, &tty->flags))
 					break;
 				if (!timeout)
 					break;
diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index e6d1a65..012ac8a 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -710,6 +710,8 @@ static void __tty_hangup(struct tty_struct *tty, int exit_session)
 		return;
 	}
 
+	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 */
@@ -764,6 +766,7 @@ static void __tty_hangup(struct tty_struct *tty, int exit_session)
 	 * from the ldisc side, which is now guaranteed.
 	 */
 	set_bit(TTY_HUPPED, &tty->flags);
+	clear_bit(TTY_HUPPING, &tty->flags);
 	tty_unlock(tty);
 
 	if (f)
diff --git a/include/linux/tty.h b/include/linux/tty.h
index 1017e904..bce2765 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -362,6 +362,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 */
 
 /* Values for tty->flow_change */
-- 
2.9.5