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]  [<ffffffff815c8c99>] schedule+0x29/0x70
>       [  240.443883]  [<ffffffff815c7f59>] schedule_timeout+0x209/0x2d0
>       [  240.444395]  [<ffffffff810974b5>] ? check_preempt_curr+0x85/0xa0
>       [  240.444850]  [<ffffffff810974e9>] ? ttwu_do_wakeup+0x19/0xd0
>       [  240.445343]  [<ffffffff8109764d>] ? 
> ttwu_do_activate.constprop.80+0x5d/0x70
>       [  240.445868]  [<ffffffff810995eb>] ? try_to_wake_up+0xeb/0x2b0
>       [  240.446363]  [<ffffffff815cbdaa>] ldsem_down_write+0xda/0x227
>       [  240.446797]  [<ffffffff81099822>] ? default_wake_function+0x12/0x20
>       [  240.447359]  [<ffffffff815cc43d>] 
> tty_ldisc_lock_pair_timeout+0x7d/0x100
>       [  240.447861]  [<ffffffff8136e519>] tty_ldisc_hangup+0xc9/0x220
>       [  240.448355]  [<ffffffff81365463>] __tty_hangup+0x363/0x4b0
>       [  240.448768]  [<ffffffff81367cc5>] tty_ioctl+0x865/0xbb0
>       [  240.449219]  [<ffffffff811bb52a>] ? do_filp_open+0x3a/0x90
>       [  240.449634]  [<ffffffff811bd900>] do_vfs_ioctl+0x2e0/0x4c0
>       [  240.450066]  [<ffffffff8124ea76>] ? file_has_perm+0x86/0xa0
>       [  240.450543]  [<ffffffff811bdb61>] SyS_ioctl+0x81/0xa0
>       [  240.450921]  [<ffffffff815d4b69>] system_call_fastpath+0x16/0x1b
> 
> Cc: Greg Kroah-Hartman <[email protected]>
> Cc: Jiri Slaby <[email protected]>
> Cc: Peter Hurley <[email protected]>
> Signed-off-by: Aristeu Rozanski <[email protected]>
> 
> 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 [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to