Forwarded: [PATCH] tty: fix break race

syzbot posted 1 patch 3 weeks, 2 days ago
drivers/tty/tty_io.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
Forwarded: [PATCH] tty: fix break race
Posted by syzbot 3 weeks, 2 days ago
For archival purposes, forwarding an incoming command email to
linux-kernel@vger.kernel.org, syzkaller-bugs@googlegroups.com.

***

Subject: [PATCH] tty: fix break race
Author: adrianox@gmail.com

A timed break (TCSBRK and TCSBRKP) can race against hangup and end up
calling into a tty driver after the underlying device has been removed,
for example when a USB serial device is disconnected while a break is in
progress. This can result in a use-after-free or NULL-pointer dereference
in the driver when the second break_ctl() call is made after the sleep,
as reported for the keyspan USB serial driver.

Make send_break() wait only for the break duration and then check the
hung-up state of the tty before invoking the driver a second time. Since
the tty is marked hung up (f_op == &hung_up_tty_fops) before the device is
torn down, the second break_ctl() is no longer issued after a hangup and
the driver is not called for a device that is gone.

Suggested-by: Johan Hovold <johan@kernel.org>
Reported-by: syzbot+473d7477c523b41d4046@syzkaller.appspotmail.com
Link: https://syzkaller.appspot.com/bug?extid=473d7477c523b41d4046
Assisted-by: opencode:deepseek-v4-flash
Cc: stable@vger.kernel.org
Signed-off-by: Adriano Cordova <adrianox@gmail.com>
#syz test
---
 drivers/tty/tty_io.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index 48569035da56..c9d8a138b84c 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -2429,6 +2429,7 @@ static int tiocgetd(struct tty_struct *tty, int __user *p)
 
 /**
  * send_break - performed time break
+ * @file: file object
  * @tty: device to break on
  * @duration: timeout in mS
  *
@@ -2438,7 +2439,7 @@ static int tiocgetd(struct tty_struct *tty, int __user *p)
  * Locking:
  *	@tty->atomic_write_lock serializes
  */
-static int send_break(struct tty_struct *tty, unsigned int duration)
+static int send_break(struct file *file, struct tty_struct *tty, unsigned int duration)
 {
 	int retval;
 
@@ -2455,7 +2456,10 @@ static int send_break(struct tty_struct *tty, unsigned int duration)
 	retval = tty->ops->break_ctl(tty, -1);
 	if (!retval) {
 		msleep_interruptible(duration);
-		retval = tty->ops->break_ctl(tty, 0);
+		if (tty_hung_up_p(file))
+			retval = -EIO;
+		else
+			retval = tty->ops->break_ctl(tty, 0);
 	} else if (retval == -EOPNOTSUPP) {
 		/* some drivers can tell only dynamically */
 		retval = 0;
@@ -2727,10 +2731,10 @@ long tty_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 		 * This is used by the tcdrain() termios function.
 		 */
 		if (!arg)
-			return send_break(tty, 250);
+			return send_break(file, tty, 250);
 		return 0;
 	case TCSBRKP:	/* support for POSIX tcsendbreak() */
-		return send_break(tty, arg ? arg*100 : 250);
+		return send_break(file, tty, arg ? arg * 100 : 250);
 
 	case TIOCMGET:
 		return tty_tiocmget(tty, p);
-- 
2.51.0