lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Tue, 13 Feb 2018 17:35:33 +0100 From: Greg Kroah-Hartman <gregkh@...uxfoundation.org> To: Tejun Heo <tj@...nel.org> Cc: Jiri Slaby <jslaby@...e.com>, Alan Cox <alan@...yncelyn.cymru>, Linus Torvalds <torvalds@...ux-foundation.org>, linux-kernel@...r.kernel.org, kernel-team@...com Subject: Re: [PATCH] tty: make n_tty_read() always abort if hangup is in progress On Tue, Feb 13, 2018 at 07:38:08AM -0800, Tejun Heo wrote: > 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@...nel.org> > Cc: Alan Cox <alan@...yncelyn.cymru> > Cc: stable@...r.kernel.org > --- > Hello, > > This patch was initially posted several months ago as a sort of RFC. > > http://lkml.kernel.org/r/20171101020651.GK3252168@devbig577.frc2.facebook.com > > IIUC Alan's explanation correctly, this workaround actually seems like > the right thing to do, so I refreshed the patch, added some comments > and updated the patch description accordingly. > > We've been running this in the fleet for the past couple months to > avoid the process hang issues and it hasn't caused any issues. Greg, > can you please pick it up? Yes, I will, give me a few days to catch up on my patch queue. thanks, greg k-h
Powered by blists - more mailing lists