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

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

[Hugo Villeneuve]

On Thu, 1 Oct 2026 12:46:05 +0300 (EEST)
Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> wrote:

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

Hi all,
is it at all possible that old is out of bounds in the first place? If
yes, is it something that should be fixed?

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

Then should the "return 0" statement be modified to "return baud", and
possibly the WARN_ON() removed? Then you wouldn't need to increase
max try?


-- 
Hugo Villeneuve

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

[Hugo Villeneuve]

Hi Hui,

On Wed, 30 Sep 2026 12:59:01 +0000
Hui Peng <benquike@gmail.com> 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")

The final statement that returns zero baud rate was already present
before 6eabce6608d6. My commit added the warning, so I am not
sure if this additional Fixes tag is justified?


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


-- 
Hugo Villeneuve

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

[Ilpo Järvinen]

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

On Tue, 6 Oct 2026, Hugo Villeneuve wrote:

> On Thu, 1 Oct 2026 12:46:05 +0300 (EEST)
> Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> wrote:
> 
> > 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.
> 
> Hi all,
> is it at all possible that old is out of bounds in the first place?

Apparently it is, given the WARNING above (unless that too is fabricate 
by LLM which seems well within possiblities given the track record of this 
particular submitter).

> If yes, is it something that should be fixed?

I agree.

I think the scenario here is (but take it with grain of salt, as the 
information seems to be constantly changing thanks to LLM/submitter 
failing to keep one's act to together) [1]:

Tested in QEMU against Linux 7.3.0-rc3 by setting /dev/ttyS1 to B115200,
lowering baud_base to 9600 (max = 9600) via TIOCSSERIAL, and calling
tcsetattr() with B57600 (old = B115200), reproducing the WARNING and
Oops: divide error in uart_get_divisor() on the unfixed kernel and
verifying clean execution with 0 warnings/faults with the fix applied.

So when rate gets lowered, it should alter the termios to prevent what is 
here seem as "old" having an invalid value.

[1] https://lore.kernel.org/linux-serial/20260930041216.155911-3-benquike@gmail.com/

> > 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.
> 
> Then should the "return 0" statement be modified to "return baud", and
> possibly the WARN_ON() removed? Then you wouldn't need to increase
> max try?

IMO, the entire loop constructs feels somewhat artificial in this case so 
I'd prefer to kill the loop entirely. But such refactoring is not going to 
be a minimal fix to the issue.

-- 
 i.

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

[Hugo Villeneuve]

Hi Ilpo,

On Wed, 7 Oct 2026 13:38:55 +0300 (EEST)
Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> wrote:

> On Tue, 6 Oct 2026, Hugo Villeneuve wrote:
> 
> > On Thu, 1 Oct 2026 12:46:05 +0300 (EEST)
> > Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> wrote:
> > 
> > > 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.
> > 
> > Hi all,
> > is it at all possible that old is out of bounds in the first place?
> 
> Apparently it is, given the WARNING above (unless that too is fabricate 
> by LLM which seems well within possiblities given the track record of this 
> particular submitter).
> 
> > If yes, is it something that should be fixed?
> 
> I agree.
> 
> I think the scenario here is (but take it with grain of salt, as the 
> information seems to be constantly changing thanks to LLM/submitter 
> failing to keep one's act to together) [1]:
> 
> Tested in QEMU against Linux 7.3.0-rc3 by setting /dev/ttyS1 to B115200,
> lowering baud_base to 9600 (max = 9600) via TIOCSSERIAL, and calling
> tcsetattr() with B57600 (old = B115200), reproducing the WARNING and
> Oops: divide error in uart_get_divisor() on the unfixed kernel and
> verifying clean execution with 0 warnings/faults with the fix applied.
> 
> So when rate gets lowered, it should alter the termios to prevent what is 
> here seem as "old" having an invalid value.

That makes sense...

> 
> [1] https://lore.kernel.org/linux-serial/20260930041216.155911-3-benquike@gmail.com/
> 
> > > 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.
> > 
> > Then should the "return 0" statement be modified to "return baud", and
> > possibly the WARN_ON() removed? Then you wouldn't need to increase
> > max try?
> 
> IMO, the entire loop constructs feels somewhat artificial in this case so 
> I'd prefer to kill the loop entirely. But such refactoring is not going to 
> be a minimal fix to the issue.

Yes, when I updated uart_get_baud_rate() a few months ago, the loop
also felt a little bit weird, but I did not remove it because I assumed
it was working ok if we assumed, like I did, that the old baud rate was
always valid.

I tried in the past to simplify it, make it more logical and
understandable by normal humans :) But I always end up with
something not so simple and obvious because of all the special cases
(B0, spd_* flags, etc).

I am still trying though, maybe I will have better luck this time :)


-- 
Hugo Villeneuve

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

[Ilpo Järvinen]

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

On Wed, 7 Oct 2026, Hugo Villeneuve wrote:

> Hi Ilpo,
> 
> On Wed, 7 Oct 2026 13:38:55 +0300 (EEST)
> Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> wrote:
> 
> > On Tue, 6 Oct 2026, Hugo Villeneuve wrote:
> > 
> > > On Thu, 1 Oct 2026 12:46:05 +0300 (EEST)
> > > Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> wrote:
> > > 
> > > > 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.
> > > 
> > > Hi all,
> > > is it at all possible that old is out of bounds in the first place?
> > 
> > Apparently it is, given the WARNING above (unless that too is fabricate 
> > by LLM which seems well within possiblities given the track record of this 
> > particular submitter).
> > 
> > > If yes, is it something that should be fixed?
> > 
> > I agree.
> > 
> > I think the scenario here is (but take it with grain of salt, as the 
> > information seems to be constantly changing thanks to LLM/submitter 
> > failing to keep one's act to together) [1]:
> > 
> > Tested in QEMU against Linux 7.3.0-rc3 by setting /dev/ttyS1 to B115200,
> > lowering baud_base to 9600 (max = 9600) via TIOCSSERIAL, and calling
> > tcsetattr() with B57600 (old = B115200), reproducing the WARNING and
> > Oops: divide error in uart_get_divisor() on the unfixed kernel and
> > verifying clean execution with 0 warnings/faults with the fix applied.
> > 
> > So when rate gets lowered, it should alter the termios to prevent what is 
> > here seem as "old" having an invalid value.
> 
> That makes sense...
> 
> > 
> > [1] https://lore.kernel.org/linux-serial/20260930041216.155911-3-benquike@gmail.com/
> > 
> > > > 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.
> > > 
> > > Then should the "return 0" statement be modified to "return baud", and
> > > possibly the WARN_ON() removed? Then you wouldn't need to increase
> > > max try?
> > 
> > IMO, the entire loop constructs feels somewhat artificial in this case so 
> > I'd prefer to kill the loop entirely. But such refactoring is not going to 
> > be a minimal fix to the issue.
> 
> Yes, when I updated uart_get_baud_rate() a few months ago, the loop
> also felt a little bit weird, but I did not remove it because I assumed
> it was working ok if we assumed, like I did, that the old baud rate was
> always valid.
> 
> I tried in the past to simplify it, make it more logical and
> understandable by normal humans :) But I always end up with
> something not so simple and obvious because of all the special cases
> (B0, spd_* flags, etc).

Trying to all that in the same function surely gets problematic and 
repetivive. But how about adding another function/helper that is just 
called multiple times to avoid duplicating them?

> I am still trying though, maybe I will have better luck this time :)
> 
> 
> 

-- 
 i.

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

[Hugo Villeneuve]

Hi Ilpo,

On Wed, 7 Oct 2026 23:09:26 +0300 (EEST)
Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> wrote:

> On Wed, 7 Oct 2026, Hugo Villeneuve wrote:
> 
> > Hi Ilpo,
> > 
> > On Wed, 7 Oct 2026 13:38:55 +0300 (EEST)
> > Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> wrote:
> > 
> > > On Tue, 6 Oct 2026, Hugo Villeneuve wrote:
> > > 
> > > > On Thu, 1 Oct 2026 12:46:05 +0300 (EEST)
> > > > Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> wrote:
> > > > 
> > > > > 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.
> > > > 
> > > > Hi all,
> > > > is it at all possible that old is out of bounds in the first place?
> > > 
> > > Apparently it is, given the WARNING above (unless that too is fabricate 
> > > by LLM which seems well within possiblities given the track record of this 
> > > particular submitter).
> > > 
> > > > If yes, is it something that should be fixed?
> > > 
> > > I agree.
> > > 
> > > I think the scenario here is (but take it with grain of salt, as the 
> > > information seems to be constantly changing thanks to LLM/submitter 
> > > failing to keep one's act to together) [1]:
> > > 
> > > Tested in QEMU against Linux 7.3.0-rc3 by setting /dev/ttyS1 to B115200,
> > > lowering baud_base to 9600 (max = 9600) via TIOCSSERIAL, and calling
> > > tcsetattr() with B57600 (old = B115200), reproducing the WARNING and
> > > Oops: divide error in uart_get_divisor() on the unfixed kernel and
> > > verifying clean execution with 0 warnings/faults with the fix applied.
> > > 
> > > So when rate gets lowered, it should alter the termios to prevent what is 
> > > here seem as "old" having an invalid value.
> > 
> > That makes sense...
> > 
> > > 
> > > [1] https://lore.kernel.org/linux-serial/20260930041216.155911-3-benquike@gmail.com/
> > > 
> > > > > 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.
> > > > 
> > > > Then should the "return 0" statement be modified to "return baud", and
> > > > possibly the WARN_ON() removed? Then you wouldn't need to increase
> > > > max try?
> > > 
> > > IMO, the entire loop constructs feels somewhat artificial in this case so 
> > > I'd prefer to kill the loop entirely. But such refactoring is not going to 
> > > be a minimal fix to the issue.
> > 
> > Yes, when I updated uart_get_baud_rate() a few months ago, the loop
> > also felt a little bit weird, but I did not remove it because I assumed
> > it was working ok if we assumed, like I did, that the old baud rate was
> > always valid.
> > 
> > I tried in the past to simplify it, make it more logical and
> > understandable by normal humans :) But I always end up with
> > something not so simple and obvious because of all the special cases
> > (B0, spd_* flags, etc).
> 
> Trying to all that in the same function surely gets problematic and 
> repetivive. But how about adding another function/helper that is just 
> called multiple times to avoid duplicating them?

That is exactly what I have been trying all day :) I implemented a
uart_validate_baud_rate() helper. Still testing all corner cases...


> > I am still trying though, maybe I will have better luck this time :)
>  i.


-- 
Hugo Villeneuve