[PATCH 0/5] serial: core: fixes and improvements to uart_get_baud_rate()

[PATCH 0/5] serial: core: fixes and improvements to uart_get_baud_rate()

[Hugo Villeneuve]

From: Hugo Villeneuve <hvilleneuve@dimonoff.com>

serial: core: fixes and improvements to uart_get_baud_rate()

Hello,
this patch series brings fixes and improvements to uart_get_baud_rate()

This is in part based on discussions following Hui patches submission [1]
to fix a divide by zero fault originating from uart_get_baud_rate().

Patch 1 is the fix for the above mentioned bug, as suggested by Hui
but with much improved comments, bug report and reproduce steps so that is is
easier to follow and understand. In original discussion, it was suggested by
Ilpo and me that when rate gets lowered, it should alter the termios to prevent
what is here seem as "old" having an invalid value. This is not handled by this
series at the moment, as this may be more complex than anticipated.

Patch 2 is another fix for a potential divide by zero scenario.

Patch 3 removes any possibility for the function to return zero baud rate
if the loop exits without a valid baud rate. History has shown that for
this function, there was often unforeseen or unanticipated conditions
that made the loop exit unexpectedly with a zero baud rate.

Patch 4 is a small improvement in readability to avoid a redundant baud
assignment without any UPF_SPD_* flags. It could be merged with the refactor
patch if deemed appropriate.

Patch 5 is a refactoring of uart_get_baud_rate() to make it easier to
understand and analyze, and more linear, as with the current loop is is
sometimes not easy to fully grasp what is happening. But in the end, I
am not sure it is worth it as the loop makes things more efficient and
simpler. I include the patch for discussion, but it may be removed from
this series.

Tested the changes on a custom board with imx uart and two SC16IS752 DUART
over a SPI interface using a Variscite IMX6 SOM.

Link: https://lore.kernel.org/all/20260930125901.778868-1-benquike@gmail.com/ [1]

Thank you.

Hugo Villeneuve (5):
  serial: core: fix division by zero if old termios is present but has
    invalid rate
  serial: core: clip out-of-range baud rate to min/max, not min+1/max-1
  serial: core: always return in-range baud rate even in case of failure
  serial: core: avoid redundant baud assignment without any UPF_SPD_*
    flags
  serial: core: refactor uart_get_baud_rate() for readability

 drivers/tty/serial/serial_core.c | 113 ++++++++++++++++++-------------
 1 file changed, 65 insertions(+), 48 deletions(-)


base-commit: 1184f87e2790e8af9be3116f608d2e86b4792d7d
-- 
2.47.3

[PATCH 1/5] serial: core: fix division by zero if old termios is present but has invalid rate

[Hugo Villeneuve]

From: Hugo Villeneuve <hvilleneuve@dimonoff.com>

If the old baud rate termios is present, it is implicitely assumed to be
valid. However, if base_baud is lowered via TIOCSSERIAL, and the new
max value is lower than the old baud rate, the later will become invalid,
and uart_get_baud_rate() loop will exit, and return a baud rate of zero,
causing this:

  WARNING: drivers/tty/serial/serial_core.c:547 at
    uart_get_baud_rate+0x104/0x180, CPU#0: uart_baud_test/782
  ...
  Division by zero in kernel.
  CPU: 0 UID: 0 PID: 782 Comm: uart_baud_test Tainted... 7.3.0-rc6-...
  Tainted: [W]=WARN
  Hardware name: Freescale i.MX6 Ultralite (Device Tree)
  Call trace:
   unwind_backtrace from show_stack+0x10/0x14
   show_stack from dump_stack_lvl+0x50/0x64
   dump_stack_lvl from Ldiv0_64+0x8/0x18
   Ldiv0_64 from div64_u64+0xb0/0xd4
   div64_u64 from uart_update_timeout+0x3c/0x44
   uart_update_timeout from sc16is7xx_set_termios+0x308/0x428 [sc16is7xx]
   sc16is7xx_set_termios [sc16is7xx] from uart_change_line_settings+0x34/0x1a8
   uart_change_line_settings from uart_set_termios+0x98/0x13c
   uart_set_termios from tty_set_termios+0x154/0x208
   ...

Reproducing scenario:
  set serial port speed to B115200
  set baud_base to 9600 via TIOCSSERIAL:
    this will lower maximum allowed baud rate to 9600
  call tcsetattr() with B57600 (old = B115200)
    uart_get_baud_rate() will return 0

Fix by running the loop an additional iteration, so to always make sure to
return a valid (clipped) baud rate if old is present but invalid.

Update confusing comments about @termios structure update: it will be
updated only if the desired baud rate is invalid, and also if @old
contained B0.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Reported-by: Hui Peng <benquike@gmail.com>
Link: https://lore.kernel.org/all/20260930125901.778868-1-benquike@gmail.com/
Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
---
 drivers/tty/serial/serial_core.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index c0deec0e6af41..c3c1a1d76405d 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -465,12 +465,9 @@ EXPORT_SYMBOL(uart_update_timeout);
  * magic 38400 baud rate (with spd_* flags), and mapping the %B0 rate to 9600
  * baud or min argument, whichever is greater.
  *
- * If the new baud rate is invalid, try the @old termios setting. If it's still
- * invalid, clip to the nearest chip supported rate.
- *
- * The @termios structure is updated to reflect the baud rate we're actually
- * going to be using. Don't do this for the case where B0 is requested ("hang
- * up").
+ * If the desired baud rate is invalid, try the @old termios setting. If @old
+ * termios is absent or if it's still invalid, clip to the nearest chip
+ * supported rate. For these cases, @termios is updated with the fallback rate.
  *
  * Locking: caller dependent
  */
@@ -501,7 +498,7 @@ uart_get_baud_rate(struct uart_port *port, struct ktermios *termios,
 		break;
 	}
 
-	for (try = 0; try < 2; try++) {
+	for (try = 0; try < 3; try++) {
 		baud = tty_termios_baud_rate(termios);
 
 		/*
-- 
2.47.3

[PATCH 2/5] serial: core: clip out-of-range baud rate to min/max, not min+1/max-1

[Hugo Villeneuve]

From: Hugo Villeneuve <hvilleneuve@dimonoff.com>

min and max values are valid baud rates, as the existing code in
uart_get_baud_rate() already checks:

    if (baud >= min && baud <= max)
        return baud;

I couldn't find any explanation in commit 16ae2a877bf4 ("serial: Fix crash
if the minimum rate of the device is > 9600 baud") as to why this is
clipped to min+1/max-1.

Also, Sashiko identified a potential problem where clipping to min+1/max-1
will be out of range when min == max. This would result in the loop exiting
and returning a zero baud rate, triggering a Division by zero fault.

Fix by clipping to min/max.

Fixes: 16ae2a877bf4 ("serial: Fix crash if the minimum rate of the device is > 9600 baud")
Cc: stable@vger.kernel.org
Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
---
 drivers/tty/serial/serial_core.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index c3c1a1d76405d..0498a8524c6da 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -533,10 +533,10 @@ uart_get_baud_rate(struct uart_port *port, struct ktermios *termios,
 		 * As a last resort, if the range cannot be met then clip to
 		 * the nearest chip supported rate.
 		 */
-		if (baud <= min)
-			baud = min + 1;
+		if (baud < min)
+			baud = min;
 		else
-			baud = max - 1;
+			baud = max;
 
 		tty_termios_encode_baud_rate(termios, baud, baud);
 	}
-- 
2.47.3

[PATCH 3/5] serial: core: always return in-range baud rate even in case of failure

[Hugo Villeneuve]

From: Hugo Villeneuve <hvilleneuve@dimonoff.com>

Returning a zero baud rate from uart_get_baud_rate() can trigger a division
by zero fault. To prevent this, always make sure to return non-zero baud
rate if the loop exits without a valid baud rate.

Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
---
 drivers/tty/serial/serial_core.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 0498a8524c6da..d9ed7db53e68d 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -542,8 +542,9 @@ uart_get_baud_rate(struct uart_port *port, struct ktermios *termios,
 	}
 
 	/* Should never happen */
-	WARN_ON(1);
-	return 0;
+	dev_warn_ratelimited(port->dev, "%s (%u): unable to find fallback baud rate\n",
+			     port->name, port->line);
+	return baud;
 }
 EXPORT_SYMBOL(uart_get_baud_rate);
 
-- 
2.47.3

[PATCH 4/5] serial: core: avoid redundant baud assignment without any UPF_SPD_* flags

[Hugo Villeneuve]

From: Hugo Villeneuve <hvilleneuve@dimonoff.com>

If no UPF_SPD_* flags are set, the spd_* flags kludge is not necessary, so
avoid redundant and confusing "baud = altbaud" assignment.

No functional change intended.

Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
---
 drivers/tty/serial/serial_core.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index d9ed7db53e68d..08a6afa75af07 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -494,7 +494,7 @@ uart_get_baud_rate(struct uart_port *port, struct ktermios *termios,
 		altbaud = 460800;
 		break;
 	default:
-		altbaud = 38400;
+		altbaud = 0;
 		break;
 	}
 
@@ -505,7 +505,7 @@ uart_get_baud_rate(struct uart_port *port, struct ktermios *termios,
 		 * The spd_hi, spd_vhi, spd_shi, spd_warp kludge...
 		 * Die! Die! Die!
 		 */
-		if (try == 0 && baud == 38400)
+		if (altbaud && baud == 38400)
 			baud = altbaud;
 
 		/*
@@ -517,6 +517,8 @@ uart_get_baud_rate(struct uart_port *port, struct ktermios *termios,
 		if (baud >= min && baud <= max)
 			return baud;
 
+		altbaud = 0; /* No more spd_* kludge for next iterations. */
+
 		/*
 		 * If the range cannot be met then try again with
 		 * the old baud rate if possible.
-- 
2.47.3

[PATCH 5/5] serial: core: refactor uart_get_baud_rate() for readability

[Hugo Villeneuve]

From: Hugo Villeneuve <hvilleneuve@dimonoff.com>

The current loop makes it hard to properly understand the code flow,
so replace it with more linear code using a validation function to
improve readability.

Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
---
 drivers/tty/serial/serial_core.c | 103 ++++++++++++++++++-------------
 1 file changed, 60 insertions(+), 43 deletions(-)

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 08a6afa75af07..7457c6b44f325 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -453,6 +453,40 @@ uart_update_timeout(struct uart_port *port, unsigned int cflag,
 }
 EXPORT_SYMBOL(uart_update_timeout);
 
+static int
+uart_validate_baud_rate(const struct ktermios *termios, unsigned int altbaud,
+			unsigned int *ret_baud, unsigned int min, unsigned int max)
+{
+	unsigned int baud;
+	int rc;
+
+	baud = tty_termios_baud_rate(termios);
+
+	/*
+	 * The spd_hi, spd_vhi, spd_shi, spd_warp kludge...
+	 * Die! Die! Die!
+	 */
+	if (altbaud && baud == 38400)
+		baud = altbaud;
+
+	if (baud == 0) {
+		/* Special case: B0 rate. */
+		baud = max(min, 9600);
+		rc = 1;
+	} else if (baud < min) {
+		baud = min; /* Clip to lowest chip supported rate. */
+		rc = -EINVAL;
+	} else if (baud > max) {
+		baud = max; /* Clip to highest chip supported rate. */
+		rc = -EINVAL;
+	} else {
+		rc = 0; /* Valid. */
+	}
+
+	*ret_baud = baud;
+	return rc;
+}
+
 /**
  * uart_get_baud_rate - return baud rate for a particular port
  * @port: uart_port structure describing the port in question.
@@ -475,7 +509,7 @@ unsigned int
 uart_get_baud_rate(struct uart_port *port, struct ktermios *termios,
 		   const struct ktermios *old, unsigned int min, unsigned int max)
 {
-	unsigned int try;
+	int rc;
 	unsigned int baud;
 	unsigned int altbaud;
 	upf_t flags = port->flags & UPF_SPD_MASK;
@@ -498,54 +532,37 @@ uart_get_baud_rate(struct uart_port *port, struct ktermios *termios,
 		break;
 	}
 
-	for (try = 0; try < 3; try++) {
-		baud = tty_termios_baud_rate(termios);
+	rc = uart_validate_baud_rate(termios, altbaud, &baud, min, max);
+	if (rc >= 0)
+		return baud;
 
-		/*
-		 * The spd_hi, spd_vhi, spd_shi, spd_warp kludge...
-		 * Die! Die! Die!
-		 */
-		if (altbaud && baud == 38400)
-			baud = altbaud;
+	/*
+	 * Invalid baud rate detected. First clear c_cflag because
+	 * tty_termios_encode_baud_rate() tests existing c_cflag before encoding new value.
+	 */
+	termios->c_cflag &= ~CBAUD;
 
-		/*
-		 * Special case: B0 rate.
-		 */
-		if (baud == 0)
-			return max(min, 9600);
+	altbaud = 0; /* No more spd_* kludge. */
 
-		if (baud >= min && baud <= max)
-			return baud;
-
-		altbaud = 0; /* No more spd_* kludge for next iterations. */
-
-		/*
-		 * If the range cannot be met then try again with
-		 * the old baud rate if possible.
-		 */
-		termios->c_cflag &= ~CBAUD;
-		if (old) {
-			baud = tty_termios_baud_rate(old);
-			tty_termios_encode_baud_rate(termios, baud, baud);
-			old = NULL;
-			continue;
+	/*
+	 * If old termios is present, retry with the old baud rate. Otherwise,
+	 * baud holds clipped value, and this will be used.
+	 */
+	if (old) {
+		rc = uart_validate_baud_rate(old, altbaud, &baud, min, max);
+		if (rc == 1) {
+			/* Special case: old was B0 (hang-up). */
+			tty_termios_encode_baud_rate(termios, 0, 0); /* Encode B0 in termios. */
+			return baud; /* Return B0-mapped baud rate, not B0. */
 		}
-
-		/*
-		 * As a last resort, if the range cannot be met then clip to
-		 * the nearest chip supported rate.
-		 */
-		if (baud < min)
-			baud = min;
-		else
-			baud = max;
-
-		tty_termios_encode_baud_rate(termios, baud, baud);
 	}
 
-	/* Should never happen */
-	dev_warn_ratelimited(port->dev, "%s (%u): unable to find fallback baud rate\n",
-			     port->name, port->line);
+	tty_termios_encode_baud_rate(termios, baud, baud);
+	rc = uart_validate_baud_rate(termios, altbaud, &baud, min, max);
+	if (rc != 0)
+		dev_warn_ratelimited(port->dev, "%s (%u): unable to find fallback baud rate\n",
+				     port->name, port->line);
+
 	return baud;
 }
 EXPORT_SYMBOL(uart_get_baud_rate);
-- 
2.47.3