linux-stable.git/drivers/tty/pty.c, branch linux-3.13.y

tty: Fix lock order in tty_do_resize()

2013-07-24T22:12:53+00:00

Commits 6a1c0680cf3ba94356ecd58833e1540c93472a57 and
9356b535fcb71db494fc434acceb79f56d15bda2, respectively
  'tty: Convert termios_mutex to termios_rwsem' and
  'n_tty: Access termios values safely'
introduced a circular lock dependency with console_lock and
termios_rwsem.

The lockdep report [1] shows that n_tty_write() will attempt
to claim console_lock while holding the termios_rwsem, whereas
tty_do_resize() may already hold the console_lock while
claiming the termios_rwsem.

Since n_tty_write() and tty_do_resize() do not contend
over the same data -- the tty->winsize structure -- correct
the lock dependency by introducing a new lock which
specifically serializes access to tty->winsize only.

[1] Lockdep report

======================================================
[ INFO: possible circular locking dependency detected ]
3.10.0-0+tip-xeon+lockdep #0+tip Not tainted
-------------------------------------------------------
modprobe/277 is trying to acquire lock:
 (&tty->termios_rwsem){++++..}, at: [] tty_do_resize+0x36/0xe0

but task is already holding lock:
 ((fb_notifier_list).rwsem){.+.+.+}, at: [] __blocking_notifier_call_chain+0x56/0xc0

which lock already depends on the new lock.

the existing dependency chain (in reverse order) is:

-> #2 ((fb_notifier_list).rwsem){.+.+.+}:
       [] lock_acquire+0x92/0x1f0
       [] down_read+0x47/0x5c
       [] __blocking_notifier_call_chain+0x56/0xc0
       [] blocking_notifier_call_chain+0x16/0x20
       [] fb_notifier_call_chain+0x1b/0x20
       [] register_framebuffer+0x1e2/0x320
       [] drm_fb_helper_initial_config+0x371/0x540 [drm_kms_helper]
       [] nouveau_fbcon_init+0x105/0x140 [nouveau]
       [] nouveau_drm_load+0x43f/0x610 [nouveau]
       [] drm_get_pci_dev+0x17e/0x2a0 [drm]
       [] nouveau_drm_probe+0x25a/0x2a0 [nouveau]
       [] local_pci_probe+0x4b/0x80
       [] pci_device_probe+0x111/0x120
       [] driver_probe_device+0x8b/0x3a0
       [] __driver_attach+0xab/0xb0
       [] bus_for_each_dev+0x5d/0xa0
       [] driver_attach+0x1e/0x20
       [] bus_add_driver+0x111/0x290
       [] driver_register+0x77/0x170
       [] __pci_register_driver+0x64/0x70
       [] drm_pci_init+0x11a/0x130 [drm]
       [] nouveau_drm_init+0x4d/0x1000 [nouveau]
       [] do_one_initcall+0xea/0x1a0
       [] load_module+0x123b/0x1bf0
       [] SyS_init_module+0xd7/0x120
       [] system_call_fastpath+0x16/0x1b

-> #1 (console_lock){+.+.+.}:
       [] lock_acquire+0x92/0x1f0
       [] console_lock+0x77/0x80
       [] con_flush_chars+0x31/0x50
       [] n_tty_write+0x1ec/0x4d0
       [] tty_write+0x159/0x2e0
       [] redirected_tty_write+0xb5/0xc0
       [] vfs_write+0xc5/0x1f0
       [] SyS_write+0x55/0xa0
       [] system_call_fastpath+0x16/0x1b

-> #0 (&tty->termios_rwsem){++++..}:
       [] __lock_acquire+0x1c43/0x1d30
       [] lock_acquire+0x92/0x1f0
       [] down_write+0x44/0x70
       [] tty_do_resize+0x36/0xe0
       [] vc_do_resize+0x3e1/0x4c0
       [] vc_resize+0x1f/0x30
       [] fbcon_init+0x385/0x5a0
       [] visual_init+0xbc/0x120
       [] do_bind_con_driver+0x163/0x320
       [] do_take_over_console+0x61/0x70
       [] do_fbcon_takeover+0x63/0xc0
       [] fbcon_event_notify+0x715/0x820
       [] notifier_call_chain+0x5d/0x110
       [] __blocking_notifier_call_chain+0x6c/0xc0
       [] blocking_notifier_call_chain+0x16/0x20
       [] fb_notifier_call_chain+0x1b/0x20
       [] register_framebuffer+0x1e2/0x320
       [] drm_fb_helper_initial_config+0x371/0x540 [drm_kms_helper]
       [] nouveau_fbcon_init+0x105/0x140 [nouveau]
       [] nouveau_drm_load+0x43f/0x610 [nouveau]
       [] drm_get_pci_dev+0x17e/0x2a0 [drm]
       [] nouveau_drm_probe+0x25a/0x2a0 [nouveau]
       [] local_pci_probe+0x4b/0x80
       [] pci_device_probe+0x111/0x120
       [] driver_probe_device+0x8b/0x3a0
       [] __driver_attach+0xab/0xb0
       [] bus_for_each_dev+0x5d/0xa0
       [] driver_attach+0x1e/0x20
       [] bus_add_driver+0x111/0x290
       [] driver_register+0x77/0x170
       [] __pci_register_driver+0x64/0x70
       [] drm_pci_init+0x11a/0x130 [drm]
       [] nouveau_drm_init+0x4d/0x1000 [nouveau]
       [] do_one_initcall+0xea/0x1a0
       [] load_module+0x123b/0x1bf0
       [] SyS_init_module+0xd7/0x120
       [] system_call_fastpath+0x16/0x1b

other info that might help us debug this:

Chain exists of:
  &tty->termios_rwsem --> console_lock --> (fb_notifier_list).rwsem

 Possible unsafe locking scenario:

       CPU0                    CPU1
       ----                    ----
  lock((fb_notifier_list).rwsem);
                               lock(console_lock);
                               lock((fb_notifier_list).rwsem);
  lock(&tty->termios_rwsem);

 *** DEADLOCK ***

7 locks held by modprobe/277:
 #0:  (&__lockdep_no_validate__){......}, at: [] __driver_attach+0x5b/0xb0
 #1:  (&__lockdep_no_validate__){......}, at: [] __driver_attach+0x69/0xb0
 #2:  (drm_global_mutex){+.+.+.}, at: [] drm_get_pci_dev+0xbd/0x2a0 [drm]
 #3:  (registration_lock){+.+.+.}, at: [] register_framebuffer+0x25/0x320
 #4:  (&fb_info->lock){+.+.+.}, at: [] lock_fb_info+0x26/0x60
 #5:  (console_lock){+.+.+.}, at: [] register_framebuffer+0x1d4/0x320
 #6:  ((fb_notifier_list).rwsem){.+.+.+}, at: [] __blocking_notifier_call_chain+0x56/0xc0

stack backtrace:
CPU: 0 PID: 277 Comm: modprobe Not tainted 3.10.0-0+tip-xeon+lockdep #0+tip
Hardware name: Dell Inc. Precision WorkStation T5400  /0RW203, BIOS A11 04/30/2012
 ffffffff8213e5e0 ffff8802aa2fb298 ffffffff81755f19 ffff8802aa2fb2e8
 ffffffff8174f506 ffff8802aa2fa000 ffff8802aa2fb378 ffff8802aa2ea8e8
 ffff8802aa2ea910 ffff8802aa2ea8e8 0000000000000006 0000000000000007
Call Trace:
 [] dump_stack+0x19/0x1b
 [] print_circular_bug+0x1fb/0x20c
 [] __lock_acquire+0x1c43/0x1d30
 [] ? mark_held_locks+0xae/0x120
 [] ? trace_hardirqs_on_caller+0x105/0x1d0
 [] lock_acquire+0x92/0x1f0
 [] ? tty_do_resize+0x36/0xe0
 [] down_write+0x44/0x70
 [] ? tty_do_resize+0x36/0xe0
 [] tty_do_resize+0x36/0xe0
 [] vc_do_resize+0x3e1/0x4c0
 [] vc_resize+0x1f/0x30
 [] fbcon_init+0x385/0x5a0
 [] visual_init+0xbc/0x120
 [] do_bind_con_driver+0x163/0x320
 [] do_take_over_console+0x61/0x70
 [] do_fbcon_takeover+0x63/0xc0
 [] fbcon_event_notify+0x715/0x820
 [] notifier_call_chain+0x5d/0x110
 [] __blocking_notifier_call_chain+0x6c/0xc0
 [] blocking_notifier_call_chain+0x16/0x20
 [] fb_notifier_call_chain+0x1b/0x20
 [] register_framebuffer+0x1e2/0x320
 [] drm_fb_helper_initial_config+0x371/0x540 [drm_kms_helper]
 [] ? kmemleak_alloc+0x5b/0xc0
 [] ? kmem_cache_alloc_trace+0x104/0x290
 [] ? drm_fb_helper_single_add_all_connectors+0x81/0xf0 [drm_kms_helper]
 [] nouveau_fbcon_init+0x105/0x140 [nouveau]
 [] nouveau_drm_load+0x43f/0x610 [nouveau]
 [] drm_get_pci_dev+0x17e/0x2a0 [drm]
 [] nouveau_drm_probe+0x25a/0x2a0 [nouveau]
 [] ? _raw_spin_unlock_irqrestore+0x42/0x80
 [] local_pci_probe+0x4b/0x80
 [] pci_device_probe+0x111/0x120
 [] driver_probe_device+0x8b/0x3a0
 [] __driver_attach+0xab/0xb0
 [] ? driver_probe_device+0x3a0/0x3a0
 [] bus_for_each_dev+0x5d/0xa0
 [] driver_attach+0x1e/0x20
 [] bus_add_driver+0x111/0x290
 [] ? 0xffffffffa0229fff
 [] driver_register+0x77/0x170
 [] ? 0xffffffffa0229fff
 [] __pci_register_driver+0x64/0x70
 [] drm_pci_init+0x11a/0x130 [drm]
 [] ? 0xffffffffa0229fff
 [] ? 0xffffffffa0229fff
 [] nouveau_drm_init+0x4d/0x1000 [nouveau]
 [] do_one_initcall+0xea/0x1a0
 [] load_module+0x123b/0x1bf0
 [] ? ddebug_proc_open+0xb0/0xb0
 [] ? trace_hardirqs_on_thunk+0x3a/0x3f
 [] SyS_init_module+0xd7/0x120
 [] system_call_fastpath+0x16/0x1b

Signed-off-by: Peter Hurley 
Signed-off-by: Greg Kroah-Hartman

tty: Remove extra wakeup from pty write() path

2013-07-24T16:29:56+00:00

Acquiring the write_wait queue spin lock now accounts for the largest
slice of cpu time on the tty write path. Two factors contribute to
this situation; a overly-pessimistic line discipline write loop which
_always_ sets up a wait loop even if i/o will immediately succeed, and
on ptys, a wakeup storm from reads and writes.

Writer wakeup does not need to be performed by the pty driver.
Firstly, since the actual i/o is performed within the write, the
line discipline write loop will continue while space remains in
the flip buffers. Secondly, when space becomes avail in the
line discipline receive buffer (and thus also in the flip buffers),
the pty unthrottle re-wakes the writer (non-flow-controlled line
disciplines unconditionally unthrottle the driver when data is
received). Thus, existing in-kernel i/o is guaranteed to advance.
Finally, writer wakeup occurs at the conclusion of the line discipline
write (in tty_write_unlock()). This guarantees that any user-space write
waiters are woken to continue additional i/o.

Signed-off-by: Peter Hurley 
Signed-off-by: Greg Kroah-Hartman

tty: Track flip buffer memory limit atomically

2013-07-23T23:47:08+00:00

Lockless flip buffers require atomically updating the bytes-in-use
watermark.

The pty driver also peeks at the watermark value to limit
memory consumption to a much lower value than the default; query
the watermark with new fn, tty_buffer_space_avail().

Signed-off-by: Peter Hurley 
Signed-off-by: Greg Kroah-Hartman

tty: Convert termios_mutex to termios_rwsem

2013-07-23T23:43:01+00:00

termios is commonly accessed unsafely (especially by N_TTY)
because the existing mutex forces exclusive access.
Convert existing usage.

Signed-off-by: Peter Hurley 
Signed-off-by: Greg Kroah-Hartman

tty: Fix transient pty write() EIO

2013-06-17T19:37:29+00:00

Commit 699390354da6c258b65bf8fa79cfd5feaede50b6
('pty: Ignore slave pty close() if never successfully opened')
introduced a bug with ptys whereby a write() in parallel with an
open() on an existing pty could mistakenly indicate an I/O error.

Only indicate an I/O error if the condition on open() actually exists.

Reported-by: Markus Trippelsdorf 
Signed-off-by: Peter Hurley 
Tested-by: Mikael Pettersson 
Cc: stable  # 3.9
Signed-off-by: Greg Kroah-Hartman

tty: fix up atime/mtime mess, take three

2013-05-01T14:32:21+00:00

We first tried to avoid updating atime/mtime entirely (commit
b0de59b5733d: "TTY: do not update atime/mtime on read/write"), and then
limited it to only update it occasionally (commit 37b7f3c76595: "TTY:
fix atime/mtime regression"), but it turns out that this was both
insufficient and overkill.

It was insufficient because we let people attach to the shared ptmx node
to see activity without even reading atime/mtime, and it was overkill
because the "only once a minute" means that you can't really tell an
idle person from an active one with 'w'.

So this tries to fix the problem properly.  It marks the shared ptmx
node as un-notifiable, and it lowers the "only once a minute" to a few
seconds instead - still long enough that you can't time individual
keystrokes, but short enough that you can tell whether somebody is
active or not.

Reported-by: Simon Kirby 
Acked-by: Jiri Slaby 
Cc: Greg Kroah-Hartman 
Cc: stable@vger.kernel.org
Signed-off-by: Linus Torvalds

TTY: pty, fix compilation warning

2013-04-15T18:02:54+00:00

When CONFIG_UNIX98_PTYS is unset, we see this warning in pty:
  drivers/tty/pty.c:409:13: warning: ‘pty_unix98_shutdown’ defined but not used

Fix that by moving the function to a section which depends on that
config.

Signed-off-by: Jiri Slaby 
Reported-by: Toralf Foerster 
Signed-off-by: Greg Kroah-Hartman

pty: Remove redundant itty reset

2013-03-15T20:00:48+00:00

port->itty has already been reset by release_tty() before
pty_cleanup() is called.

Call stack:
release_tty()
  tty_kref_put()
    queue_release_one_tty()
      release_one_tty() : workqueue
        tty->ops->cleanup()
          pty_cleanup()

Signed-off-by: Peter Hurley 
Signed-off-by: Greg Kroah-Hartman

pty: Ignore slave open count for master pty open

2013-02-04T23:40:29+00:00

Multiple slave pty opens may be performed in parallel with the
master open. Of course, all the slave opens will fail because the
master pty is still locked but during this time the slave pty
count will be artificially greater than 1. This is should not
cause the master pty open to fail.

Signed-off-by: Peter Hurley 
Signed-off-by: Greg Kroah-Hartman

pty: Ignore slave pty close() if never successfully opened

2013-02-04T23:40:28+00:00

If the master and slave ptys are opened in parallel, the slave open
fails because the pty is still locked. This is as designed.
However, pty_close() is still called for the slave pty which sets
TTY_OTHER_CLOSED in the master pty. This can cause the master open
to fail as well.

Use a common pattern in other tty drivers by setting TTY_IO_ERROR
until the open is successful and only closing the pty if not set.

Note: the master pty always closes regardless of whether the open
was successful, so that proper cleanup can occur.

Signed-off-by: Peter Hurley 
Signed-off-by: Greg Kroah-Hartman