From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933060AbdKACHD (ORCPT ); Tue, 31 Oct 2017 22:07:03 -0400 Received: from mail-qt0-f194.google.com ([209.85.216.194]:57064 "EHLO mail-qt0-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932210AbdKACHB (ORCPT ); Tue, 31 Oct 2017 22:07:01 -0400 X-Google-Smtp-Source: ABhQp+TOVs3h9qL59eVvmHnNLrEC2vEhHGsMOZfmTxdV8xtcSg18aKzivlkr69Qun7B8/N2gnIujTw== Date: Tue, 31 Oct 2017 19:06:51 -0700 From: Tejun Heo To: Greg Kroah-Hartman , Jiri Slaby , Alan Cox , Linus Torvalds Cc: linux-kernel@vger.kernel.org, kernel-team@fb.com Subject: [BUG] tty: Userland can create hung tasks Message-ID: <20171101020651.GK3252168@devbig577.frc2.facebook.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.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 #include #include #include #include #include #include #include #include #include #include #include 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