Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752808AbbBQV2h (ORCPT ); Tue, 17 Feb 2015 16:28:37 -0500 Received: from mail-qg0-f48.google.com ([209.85.192.48]:58372 "EHLO mail-qg0-f48.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751596AbbBQV2f (ORCPT ); Tue, 17 Feb 2015 16:28:35 -0500 Message-ID: <54E3B27E.9020506@hurleysoftware.com> Date: Tue, 17 Feb 2015 16:28:30 -0500 From: Peter Hurley User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.4.0 MIME-Version: 1.0 To: Aristeu Rozanski CC: linux-kernel@vger.kernel.org, Greg Kroah-Hartman , Jiri Slaby Subject: Re: [PATCH] n_tty_read: check for hanging tty while waiting for input References: <20150217210609.GA13666@redhat.com> In-Reply-To: <20150217210609.GA13666@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5608 Lines: 122 On 02/17/2015 04:06 PM, Aristeu Rozanski wrote: > If the console has a canonical reader and the respective tty hangs up, > it'll waste a wake up and will never release the last ldisc reference so > the hangup process can finish: This behavior is by-design; /dev/console cannot be hung-up. > n_tty_read(): > (..) > add_wait_queue(&tty->read_wait, &wait); > while (nr) { > (..) > if (!input_available_p(tty, 0)) { > if (test_bit(TTY_OTHER_CLOSED, &tty->flags)) { > up_read(&tty->termios_rwsem); > tty_flush_to_ldisc(tty); > down_read(&tty->termios_rwsem); > if (!input_available_p(tty, 0)) { > retval = -EIO; > break; > } > } else { > -> if (tty_hung_up_p(file)) > break; > this won't work because file->f_op never gets set to &hung_up_tty_fops: > __tty_hangup(): > > spin_lock(&tty_files_lock); > /* This breaks for file handles being sent over AF_UNIX sockets ? */ > list_for_each_entry(priv, &tty->tty_files, list) { > filp = priv->file; > if (filp->f_op->write == redirected_tty_write) > cons_filp = filp; > -> if (filp->f_op->write != tty_write) > -> continue; > closecount++; > __tty_fasync(-1, filp, 0); /* can't block */ > -> filp->f_op = &hung_up_tty_fops; > } > spin_unlock(&tty_files_lock); > > refs = tty_signal_session_leader(tty, exit_session); > /* Account for the p->signal references we killed */ > while (refs--) > tty_kref_put(tty); > > /* > * it drops BTM and thus races with reopen > * we protect the race by TTY_HUPPING > */ > -> tty_ldisc_hangup(tty); > > So while the canonical read waits for input, it'll sleep, be awaken by > tty_ldisc_hangup() and then immediately going back to sleep without > dropping the reference to the ldisc gained on tty_read(). This isn't > noticiable in a non canonical read due that it'll eventually timeout. > > The proposed patch checks for TTY_HUPPING flag in order to leave if > there's no input. > > This is easily reproduced by opening /dev/console (my test case was a > virtual machine with serial console), setting as canonical and waiting > on a read(). Then, in another session, killing agetty that is running on > ttyS0 which will issue a hangup. What process is sleeping on /dev/console read() and what is its controlling tty? I ask because console teardown usually happens when SIGHUP is received by the process group. > [ 240.439045] INFO: task (agetty):1323 blocked for more than 120 seconds. > [ 240.439569] Not tainted 3.13.0-rc3+ #11 > [ 240.439972] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. > [ 240.440596] (agetty) D ffff88007fd94440 0 1323 1 0x00000080 > [ 240.441253] ffff88007bca1c50 0000000000000086 ffff88007989b0c0 0000000000014440 > [ 240.441857] ffff88007bca1fd8 0000000000014440 ffff88007989b0c0 ffff88007989b0c0 > [ 240.442561] ffff88007ad46c30 7fffffffffffffff 0000000000000001 ffff88007ad46c28 > [ 240.443296] Call Trace: > [ 240.443506] [] schedule+0x29/0x70 > [ 240.443883] [] schedule_timeout+0x209/0x2d0 > [ 240.444395] [] ? check_preempt_curr+0x85/0xa0 > [ 240.444850] [] ? ttwu_do_wakeup+0x19/0xd0 > [ 240.445343] [] ? ttwu_do_activate.constprop.80+0x5d/0x70 > [ 240.445868] [] ? try_to_wake_up+0xeb/0x2b0 > [ 240.446363] [] ldsem_down_write+0xda/0x227 > [ 240.446797] [] ? default_wake_function+0x12/0x20 > [ 240.447359] [] tty_ldisc_lock_pair_timeout+0x7d/0x100 > [ 240.447861] [] tty_ldisc_hangup+0xc9/0x220 > [ 240.448355] [] __tty_hangup+0x363/0x4b0 > [ 240.448768] [] tty_ioctl+0x865/0xbb0 > [ 240.449219] [] ? do_filp_open+0x3a/0x90 > [ 240.449634] [] do_vfs_ioctl+0x2e0/0x4c0 > [ 240.450066] [] ? file_has_perm+0x86/0xa0 > [ 240.450543] [] SyS_ioctl+0x81/0xa0 > [ 240.450921] [] system_call_fastpath+0x16/0x1b > > Cc: Greg Kroah-Hartman > Cc: Jiri Slaby > Cc: Peter Hurley > Signed-off-by: Aristeu Rozanski > > diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c > index 0f74945..4fb909d 100644 > --- a/drivers/tty/n_tty.c > +++ b/drivers/tty/n_tty.c > @@ -2189,6 +2189,8 @@ static ssize_t n_tty_read(struct tty_struct *tty, struct file *file, > } else { > if (tty_hung_up_p(file)) > break; > + if (test_bit(TTY_HUPPING, &tty->flags)) > + break; > if (!timeout) > break; > if (file->f_flags & O_NONBLOCK) { > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/