[PATCH v7 0/2] serial: core: fix baud rate fallback loop and baud_base overflow

[PATCH v7 0/2] serial: core: fix baud rate fallback loop and baud_base overflow

[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.

Note on v6 1/3:
Patch 1/3 from v6 ("serial: 8250: fix deadlock in serial8250_register_ports")
has been dropped. The hash_mutex race condition was originally found and
reproduced on Linux 6.18.14 (where guard(mutex)(&hash_mutex) was dropped
inside serial_get_or_create_irq_info() before serial_link_irq_chain() linked
the port). We did not realize that it was already fixed in mainline by
commit b339809edda1 ("serial: 8250: use guard()s"), which refactored to
serial_get_or_create_irq_info_locked() with guard(mutex)(&hash_mutex) moved
into serial_link_irq_chain(). Apologies for the noise in v6 1/3.

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

Changes in v7:
- Dropped patch 1/3 (already resolved in mainline).
- Clarified in cover letter that 1/3 was found and reproduced on 6.18.14 and
  we did not realize it was already fixed in mainline.
- Retained patch 1/2 and patch 2/2 using check_mul_overflow() as requested
  by Jiri Slaby.

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

[PATCH v7 1/2] serial: core: fix baud rate fallback in uart_get_baud_rate()

[Hui Peng]

When uart_get_baud_rate() is called with a baud rate exceeding the port's
maximum supported speed (port->uartclk / 16), it clips baud to [min,
max - 1] and encodes it into termios via tty_termios_encode_baud_rate().

However, because the loop bound is for (try = 0; try < 2; try++), the
loop terminates immediately after try == 1 without re-evaluating
baud = tty_termios_baud_rate(termios) for the clipped rate, hitting
WARN_ON(1) and returning 0, which then 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 retry count in uart_get_baud_rate() from 2 to 3 iterations so
that clipped baud rates are re-evaluated in the third iteration.

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
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Assisted-by: LLM
Signed-off-by: Hui Peng <benquike@gmail.com>
---
 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

[PATCH v7 2/2] serial: core: reject baud_base values that overflow port->uartclk in uart_set_info()

[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")
Fixes: 6eabce6608d6 ("serial: core: check uartclk for zero to avoid divide by zero")
Cc: stable@vger.kernel.org
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Assisted-by: LLM
Signed-off-by: Hui Peng <benquike@gmail.com>
---
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

Re: [PATCH v7 2/2] serial: core: reject baud_base values that overflow port->uartclk in uart_set_info()

[Andy Shevchenko]

On Wed, Sep 30, 2026 at 12:59:01PM +0000, Hui Peng wrote:
> 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.

...

>  	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;

Don't move the variable definition (by a location) without need. I do not
see any need of doing it like that. Moreover this change will add a (style)
regression, id est unneeded blank line. So, just drop the assignment and
leave the definition as it's now.

> +		if (check_mul_overflow(new_info->baud_base, 16, &uartclk) ||
> +		    uartclk == 0)
>  			return -EINVAL;
>  	}

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v7 0/2] serial: core: fix baud rate fallback loop and baud_base overflow

[Greg Kroah-Hartman]

On Wed, Sep 30, 2026 at 12:58:59PM +0000, Hui Peng wrote:
> 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.
> 
> Note on v6 1/3:
> Patch 1/3 from v6 ("serial: 8250: fix deadlock in serial8250_register_ports")
> has been dropped. The hash_mutex race condition was originally found and
> reproduced on Linux 6.18.14 (where guard(mutex)(&hash_mutex) was dropped
> inside serial_get_or_create_irq_info() before serial_link_irq_chain() linked
> the port). We did not realize that it was already fixed in mainline by
> commit b339809edda1 ("serial: 8250: use guard()s"), which refactored to
> serial_get_or_create_irq_info_locked() with guard(mutex)(&hash_mutex) moved
> into serial_link_irq_chain(). Apologies for the noise in v6 1/3.
> 
> All remaining patches are ported to the latest tip of mainline (551c722f4080)
> and dynamically verified in QEMU KVM with CONFIG_KASAN=y.
> 
> Changes in v7:
> - Dropped patch 1/3 (already resolved in mainline).
> - Clarified in cover letter that 1/3 was found and reproduced on 6.18.14 and
>   we did not realize it was already fixed in mainline.
> - Retained patch 1/2 and patch 2/2 using check_mul_overflow() as requested
>   by Jiri Slaby.
> 
> 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

You have sent 3 versions of this series in one day.  Please relax and
slow down.  I've now deleted all of these patches from my review queue.
Please wait a few days, take the time to make sure the submission is
correct, and then send a new version.  Don't overwhelm reviewers without
helping them with review of other submissions, to do otherwise is not
very kind.

thanks,

greg k-h

Re: [PATCH v7 1/2] serial: core: fix baud rate fallback in uart_get_baud_rate()

[Greg Kroah-Hartman]

On Wed, Sep 30, 2026 at 12:59:00PM +0000, Hui Peng wrote:
> When uart_get_baud_rate() is called with a baud rate exceeding the port's
> maximum supported speed (port->uartclk / 16), it clips baud to [min,
> max - 1] and encodes it into termios via tty_termios_encode_baud_rate().
> 
> However, because the loop bound is for (try = 0; try < 2; try++), the
> loop terminates immediately after try == 1 without re-evaluating

But try == 1 should keep the loop going as it is < 2, right?  What am I
missing here?  Do I need more coffee?

> baud = tty_termios_baud_rate(termios) for the clipped rate, hitting
> WARN_ON(1) and returning 0, which then 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 retry count in uart_get_baud_rate() from 2 to 3 iterations so
> that clipped baud rates are re-evaluated in the third iteration.

What is the magic 2 here, and why turning it into a magic 3 somehow fix
things?

thanks,

greg k-h

Re: [PATCH v7 2/2] serial: core: reject baud_base values that overflow port->uartclk in uart_set_info()

[Greg Kroah-Hartman]

On Wed, Sep 30, 2026 at 12:59:01PM +0000, Hui Peng wrote:
> 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")
> Fixes: 6eabce6608d6 ("serial: core: check uartclk for zero to avoid divide by zero")
> Cc: stable@vger.kernel.org
> Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> Assisted-by: LLM
> Signed-off-by: Hui Peng <benquike@gmail.com>
> ---
> 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;

As Andy said, this style change is not ok.

Please slow down and be more careful.

thanks,

greg k-h

Re: [PATCH v7 1/2] serial: core: fix baud rate fallback in uart_get_baud_rate()

[Ilpo Järvinen]

On Thu, 1 Oct 2026, Greg Kroah-Hartman wrote:

> On Wed, Sep 30, 2026 at 12:59:00PM +0000, Hui Peng wrote:
> > When uart_get_baud_rate() is called with a baud rate exceeding the port's
> > maximum supported speed (port->uartclk / 16), it clips baud to [min,
> > max - 1] and encodes it into termios via tty_termios_encode_baud_rate().
> > 
> > However, because the loop bound is for (try = 0; try < 2; try++), the
> > loop terminates immediately after try == 1 without re-evaluating
> 
> But try == 1 should keep the loop going as it is < 2, right?  What am I
> missing here?  Do I need more coffee?
>
> > baud = tty_termios_baud_rate(termios) for the clipped rate, hitting
> > WARN_ON(1) and returning 0, which then 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 retry count in uart_get_baud_rate() from 2 to 3 iterations so
> > that clipped baud rates are re-evaluated in the third iteration.
> 
> What is the magic 2 here, and why turning it into a magic 3 somehow fix
> things?

Hi Greg & Hui,

First of all, I'm withdrawing my Reviewed-by from this!!!

Lets hope the submitter can finally get his/her act together and not make 
unlisted changes between versions or send non-sense.

While the code change is still fine, it seems the submitter (or more 
likely AI) has changed the changelog from what I read when I reviewed 
this. And the new one is way worse than it used to be so not being able 
to follow what's going on is very understandable given the lackluster 
explanation that remains.


What you're missing is that the baud returns happens within the loop, so:

try == 0: use new, if baud is out of bound and old is available, switch to 
          old
try == 1: if old is also out of bounds, there's the last resort rule 
          towards the end of the loop which is applied forcing baud to the 
          accetable range.
try == 2: loop exits => WARN_ON(1) triggers.

What we'd want to happen with try == 2, is for it to use the return baud 
which is within the loop body.


Hui, I suggest keeping the previous versions of the patches available as 
files. And right before sending the next version, go manually throught the 
diff of diffs to make sure there are ZERO unexpected changes from version 
to version (has saved me tons of times from making stupid mistakes).


-- 
 i.