From: Vasiliy Kovalev kovalev@altlinux.org
A possible scenario in which a deadlock may occur is as follows:
flush_to_ldisc() {
mutex_lock(&buf->lock);
tty_port_default_receive_buf() { tty_ldisc_receive_buf() { n_tty_receive_buf2() { n_tty_receive_buf_common() { n_tty_receive_char_special() { isig() { tty_driver_flush_buffer() { pty_flush_buffer() { tty_buffer_flush() {
mutex_lock(&buf->lock); (DEADLOCK)
flush_to_ldisc() and tty_buffer_flush() functions they use the same mutex (&buf->lock), but not necessarily the same struct tty_bufhead object. However, you should probably use a separate mutex for the tty_buffer_flush() function to exclude such a situation.
Found by Syzkaller: ====================================================== WARNING: possible circular locking dependency detected 5.10.213-std-def-alt1 #1 Not tainted ------------------------------------------------------ kworker/u6:8/428 is trying to acquire lock: ffff88810c3498b8 (&buf->lock){+.+.}-{3:3}, at: tty_buffer_flush+0x7b/0x2b0 drivers/tty/tty_buffer.c:228
but task is already holding lock: ffff888114dca2e8 (&o_tty->termios_rwsem/1){++++}-{3:3}, at: isig+0xef/0x440 drivers/tty/n_tty.c:1127
which lock already depends on the new lock.
Chain exists of: &buf->lock --> &port->buf.lock/1 --> &o_tty->termios_rwsem/1
Possible unsafe locking scenario:
CPU0 CPU1 ---- ---- lock(&o_tty->termios_rwsem/1); lock(&port->buf.lock/1); lock(&o_tty->termios_rwsem/1); lock(&buf->lock);
stack backtrace: CPU: 0 PID: 428 Comm: kworker/u6:8 Not tainted 5.10.213-std-def-alt1 #1 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.0-alt1 04/01/2014 Workqueue: events_unbound flush_to_ldisc Call Trace: __dump_stack lib/dump_stack.c:77 [inline] dump_stack+0x19b/0x203 lib/dump_stack.c:118 print_circular_bug.cold+0x162/0x171 kernel/locking/lockdep.c:2002 check_noncircular+0x263/0x2e0 kernel/locking/lockdep.c:2123 check_prev_add kernel/locking/lockdep.c:2988 [inline] check_prevs_add kernel/locking/lockdep.c:3113 [inline] validate_chain kernel/locking/lockdep.c:3729 [inline] __lock_acquire+0x298f/0x5500 kernel/locking/lockdep.c:4955 lock_acquire kernel/locking/lockdep.c:5566 [inline] lock_acquire+0x1fe/0x550 kernel/locking/lockdep.c:5531 __mutex_lock_common kernel/locking/mutex.c:968 [inline] __mutex_lock+0x142/0x10c0 kernel/locking/mutex.c:1109 mutex_lock_nested+0x17/0x20 kernel/locking/mutex.c:1124 tty_buffer_flush+0x7b/0x2b0 drivers/tty/tty_buffer.c:228 pty_flush_buffer+0x4e/0x170 drivers/tty/pty.c:222 tty_driver_flush_buffer+0x65/0x80 drivers/tty/tty_ioctl.c:96 isig+0x1e4/0x440 drivers/tty/n_tty.c:1138 n_tty_receive_signal_char+0x24/0x160 drivers/tty/n_tty.c:1239 n_tty_receive_char_special+0x1261/0x2a70 drivers/tty/n_tty.c:1285 n_tty_receive_buf_fast drivers/tty/n_tty.c:1606 [inline] __receive_buf drivers/tty/n_tty.c:1640 [inline] n_tty_receive_buf_common+0x1e76/0x2b60 drivers/tty/n_tty.c:1738 n_tty_receive_buf2+0x34/0x40 drivers/tty/n_tty.c:1773 tty_ldisc_receive_buf+0xb1/0x1a0 drivers/tty/tty_buffer.c:441 tty_port_default_receive_buf+0x73/0xa0 drivers/tty/tty_port.c:39 receive_buf drivers/tty/tty_buffer.c:461 [inline] flush_to_ldisc+0x21c/0x400 drivers/tty/tty_buffer.c:513 process_one_work+0x9ae/0x14b0 kernel/workqueue.c:2282 worker_thread+0x622/0x1320 kernel/workqueue.c:2428 kthread+0x396/0x470 kernel/kthread.c:313 ret_from_fork+0x22/0x30 arch/x86/entry/entry_64.S:299
Cc: stable@vger.kernel.org Signed-off-by: Vasiliy Kovalev kovalev@altlinux.org --- drivers/tty/tty_buffer.c | 5 +++-- include/linux/tty_buffer.h | 1 + 2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c index 79f0ff94ce00da..e777bd5f3a2fca 100644 --- a/drivers/tty/tty_buffer.c +++ b/drivers/tty/tty_buffer.c @@ -226,7 +226,7 @@ void tty_buffer_flush(struct tty_struct *tty, struct tty_ldisc *ld)
atomic_inc(&buf->priority);
- mutex_lock(&buf->lock); + mutex_lock(&buf->flush_mtx); /* paired w/ release in __tty_buffer_request_room; ensures there are * no pending memory accesses to the freed buffer */ @@ -241,7 +241,7 @@ void tty_buffer_flush(struct tty_struct *tty, struct tty_ldisc *ld) ld->ops->flush_buffer(tty);
atomic_dec(&buf->priority); - mutex_unlock(&buf->lock); + mutex_unlock(&buf->flush_mtx); }
/** @@ -577,6 +577,7 @@ void tty_buffer_init(struct tty_port *port) { struct tty_bufhead *buf = &port->buf;
+ mutex_init(&buf->flush_mtx); mutex_init(&buf->lock); tty_buffer_reset(&buf->sentinel, 0); buf->head = &buf->sentinel; diff --git a/include/linux/tty_buffer.h b/include/linux/tty_buffer.h index 31125e3be3c55e..cea4eacc3b70d3 100644 --- a/include/linux/tty_buffer.h +++ b/include/linux/tty_buffer.h @@ -35,6 +35,7 @@ static inline u8 *flag_buf_ptr(struct tty_buffer *b, unsigned int ofs) struct tty_bufhead { struct tty_buffer *head; /* Queue head */ struct work_struct work; + struct mutex flush_mtx; /* For use in tty_buffer_flush() */ struct mutex lock; atomic_t priority; struct tty_buffer sentinel;