Skip to content

Commit 421b40a

Browse files
peterhurleygregkh
authored andcommitted
tty/vt: Fix vc_deallocate() lock order
Now that the tty port owns the flip buffers and i/o is allowed from the driver even when no tty is attached, the destruction of the tty port (and the flip buffers) must ensure that no outstanding work is pending. Unfortunately, this creates a lock order problem with the console_lock (see attached lockdep report [1] below). For single console deallocation, drop the console_lock prior to port destruction. When multiple console deallocation, defer port destruction until the consoles have been deallocated. tty_port_destroy() is not required if the port has not been used; remove from vc_allocate() failure path. [1] lockdep report from Dave Jones <[email protected]> ====================================================== [ INFO: possible circular locking dependency detected ] 3.9.0+ #16 Not tainted ------------------------------------------------------- (agetty)/26163 is trying to acquire lock: blocked: ((&buf->work)){+.+...}, instance: ffff88011c8b0020, at: [<ffffffff81062065>] flush_work+0x5/0x2e0 but task is already holding lock: blocked: (console_lock){+.+.+.}, instance: ffffffff81c2fde0, at: [<ffffffff813bc201>] vt_ioctl+0xb61/0x1230 which lock already depends on the new lock. the existing dependency chain (in reverse order) is: -> #1 (console_lock){+.+.+.}: [<ffffffff810b3f74>] lock_acquire+0xa4/0x210 [<ffffffff810416c7>] console_lock+0x77/0x80 [<ffffffff813c3dcd>] con_flush_chars+0x2d/0x50 [<ffffffff813b32b2>] n_tty_receive_buf+0x122/0x14d0 [<ffffffff813b7709>] flush_to_ldisc+0x119/0x170 [<ffffffff81064381>] process_one_work+0x211/0x700 [<ffffffff8106498b>] worker_thread+0x11b/0x3a0 [<ffffffff8106ce5d>] kthread+0xed/0x100 [<ffffffff81601cac>] ret_from_fork+0x7c/0xb0 -> #0 ((&buf->work)){+.+...}: [<ffffffff810b349a>] __lock_acquire+0x193a/0x1c00 [<ffffffff810b3f74>] lock_acquire+0xa4/0x210 [<ffffffff810620ae>] flush_work+0x4e/0x2e0 [<ffffffff81065305>] __cancel_work_timer+0x95/0x130 [<ffffffff810653b0>] cancel_work_sync+0x10/0x20 [<ffffffff813b8212>] tty_port_destroy+0x12/0x20 [<ffffffff813c65e8>] vc_deallocate+0xf8/0x110 [<ffffffff813bc20c>] vt_ioctl+0xb6c/0x1230 [<ffffffff813b01a5>] tty_ioctl+0x285/0xd50 [<ffffffff811ba825>] do_vfs_ioctl+0x305/0x530 [<ffffffff811baad1>] sys_ioctl+0x81/0xa0 [<ffffffff81601d59>] system_call_fastpath+0x16/0x1b other info that might help us debug this: [ 6760.076175] Possible unsafe locking scenario: CPU0 CPU1 ---- ---- lock(console_lock); lock((&buf->work)); lock(console_lock); lock((&buf->work)); *** DEADLOCK *** 1 lock on stack by (agetty)/26163: #0: blocked: (console_lock){+.+.+.}, instance: ffffffff81c2fde0, at: [<ffffffff813bc201>] vt_ioctl+0xb61/0x1230 stack backtrace: Pid: 26163, comm: (agetty) Not tainted 3.9.0+ #16 Call Trace: [<ffffffff815edb14>] print_circular_bug+0x200/0x20e [<ffffffff810b349a>] __lock_acquire+0x193a/0x1c00 [<ffffffff8100a269>] ? sched_clock+0x9/0x10 [<ffffffff8100a269>] ? sched_clock+0x9/0x10 [<ffffffff8100a200>] ? native_sched_clock+0x20/0x80 [<ffffffff810b3f74>] lock_acquire+0xa4/0x210 [<ffffffff81062065>] ? flush_work+0x5/0x2e0 [<ffffffff810620ae>] flush_work+0x4e/0x2e0 [<ffffffff81062065>] ? flush_work+0x5/0x2e0 [<ffffffff810b15db>] ? mark_held_locks+0xbb/0x140 [<ffffffff8113c8a3>] ? __free_pages_ok.part.57+0x93/0xc0 [<ffffffff810b15db>] ? mark_held_locks+0xbb/0x140 [<ffffffff810652f2>] ? __cancel_work_timer+0x82/0x130 [<ffffffff81065305>] __cancel_work_timer+0x95/0x130 [<ffffffff810653b0>] cancel_work_sync+0x10/0x20 [<ffffffff813b8212>] tty_port_destroy+0x12/0x20 [<ffffffff813c65e8>] vc_deallocate+0xf8/0x110 [<ffffffff813bc20c>] vt_ioctl+0xb6c/0x1230 [<ffffffff810aec41>] ? lock_release_holdtime.part.30+0xa1/0x170 [<ffffffff813b01a5>] tty_ioctl+0x285/0xd50 [<ffffffff812b00f6>] ? inode_has_perm.isra.46.constprop.61+0x56/0x80 [<ffffffff811ba825>] do_vfs_ioctl+0x305/0x530 [<ffffffff812b04db>] ? selinux_file_ioctl+0x5b/0x110 [<ffffffff811baad1>] sys_ioctl+0x81/0xa0 [<ffffffff81601d59>] system_call_fastpath+0x16/0x1b Cc: Dave Jones <[email protected]> Signed-off-by: Peter Hurley <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
1 parent df957d2 commit 421b40a

File tree

3 files changed

+56
-27
lines changed

3 files changed

+56
-27
lines changed

drivers/tty/vt/vt.c

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -779,7 +779,6 @@ int vc_allocate(unsigned int currcons) /* return 0 on success */
779779
con_set_default_unimap(vc);
780780
vc->vc_screenbuf = kmalloc(vc->vc_screenbuf_size, GFP_KERNEL);
781781
if (!vc->vc_screenbuf) {
782-
tty_port_destroy(&vc->port);
783782
kfree(vc);
784783
vc_cons[currcons].d = NULL;
785784
return -ENOMEM;
@@ -986,26 +985,25 @@ static int vt_resize(struct tty_struct *tty, struct winsize *ws)
986985
return ret;
987986
}
988987

989-
void vc_deallocate(unsigned int currcons)
988+
struct vc_data *vc_deallocate(unsigned int currcons)
990989
{
990+
struct vc_data *vc = NULL;
991+
991992
WARN_CONSOLE_UNLOCKED();
992993

993994
if (vc_cons_allocated(currcons)) {
994-
struct vc_data *vc = vc_cons[currcons].d;
995-
struct vt_notifier_param param = { .vc = vc };
995+
struct vt_notifier_param param;
996996

997+
param.vc = vc = vc_cons[currcons].d;
997998
atomic_notifier_call_chain(&vt_notifier_list, VT_DEALLOCATE, &param);
998999
vcs_remove_sysfs(currcons);
9991000
vc->vc_sw->con_deinit(vc);
10001001
put_pid(vc->vt_pid);
10011002
module_put(vc->vc_sw->owner);
10021003
kfree(vc->vc_screenbuf);
1003-
if (currcons >= MIN_NR_CONSOLES) {
1004-
tty_port_destroy(&vc->port);
1005-
kfree(vc);
1006-
}
10071004
vc_cons[currcons].d = NULL;
10081005
}
1006+
return vc;
10091007
}
10101008

10111009
/*

drivers/tty/vt/vt_ioctl.c

Lines changed: 49 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -283,6 +283,51 @@ do_unimap_ioctl(int cmd, struct unimapdesc __user *user_ud, int perm, struct vc_
283283
return 0;
284284
}
285285

286+
/* deallocate a single console, if possible (leave 0) */
287+
static int vt_disallocate(unsigned int vc_num)
288+
{
289+
struct vc_data *vc = NULL;
290+
int ret = 0;
291+
292+
if (!vc_num)
293+
return 0;
294+
295+
console_lock();
296+
if (VT_BUSY(vc_num))
297+
ret = -EBUSY;
298+
else
299+
vc = vc_deallocate(vc_num);
300+
console_unlock();
301+
302+
if (vc && vc_num >= MIN_NR_CONSOLES) {
303+
tty_port_destroy(&vc->port);
304+
kfree(vc);
305+
}
306+
307+
return ret;
308+
}
309+
310+
/* deallocate all unused consoles, but leave 0 */
311+
static void vt_disallocate_all(void)
312+
{
313+
struct vc_data *vc[MAX_NR_CONSOLES];
314+
int i;
315+
316+
console_lock();
317+
for (i = 1; i < MAX_NR_CONSOLES; i++)
318+
if (!VT_BUSY(i))
319+
vc[i] = vc_deallocate(i);
320+
else
321+
vc[i] = NULL;
322+
console_unlock();
323+
324+
for (i = 1; i < MAX_NR_CONSOLES; i++) {
325+
if (vc[i] && i >= MIN_NR_CONSOLES) {
326+
tty_port_destroy(&vc[i]->port);
327+
kfree(vc[i]);
328+
}
329+
}
330+
}
286331

287332

288333
/*
@@ -769,24 +814,10 @@ int vt_ioctl(struct tty_struct *tty,
769814
ret = -ENXIO;
770815
break;
771816
}
772-
if (arg == 0) {
773-
/* deallocate all unused consoles, but leave 0 */
774-
console_lock();
775-
for (i=1; i<MAX_NR_CONSOLES; i++)
776-
if (! VT_BUSY(i))
777-
vc_deallocate(i);
778-
console_unlock();
779-
} else {
780-
/* deallocate a single console, if possible */
781-
arg--;
782-
if (VT_BUSY(arg))
783-
ret = -EBUSY;
784-
else if (arg) { /* leave 0 */
785-
console_lock();
786-
vc_deallocate(arg);
787-
console_unlock();
788-
}
789-
}
817+
if (arg == 0)
818+
vt_disallocate_all();
819+
else
820+
ret = vt_disallocate(--arg);
790821
break;
791822

792823
case VT_RESIZE:

include/linux/vt_kern.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ extern int fg_console, last_console, want_console;
3636
int vc_allocate(unsigned int console);
3737
int vc_cons_allocated(unsigned int console);
3838
int vc_resize(struct vc_data *vc, unsigned int cols, unsigned int lines);
39-
void vc_deallocate(unsigned int console);
39+
struct vc_data *vc_deallocate(unsigned int console);
4040
void reset_palette(struct vc_data *vc);
4141
void do_blank_screen(int entering_gfx);
4242
void do_unblank_screen(int leaving_gfx);

0 commit comments

Comments
 (0)