[PATCH v2 0/2] tty: fix break race

[PATCH v2 0/2] tty: fix break race

[Adriano Cordova]

A timed break (TCSBRK/TCSBRKP) and the unconditional TIOCSBRK/TIOCCBRK
can race against USB disconnect and call break_ctl() after the device is
gone, dereferencing freed port data in keyspan_break_ctl().

Both patches are needed: the tty core must not issue break_ctl() to a
hung-up tty, and serial_break() must not call the driver after
disconnect. This is the same two-layer protection read and write already
use.

Changes since v1:
  - v1 https://lore.kernel.org/stable/20260903022452.111316-2-adrianox@gmail.com/T/#t
  - Proper check on the tty side, plus check on the USB side.
  - Tested syzbot

Reported-by: syzbot+473d7477c523b41d4046@syzkaller.appspotmail.com
Link: https://syzkaller.appspot.com/bug?extid=473d7477c523b41d4046
Tested-by: syzbot+473d7477c523b41d4046@syzkaller.appspotmail.com

Adriano Cordova (2):
  USB: serial: fix break_ctl() NULL-pointer dereference
  tty: fix break race

 drivers/tty/tty_io.c            | 19 +++++++++++++++----
 drivers/usb/serial/usb-serial.c |  3 +++
 2 files changed, 18 insertions(+), 4 deletions(-)

-- 
2.51.0

[PATCH v2 1/2] USB: serial: fix break_ctl() NULL-pointer dereference

[Adriano Cordova]

Don't call into the driver after disconnect, just like serial_write()
already does.

Fixes: 9e98966c7bb9 ("tty: rework break handling")
Reported-by: syzbot+473d7477c523b41d4046@syzkaller.appspotmail.com
Link: https://syzkaller.appspot.com/bug?extid=473d7477c523b41d4046
Tested-by: syzbot+473d7477c523b41d4046@syzkaller.appspotmail.com
Cc: stable@vger.kernel.org
Signed-off-by: Adriano Cordova <adrianox@gmail.com>
---
 drivers/usb/serial/usb-serial.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/usb/serial/usb-serial.c b/drivers/usb/serial/usb-serial.c
index 17edc057a311..5f42bfe39212 100644
--- a/drivers/usb/serial/usb-serial.c
+++ b/drivers/usb/serial/usb-serial.c
@@ -532,6 +532,9 @@ static int serial_break(struct tty_struct *tty, int break_state)
 {
 	struct usb_serial_port *port = tty->driver_data;
 
+	if (port->serial->dev->state == USB_STATE_NOTATTACHED)
+		return -ENODEV;
+
 	dev_dbg(&port->dev, "%s\n", __func__);
 
 	if (port->serial->type->break_ctl)
-- 
2.51.0

[PATCH v2 2/2] tty: fix break race

[Adriano Cordova]

A timed break (TCSBRK and TCSBRKP) can race against hangup and call
break_ctl() on a tty whose driver data is gone, causing a use-after-free
or NULL-pointer dereference in the driver.

Check the tty hung-up state before each break_ctl() call, including the
unconditional TIOCSBRK/TIOCCBRK paths, so the driver is no longer called
after hangup.

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

diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index 48569035da56..272f2839acdb 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,13 +2439,16 @@ 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;
 
 	if (tty->ops->break_ctl == NULL)
 		return 0;
 
+	if (tty_hung_up_p(file))
+		return -EIO;
+
 	if (tty->driver->flags & TTY_DRIVER_HARDWARE_BREAK)
 		return tty->ops->break_ctl(tty, duration);
 
@@ -2455,7 +2459,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;
@@ -2714,10 +2721,14 @@ long tty_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 	 * Break handling
 	 */
 	case TIOCSBRK:	/* Turn break on, unconditionally */
+		if (tty_hung_up_p(file))
+			return -EIO;
 		if (tty->ops->break_ctl)
 			return tty->ops->break_ctl(tty, -1);
 		return 0;
 	case TIOCCBRK:	/* Turn break off, unconditionally */
+		if (tty_hung_up_p(file))
+			return -EIO;
 		if (tty->ops->break_ctl)
 			return tty->ops->break_ctl(tty, 0);
 		return 0;
@@ -2727,10 +2738,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

Re: [PATCH v2 0/2] tty: fix break race

[Johan Hovold]

On Thu, Sep 03, 2026 at 12:24:27AM -0400, Adriano Cordova wrote:
> A timed break (TCSBRK/TCSBRKP) and the unconditional TIOCSBRK/TIOCCBRK
> can race against USB disconnect and call break_ctl() after the device is
> gone, dereferencing freed port data in keyspan_break_ctl().
> 
> Both patches are needed: the tty core must not issue break_ctl() to a
> hung-up tty, and serial_break() must not call the driver after
> disconnect. This is the same two-layer protection read and write already
> use.

This doesn't work and as I said in my reply to the syzbot report I'm
preparing a proper fix.

Johan