From: Vasiliy Kovalev <kovalev@xxxxxxxxxxxx>
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@xxxxxxxxxxxxxxx
Signed-off-by: Vasiliy Kovalev <kovalev@xxxxxxxxxxxx>
---
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;
--
2.33.8
