mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v8 0/2] serial: core: fix baud rate fallback loop and baud_base overflow
@ 2026-10-07  9:47 Hui Peng
  2026-10-07  9:47 ` [PATCH v8 1/2] serial: core: fix baud rate fallback in uart_get_baud_rate() Hui Peng
  2026-10-07  9:47 ` [PATCH v8 2/2] serial: core: reject baud_base values that overflow port->uartclk in uart_set_info() Hui Peng
  0 siblings, 2 replies; 5+ messages in thread
From: Hui Peng @ 2026-10-07  9:47 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jiri Slaby, John Ogness, Ilpo Järvinen
  Cc: Andy Shevchenko, Hugo Villeneuve, linux-serial, linux-kernel,
	stable, Hui Peng

This series addresses two issues in serial core:
1. Division-by-zero WARN/Oops during baud rate fallback in uart_get_baud_rate().
2. 32-bit unsigned overflow in uart_set_info() when multiplying new_info->baud_base by 16.

All patches are ported to the latest tip of mainline (551c722f4080) and
dynamically verified in QEMU KVM with CONFIG_KASAN=y.

Changes in v8:
- 1/2: Rewrote commit description to detail the step-by-step loop iteration
  progression (try == 0, try == 1, try == 2) as requested by Ilpo Järvinen
  and Greg Kroah-Hartman.
- 2/2: Dropped secondary Fixes: 6eabce6608d6 tag per Hugo Villeneuve feedback.

Hui Peng (2):
  serial: core: fix baud rate fallback in uart_get_baud_rate()
  serial: core: reject baud_base values that overflow port->uartclk in
    uart_set_info()

 drivers/tty/serial/serial_core.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)
-- 
2.47.3

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v8 1/2] serial: core: fix baud rate fallback in uart_get_baud_rate()
  2026-10-07  9:47 [PATCH v8 0/2] serial: core: fix baud rate fallback loop and baud_base overflow Hui Peng
@ 2026-10-07  9:47 ` Hui Peng
  2026-10-07 10:43   ` Ilpo Järvinen
  2026-10-07 14:13   ` Hugo Villeneuve
  2026-10-07  9:47 ` [PATCH v8 2/2] serial: core: reject baud_base values that overflow port->uartclk in uart_set_info() Hui Peng
  1 sibling, 2 replies; 5+ messages in thread
From: Hui Peng @ 2026-10-07  9:47 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jiri Slaby, John Ogness, Ilpo Järvinen
  Cc: Andy Shevchenko, Hugo Villeneuve, linux-serial, linux-kernel,
	stable, Hui Peng

When uart_get_baud_rate() evaluates a requested baud rate against a port's
supported limits [min, max], the retry loop operates as follows:

  try == 0: Evaluates the requested baud rate. If out of range and an old
            termios is available, it switches termios to old.
  try == 1: If old is also out of range (or unavailable), the fallback rule
            at the end of the loop clips baud to [min, max - 1] and encodes
            it into termios via tty_termios_encode_baud_rate(termios, baud,
            baud).
  try == 2: Evaluates baud = tty_termios_baud_rate(termios) for the newly
            encoded clipped rate, which now satisfies min <= baud && baud
            <= max and returns baud from within the loop body.

However, because the current loop bound is for (try = 0; try < 2; try++),
the loop terminates immediately after try == 1 without executing try == 2 to
re-evaluate the clipped rate. Upon loop exit, the function hits WARN_ON(1)
and returns 0, which triggers a fatal divide-by-zero (Oops: divide error)
in uart_get_divisor():

  WARNING: drivers/tty/serial/serial_core.c:548 at uart_get_baud_rate+0x136/0x260
  [ ... ]
  divide error: 0000 [#1] PREEMPT SMP KASAN

Increase the loop retry limit in uart_get_baud_rate() from 2 to 3 iterations
(try < 3) so that clipped fallback baud rates are re-evaluated in try == 2.

Tested in QEMU against Linux 7.3.0-rc3 by setting B4000000 on /dev/ttyS0 via
tcsetattr(): on the unfixed kernel it triggers WARN_ON(1) and divide-by-zero
Oops, whereas with this fix applied uart_get_baud_rate() smoothly falls back
to 115200 without error.

Fixes: 091ea8e5d34e ("serial: core: prevent division by zero by always returning non-zero baud rate")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Hui Peng <benquike@gmail.com>
---
Changes in v8:
- Rewrote commit description to detail the step-by-step loop iteration
  progression (try == 0, try == 1, try == 2) as requested by Ilpo Järvinen
  and Greg Kroah-Hartman.

 drivers/tty/serial/serial_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index f91bcfa30113..6ed0195e6912 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -470,7 +470,7 @@ unsigned int uart_get_baud_rate(struct uart_port *port, struct ktermios *termi
 	 * Ask the low level driver to verify the baud rate if it can't
 	 * then it will have encode_baud_rate set the Closet else .
 	 */
-	for (try = 0; try < 2; try++) {
+	for (try = 0; try < 3; try++) {
 		baud = tty_termios_baud_rate(termios);

 		/*
-- 
2.47.3

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v8 2/2] serial: core: reject baud_base values that overflow port->uartclk in uart_set_info()
  2026-10-07  9:47 [PATCH v8 0/2] serial: core: fix baud rate fallback loop and baud_base overflow Hui Peng
  2026-10-07  9:47 ` [PATCH v8 1/2] serial: core: fix baud rate fallback in uart_get_baud_rate() Hui Peng
@ 2026-10-07  9:47 ` Hui Peng
  1 sibling, 0 replies; 5+ messages in thread
From: Hui Peng @ 2026-10-07  9:47 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jiri Slaby, John Ogness, Ilpo Järvinen
  Cc: Andy Shevchenko, Hugo Villeneuve, linux-serial, linux-kernel,
	stable, Hui Peng

In uart_set_info(), new_info->baud_base is multiplied by 16 and stored in
uport->uartclk (an unsigned int):

  uport->uartclk = new_info->baud_base * 16;

While uart_set_info() checks if (uartclk == 0) and
if (new_info->baud_base < 9600), when new_info->baud_base exceeds
UINT_MAX / 16 with low bits set (for example, 0x10000001), multiplying
by 16 wraps around in 32-bit unsigned arithmetic to a small non-zero value
(16), bypassing both uartclk == 0 and new_info->baud_base < 9600 and
setting uport->uartclk = 16 (baud_base = 1, well below the required
minimum of 9600 * 16).

Use check_mul_overflow(new_info->baud_base, 16, &uartclk) in
uart_set_info() to reject overflowing baud_base values with -EINVAL.

Tested in QEMU against Linux 7.3.0-rc3 by calling ioctl(fd, TIOCSSERIAL,
&ss) with ss.baud_base = 0x10000001 on /dev/ttyS1: on the unfixed kernel
TIOCSSERIAL succeeds (ret = 0) and wraps uport->uartclk to 16
(TIOCGSERIAL reports baud_base = 1), whereas with the fix applied
TIOCSSERIAL returns -EINVAL and preserves the existing uport->uartclk.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Hui Peng <benquike@gmail.com>
---
Changes in v8:
- Dropped secondary Fixes: 6eabce6608d6 tag per Hugo Villeneuve feedback.

Changes in v7:
- Use check_mul_overflow() instead of raw UINT_MAX / 16 comparison as
  suggested by Jiri Slaby.

 drivers/tty/serial/serial_core.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 6ed0195e6912..9a8f4c2e1180 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -14,6 +14,7 @@
 #include <linux/tty_flip.h>
 #include <linux/serial_core.h>
 #include <linux/console.h>
+#include <linux/overflow.h>
 #include <linux/gpio/consumer.h>
 #include <linux/kernel.h>
 #include <linux/of.h>
@@ -931,9 +932,11 @@ static int uart_set_info(struct tty_struct *tty, struct tty_port *port,
 	old_custom_divisor = uport->custom_divisor;

 	if (!(uport->flags & UPF_FIXED_PORT)) {
-		unsigned int uartclk = new_info->baud_base * 16;

 		/* check needs to be done here before other settings made */
-		if (uartclk == 0)
+		unsigned int uartclk;
+
+		if (check_mul_overflow(new_info->baud_base, 16, &uartclk) ||
+		    uartclk == 0)
 			return -EINVAL;
 	}
-- 
2.47.3

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v8 1/2] serial: core: fix baud rate fallback in uart_get_baud_rate()
  2026-10-07  9:47 ` [PATCH v8 1/2] serial: core: fix baud rate fallback in uart_get_baud_rate() Hui Peng
@ 2026-10-07 10:43   ` Ilpo Järvinen
  2026-10-07 14:13   ` Hugo Villeneuve
  1 sibling, 0 replies; 5+ messages in thread
From: Ilpo Järvinen @ 2026-10-07 10:43 UTC (permalink / raw)
  To: Hui Peng
  Cc: Greg Kroah-Hartman, Jiri Slaby, John Ogness, Andy Shevchenko,
	Hugo Villeneuve, linux-serial, LKML, stable

[-- Attachment #1: Type: text/plain, Size: 3189 bytes --]

On Wed, 7 Oct 2026, Hui Peng wrote:

> When uart_get_baud_rate() evaluates a requested baud rate against a port's
> supported limits [min, max], the retry loop operates as follows:
> 
>   try == 0: Evaluates the requested baud rate. If out of range and an old
>             termios is available, it switches termios to old.
>   try == 1: If old is also out of range

> (or unavailable)

This is not equal to the case where old is out of range because try == 0 
didn't use continue if that's the case so baud is within bounds.

But as suggested by Hugo in the older version thread, the better approach 
would be to prevent "old" from getting invalid in the first place so 
please look into that instead.

-- 
 i.

> , the fallback rule
>             at the end of the loop clips baud to [min, max - 1] and encodes
>             it into termios via tty_termios_encode_baud_rate(termios, baud,
>             baud).
>   try == 2: Evaluates baud = tty_termios_baud_rate(termios) for the newly
>             encoded clipped rate, which now satisfies min <= baud && baud
>             <= max and returns baud from within the loop body.
> 
> However, because the current loop bound is for (try = 0; try < 2; try++),
> the loop terminates immediately after try == 1 without executing try == 2 to
> re-evaluate the clipped rate. Upon loop exit, the function hits WARN_ON(1)
> and returns 0, which triggers a fatal divide-by-zero (Oops: divide error)
> in uart_get_divisor():
> 
>   WARNING: drivers/tty/serial/serial_core.c:548 at uart_get_baud_rate+0x136/0x260
>   [ ... ]
>   divide error: 0000 [#1] PREEMPT SMP KASAN
> 
> Increase the loop retry limit in uart_get_baud_rate() from 2 to 3 iterations
> (try < 3) so that clipped fallback baud rates are re-evaluated in try == 2.
> 
> Tested in QEMU against Linux 7.3.0-rc3 by setting B4000000 on /dev/ttyS0 via
> tcsetattr(): on the unfixed kernel it triggers WARN_ON(1) and divide-by-zero
> Oops, whereas with this fix applied uart_get_baud_rate() smoothly falls back
> to 115200 without error.
> 
> Fixes: 091ea8e5d34e ("serial: core: prevent division by zero by always returning non-zero baud rate")
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Hui Peng <benquike@gmail.com>
> ---
> Changes in v8:
> - Rewrote commit description to detail the step-by-step loop iteration
>   progression (try == 0, try == 1, try == 2) as requested by Ilpo Järvinen
>   and Greg Kroah-Hartman.
> 
>  drivers/tty/serial/serial_core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
> index f91bcfa30113..6ed0195e6912 100644
> --- a/drivers/tty/serial/serial_core.c
> +++ b/drivers/tty/serial/serial_core.c
> @@ -470,7 +470,7 @@ unsigned int uart_get_baud_rate(struct uart_port *port, struct ktermios *termi
>  	 * Ask the low level driver to verify the baud rate if it can't
>  	 * then it will have encode_baud_rate set the Closet else .
>  	 */
> -	for (try = 0; try < 2; try++) {
> +	for (try = 0; try < 3; try++) {
>  		baud = tty_termios_baud_rate(termios);
> 
>  		/*
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v8 1/2] serial: core: fix baud rate fallback in uart_get_baud_rate()
  2026-10-07  9:47 ` [PATCH v8 1/2] serial: core: fix baud rate fallback in uart_get_baud_rate() Hui Peng
  2026-10-07 10:43   ` Ilpo Järvinen
@ 2026-10-07 14:13   ` Hugo Villeneuve
  1 sibling, 0 replies; 5+ messages in thread
From: Hugo Villeneuve @ 2026-10-07 14:13 UTC (permalink / raw)
  To: Hui Peng
  Cc: Greg Kroah-Hartman, Jiri Slaby, John Ogness, Ilpo Järvinen,
	Andy Shevchenko, linux-serial, linux-kernel, stable

Hi Hui,

On Wed,  7 Oct 2026 09:47:17 +0000
Hui Peng <benquike@gmail.com> wrote:

> When uart_get_baud_rate() evaluates a requested baud rate against a port's
> supported limits [min, max], the retry loop operates as follows:
> 
>   try == 0: Evaluates the requested baud rate. If out of range and an old
>             termios is available, it switches termios to old.
>   try == 1: If old is also out of range (or unavailable), the fallback rule
>             at the end of the loop clips baud to [min, max - 1] and encodes
>             it into termios via tty_termios_encode_baud_rate(termios, baud,
>             baud).
>   try == 2: Evaluates baud = tty_termios_baud_rate(termios) for the newly
>             encoded clipped rate, which now satisfies min <= baud && baud
>             <= max and returns baud from within the loop body.

The loop terminates immediately after try == 1. I see that below even
you acknowledge that fact. This is confusing and it does not
properly describe the current behavior. You should instead write
something like:

     try == 2: no more action, the loop exit immediately

and adjust your other comments accordingly.


> However, because the current loop bound is for (try = 0; try < 2; try++),
> the loop terminates immediately after try == 1 without executing try == 2 to
> re-evaluate the clipped rate. Upon loop exit, the function hits WARN_ON(1)
> and returns 0, which triggers a fatal divide-by-zero (Oops: divide error)
> in uart_get_divisor():
> 
>   WARNING: drivers/tty/serial/serial_core.c:548 at uart_get_baud_rate+0x136/0x260
>   [ ... ]
>   divide error: 0000 [#1] PREEMPT SMP KASAN
> 
> Increase the loop retry limit in uart_get_baud_rate() from 2 to 3 iterations
> (try < 3) so that clipped fallback baud rates are re-evaluated in try == 2.
> 
> Tested in QEMU against Linux 7.3.0-rc3 by setting B4000000 on /dev/ttyS0 via
> tcsetattr(): on the unfixed kernel it triggers WARN_ON(1) and divide-by-zero
> Oops, whereas with this fix applied uart_get_baud_rate() smoothly falls back
> to 115200 without error.
> 
> Fixes: 091ea8e5d34e ("serial: core: prevent division by zero by always returning non-zero baud rate")
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Hui Peng <benquike@gmail.com>
> ---
> Changes in v8:
> - Rewrote commit description to detail the step-by-step loop iteration
>   progression (try == 0, try == 1, try == 2) as requested by Ilpo Järvinen
>   and Greg Kroah-Hartman.
> 
>  drivers/tty/serial/serial_core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
> index f91bcfa30113..6ed0195e6912 100644
> --- a/drivers/tty/serial/serial_core.c
> +++ b/drivers/tty/serial/serial_core.c
> @@ -470,7 +470,7 @@ unsigned int uart_get_baud_rate(struct uart_port *port, struct ktermios *termi
>  	 * Ask the low level driver to verify the baud rate if it can't
>  	 * then it will have encode_baud_rate set the Closet else .
>  	 */
> -	for (try = 0; try < 2; try++) {
> +	for (try = 0; try < 3; try++) {
>  		baud = tty_termios_baud_rate(termios);
> 
>  		/*
> -- 
> 2.47.3
> 


-- 
Hugo Villeneuve

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-10-07 14:13 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-07  9:47 [PATCH v8 0/2] serial: core: fix baud rate fallback loop and baud_base overflow Hui Peng
2026-10-07  9:47 ` [PATCH v8 1/2] serial: core: fix baud rate fallback in uart_get_baud_rate() Hui Peng
2026-10-07 10:43   ` Ilpo Järvinen
2026-10-07 14:13   ` Hugo Villeneuve
2026-10-07  9:47 ` [PATCH v8 2/2] serial: core: reject baud_base values that overflow port->uartclk in uart_set_info() Hui Peng

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®