[PATCH] tty: abort break signalling on hangup

[PATCH] tty: abort break signalling on hangup

[Johan Hovold]

The break ioctls can race with hangup and end up calling into a tty
driver for a device that is already gone or powered down.

Drivers must handle this race, but TCSBRK and TCSBRKP should still be
aborted to avoid calling back into the driver after a user-controlled
timeout (possibly even after the tty has been reopened).

Note that drivers should disable any break state on shutdown so just
return on hangup (calling break_ctl() again is racy and will most likely
fail for drivers handling the race).

Fixes: f34d7a5b7010 ("tty: The big operations rework")
Cc: stable@vger.kernel.org      # 2.6.26
Signed-off-by: Johan Hovold <johan@kernel.org>
---

This is related to the USB serial and serial core fixes I just posted:

	https://lore.kernel.org/r/20260903163146.1498497-1-johan@kernel.org/
	https://lore.kernel.org/r/20260903163439.1499055-1-johan@kernel.org

Will take a closer look at the other TTY drivers in the coming days.

Johan


 drivers/tty/tty_io.c | 36 +++++++++++++++++++++++++++---------
 include/linux/tty.h  |  1 +
 2 files changed, 28 insertions(+), 9 deletions(-)

diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index 48569035da56..1c30faae9ec1 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -620,6 +620,8 @@ static void __tty_hangup(struct tty_struct *tty, int exit_session)
 
 	tty_ldisc_hangup(tty, cons_filp != NULL);
 
+	wake_up_interruptible(&tty->break_wait);
+
 	spin_lock_irq(&tty->ctrl.lock);
 	clear_bit(TTY_THROTTLED, &tty->flags);
 	clear_bit(TTY_DO_WRITE_WAKEUP, &tty->flags);
@@ -2431,6 +2433,7 @@ static int tiocgetd(struct tty_struct *tty, int __user *p)
  * send_break - performed time break
  * @tty: device to break on
  * @duration: timeout in mS
+ * @file: file object
  *
  * Perform a timed break on hardware that lacks its own driver level timed
  * break functionality.
@@ -2438,8 +2441,9 @@ 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)
 {
+	long timeout;
 	int retval;
 
 	if (tty->ops->break_ctl == NULL)
@@ -2453,13 +2457,26 @@ static int send_break(struct tty_struct *tty, unsigned int duration)
 		return -EINTR;
 
 	retval = tty->ops->break_ctl(tty, -1);
-	if (!retval) {
-		msleep_interruptible(duration);
-		retval = tty->ops->break_ctl(tty, 0);
-	} else if (retval == -EOPNOTSUPP) {
-		/* some drivers can tell only dynamically */
-		retval = 0;
+	if (retval) {
+		if (retval == -EOPNOTSUPP) {
+			/* some drivers can tell only dynamically */
+			retval = 0;
+		}
+		goto out_unlock;
+	}
+
+	timeout = msecs_to_jiffies(duration);
+	timeout = wait_event_interruptible_timeout(tty->break_wait,
+						   tty_hung_up_p(file),
+						   timeout);
+	/* return early on hangup only */
+	if (timeout > 0) {
+		retval = -EIO;
+		goto out_unlock;
 	}
+
+	retval = tty->ops->break_ctl(tty, 0);
+out_unlock:
 	tty_write_unlock(tty);
 
 	if (signal_pending(current))
@@ -2727,10 +2744,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);
@@ -3090,6 +3107,7 @@ struct tty_struct *alloc_tty_struct(struct tty_driver *driver, int idx)
 	init_ldsem(&tty->ldisc_sem);
 	init_waitqueue_head(&tty->write_wait);
 	init_waitqueue_head(&tty->read_wait);
+	init_waitqueue_head(&tty->break_wait);
 	INIT_WORK(&tty->hangup_work, do_tty_hangup);
 	mutex_init(&tty->atomic_write_lock);
 	spin_lock_init(&tty->ctrl.lock);
diff --git a/include/linux/tty.h b/include/linux/tty.h
index 0a46e4054dec..ad4e4a835f90 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -230,6 +230,7 @@ struct tty_struct {
 	struct fasync_struct *fasync;
 	wait_queue_head_t write_wait;
 	wait_queue_head_t read_wait;
+	wait_queue_head_t break_wait;
 	struct work_struct hangup_work;
 	void *disc_data;
 	void *driver_data;
-- 
2.55.0

Re: [PATCH] tty: abort break signalling on hangup

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing hangup check before initiating the break state leaves the tty in a permanent break state if the port is already hung up.
--

commit fb29de6b768e6b046a458b30129e8f3ab80e0b86
Author: Johan Hovold <johan@kernel.org>

tty: abort break signalling on hangup

This commit aborts TCSBRK and TCSBRKP break signalling on hangup by waking
up pending break ioctls and returning early. This prevents calling back into
the driver after a user-controlled timeout for a disconnected device.

> diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
> index 48569035da56e..1c30faae9ec13 100644
> --- a/drivers/tty/tty_io.c
> +++ b/drivers/tty/tty_io.c
[ ... ]
> @@ -2453,13 +2457,26 @@ static int send_break(struct tty_struct *tty, unsigned int duration)
>  		return -EINTR;
>  
>  	retval = tty->ops->break_ctl(tty, -1);
> -	if (!retval) {
> -		msleep_interruptible(duration);
> -		retval = tty->ops->break_ctl(tty, 0);
> -	} else if (retval == -EOPNOTSUPP) {
> -		/* some drivers can tell only dynamically */
> -		retval = 0;
> +	if (retval) {
> +		if (retval == -EOPNOTSUPP) {
> +			/* some drivers can tell only dynamically */
> +			retval = 0;
> +		}
> +		goto out_unlock;
> +	}
> +
> +	timeout = msecs_to_jiffies(duration);
> +	timeout = wait_event_interruptible_timeout(tty->break_wait,
> +						   tty_hung_up_p(file),
> +						   timeout);
> +	/* return early on hangup only */
> +	if (timeout > 0) {
> +		retval = -EIO;
> +		goto out_unlock;
>  	}

[Severity: High]
Does this leave the tty in a permanent break state if the port was already
hung up before calling send_break()?

If a thread is sleeping in tty_wait_until_sent() during an ioctl(TCSBRK)
and a hangup occurs, __tty_hangup() marks the file as hung up and wakes
the thread. The thread then proceeds to send_break(), unconditionally
asserting the break state via:

    retval = tty->ops->break_ctl(tty, -1);

Since the port is already hung up, the wait_event_interruptible_timeout()
condition tty_hung_up_p(file) evaluates to true immediately, returning the
initial timeout which is > 0.

Taking this early return path skips the cleanup call to
tty->ops->break_ctl(tty, 0), which can force the hardware TX line into a
continuous break state and corrupt outgoing data for any subsequent user.

> +
> +	retval = tty->ops->break_ctl(tty, 0);
> +out_unlock:
>  	tty_write_unlock(tty);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260903163736.1499280-1-johan@kernel.org?part=1

Re: [PATCH] tty: abort break signalling on hangup

[Johan Hovold]

On Thu, Sep 03, 2026 at 04:50:04PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Missing hangup check before initiating the break state leaves
> the tty in a permanent break state if the port is already hung up.

As the commit message explains, drivers should already be clearing any
break state on shutdown (cf. closing a port after enabling break).

Johan