[PATCH] tty: limit TCSBRKP break duration

[PATCH] tty: limit TCSBRKP break duration

[Xincheng Wang]

An RCU stall was observed while exercising tty ioctls on an
8250 Pericom PCI serial port.

The reproducer passed a very large argument to TCSBRKP. tty_ioctl()
multiplies the argument by 100 and forwards it to send_break(), whose
duration is an unsigned int. That can leave the serial break condition
asserted for an extremely long time and keep the IRQ line active on the
affected setup.

Clamp the duration before calling send_break() so normal values keep
their current behavior, while preventing excessively long break
intervals.

Signed-off-by: Xincheng Wang <22009200607@stu.xidian.edu.cn>
---
 drivers/tty/tty_io.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index 6b283fd03..5872b2432 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -2454,6 +2454,21 @@ static int tiocgetd(struct tty_struct *tty, int __user *p)
 	return ret;
 }
 
+#define TTY_BREAK_DEFAULT_MS	250
+#define TTY_TCSBRKP_UNIT_MS	100
+#define TTY_TCSBRKP_MAX_MS	5000
+
+static unsigned int tcsbrkp_duration(unsigned long arg)
+{
+	if (!arg)
+		return TTY_BREAK_DEFAULT_MS;
+
+	if (arg > TTY_TCSBRKP_MAX_MS / TTY_TCSBRKP_UNIT_MS)
+		return TTY_TCSBRKP_MAX_MS;
+
+	return arg * TTY_TCSBRKP_UNIT_MS;
+}
+
 /**
  * send_break - performed time break
  * @tty: device to break on
@@ -2757,7 +2772,7 @@ long tty_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 			return send_break(tty, 250);
 		return 0;
 	case TCSBRKP:	/* support for POSIX tcsendbreak() */
-		return send_break(tty, arg ? arg*100 : 250);
+		return send_break(tty, tcsbrkp_duration(arg));
 
 	case TIOCMGET:
 		return tty_tiocmget(tty, p);
-- 
2.34.1

Re: [PATCH] tty: limit TCSBRKP break duration

[Greg Kroah-Hartman]

On Mon, Jul 27, 2026 at 01:55:25PM +0800, Xincheng Wang wrote:
> An RCU stall was observed while exercising tty ioctls on an
> 8250 Pericom PCI serial port.
> 
> The reproducer passed a very large argument to TCSBRKP. tty_ioctl()
> multiplies the argument by 100 and forwards it to send_break(), whose
> duration is an unsigned int. That can leave the serial break condition
> asserted for an extremely long time and keep the IRQ line active on the
> affected setup.
> 
> Clamp the duration before calling send_break() so normal values keep
> their current behavior, while preventing excessively long break
> intervals.

How do we know that systems don't want those long break intervals set?
You are changing something that might break people's working systems.

What specific error are you getting when you send a long break time and
why can't your hardware handle that?

thanks,

greg k-h

Re: [PATCH] tty: limit TCSBRKP break duration

[Xincheng Wang]

Hi Greg,

Thanks for the review.

You are right. The RCU stall I observed does not by itself justify
clamping long break intervals in the tty core, so I will drop that
patch.

While looking at this path, I noticed that tty_ioctl() multiplies the
TCSBRKP argument by 100 and passes the result to send_break(), whose
duration argument is unsigned int. Values whose product exceeds UINT_MAX
are silently truncated to an unintended duration.

Do you think it is worth fixing that separately by rejecting TCSBRKP
values that cannot fit in the unsigned-int millisecond duration used by
send_break()?

Thanks,
Xincheng

Re: [PATCH] tty: limit TCSBRKP break duration

[Greg Kroah-Hartman]

On Wed, Jul 29, 2026 at 07:19:27PM +0800, Xincheng Wang wrote:
> Hi Greg,
> 
> Thanks for the review.
> 
> You are right. The RCU stall I observed does not by itself justify
> clamping long break intervals in the tty core, so I will drop that
> patch.
> 
> While looking at this path, I noticed that tty_ioctl() multiplies the
> TCSBRKP argument by 100 and passes the result to send_break(), whose
> duration argument is unsigned int. Values whose product exceeds UINT_MAX
> are silently truncated to an unintended duration.
> 
> Do you think it is worth fixing that separately by rejecting TCSBRKP
> values that cannot fit in the unsigned-int millisecond duration used by
> send_break()?

Wouldn't that break userspace that is currently relying on that
truncation?

thanks,

greg k-h

Re: [PATCH] tty: limit TCSBRKP break duration

[Xincheng Wang]

Hi Greg,

Yes, it would still be user-visible.

My concern is that tty_ioctl() first multiplies the TCSBRKP argument by
100 before passing it to send_break(), whose duration argument is
unsigned int. For values above UINT_MAX / 100, the current behavior uses
the low 32 bits of the multiplied product, which can be a duration very
different from the requested interval.

But I agree that rejecting or saturating that value would also change
current behavior. Would you be open to a fix for this, or would you
rather leave the path as-is?

Thanks,
Xincheng

Re: [PATCH] tty: limit TCSBRKP break duration

[Greg Kroah-Hartman]

On Fri, Jul 31, 2026 at 10:30:41PM +0800, Xincheng Wang wrote:
> Hi Greg,
> 
> Yes, it would still be user-visible.
> 
> My concern is that tty_ioctl() first multiplies the TCSBRKP argument by
> 100 before passing it to send_break(), whose duration argument is
> unsigned int. For values above UINT_MAX / 100, the current behavior uses
> the low 32 bits of the multiplied product, which can be a duration very
> different from the requested interval.
> 
> But I agree that rejecting or saturating that value would also change
> current behavior. Would you be open to a fix for this, or would you
> rather leave the path as-is?

I lost all context here :(

ANyway, keeping things as-is is always good, unless you know of a way
the current stuff is broken and you can fix it without changing any
existing users.  user apis are hard...

thanks,

greg k-h