mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] tty: fix lock order inversion in tty_buffer_flush()
@ 2026-08-04 14:02 Ömer Mete Kaya
  2026-09-23 12:56 ` Greg KH
  0 siblings, 1 reply; 2+ messages in thread
From: Ömer Mete Kaya @ 2026-08-04 14:02 UTC (permalink / raw)
  To: gregkh, jirislaby
  Cc: linux-kernel, linux-serial, syzbot+d84934b4184d7f9a1aed,
	Ömer Mete Kaya

tty_buffer_flush() calls ld->ops->flush_buffer() while holding
buf->lock. For the N_TTY line discipline, flush_buffer() is
n_tty_flush_buffer(), which acquires termios_rwsem.

However, the documented stable lock order (see tty.h) is:
  termios_rwsem -> buf->lock

Calling flush_buffer() under buf->lock inverts this order and creates
a circular locking dependency detected by lockdep:

  &buf->lock -> console_lock -> &port->mutex -> &tty->termios_rwsem
                                                -> &buf->lock

Fix this by moving the flush_buffer() call outside buf->lock.
The buffer data has already been discarded at this point, so the
ordering change is safe.

Reported-by: syzbot+d84934b4184d7f9a1aed@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=d84934b4184d7f9a1aed
Signed-off-by: Ömer Mete Kaya <omermetekaya0@gmail.com>
---
 drivers/tty/tty_buffer.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c
index 96be90db5..bb7b7fbcb 100644
--- a/drivers/tty/tty_buffer.c
+++ b/drivers/tty/tty_buffer.c
@@ -244,11 +244,15 @@ void tty_buffer_flush(struct tty_struct *tty, struct tty_ldisc *ld)
 	buf->head->read = buf->head->commit;
 	buf->head->lookahead = buf->head->read;
 
-	if (ld && ld->ops->flush_buffer)
-		ld->ops->flush_buffer(tty);
-
 	atomic_dec(&buf->priority);
 	mutex_unlock(&buf->lock);
+	/*
+	 * Call flush_buffer() outside buf->lock: n_tty_flush_buffer()
+	 * acquires termios_rwsem, but the required lock order is
+	 * termios_rwsem -> buf->lock, not the other way around.
+	 */
+	if (ld && ld->ops->flush_buffer)
+		ld->ops->flush_buffer(tty);
 }
 
 /**
-- 
2.55.0


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] tty: fix lock order inversion in tty_buffer_flush()
  2026-08-04 14:02 [PATCH] tty: fix lock order inversion in tty_buffer_flush() Ömer Mete Kaya
@ 2026-09-23 12:56 ` Greg KH
  0 siblings, 0 replies; 2+ messages in thread
From: Greg KH @ 2026-09-23 12:56 UTC (permalink / raw)
  To: Ömer Mete Kaya
  Cc: jirislaby, linux-kernel, linux-serial, syzbot+d84934b4184d7f9a1aed

On Tue, Aug 04, 2026 at 05:02:51PM +0300, Ömer Mete Kaya wrote:
> tty_buffer_flush() calls ld->ops->flush_buffer() while holding
> buf->lock. For the N_TTY line discipline, flush_buffer() is
> n_tty_flush_buffer(), which acquires termios_rwsem.
> 
> However, the documented stable lock order (see tty.h) is:
>   termios_rwsem -> buf->lock
> 
> Calling flush_buffer() under buf->lock inverts this order and creates
> a circular locking dependency detected by lockdep:
> 
>   &buf->lock -> console_lock -> &port->mutex -> &tty->termios_rwsem
>                                                 -> &buf->lock
> 
> Fix this by moving the flush_buffer() call outside buf->lock.
> The buffer data has already been discarded at this point, so the
> ordering change is safe.
> 
> Reported-by: syzbot+d84934b4184d7f9a1aed@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=d84934b4184d7f9a1aed

Did syzbot actually test this?  How did you test this?

Did you forget an Assisted-by: tag?

> Signed-off-by: Ömer Mete Kaya <omermetekaya0@gmail.com>
> ---
>  drivers/tty/tty_buffer.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c
> index 96be90db5..bb7b7fbcb 100644
> --- a/drivers/tty/tty_buffer.c
> +++ b/drivers/tty/tty_buffer.c
> @@ -244,11 +244,15 @@ void tty_buffer_flush(struct tty_struct *tty, struct tty_ldisc *ld)
>  	buf->head->read = buf->head->commit;
>  	buf->head->lookahead = buf->head->read;
>  
> -	if (ld && ld->ops->flush_buffer)
> -		ld->ops->flush_buffer(tty);
> -
>  	atomic_dec(&buf->priority);
>  	mutex_unlock(&buf->lock);
> +	/*
> +	 * Call flush_buffer() outside buf->lock: n_tty_flush_buffer()
> +	 * acquires termios_rwsem, but the required lock order is
> +	 * termios_rwsem -> buf->lock, not the other way around.
> +	 */
> +	if (ld && ld->ops->flush_buffer)
> +		ld->ops->flush_buffer(tty);

This feels wrong, what is going to break with this change?  What commit
does this change fix?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-09-23 12:56 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-08-04 14:02 [PATCH] tty: fix lock order inversion in tty_buffer_flush() Ömer Mete Kaya
2026-09-23 12:56 ` Greg KH

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®