* [PATCH v2 0/1] serial: core: Don't clobber the baud rate on hangup via B0 @ 2026-10-12 1:49 Maciej W. Rozycki 2026-10-12 1:49 ` [PATCH v2 1/1] " Maciej W. Rozycki 0 siblings, 1 reply; 5+ messages in thread From: Maciej W. Rozycki @ 2026-10-12 1:49 UTC (permalink / raw) To: Greg Kroah-Hartman, Jiri Slaby Cc: Hugo Villeneuve, Kees Cook, Lukas Wunner, John Ogness, Jiayuan Chen, Gerhard Engleder, linux-serial, linux-kernel [-- Attachment #1: Type: text/plain, Size: 787 bytes --] Greg, As we discussed at LPC, this is an update to the fix previously posted as an RFC. I have verified it with the system originally affected, running the console at 115200bps; see the attached console output logs for the bad vs fixed case. All that needed to be done to reproduce it was to keep killing getty(8)s repeatedly until init(8) decided something was wrong and chose to put their inittab(5) entries into a snooze, leaving the line(s) at the wrong speed setting. NB this fixes the missing %B0 literal markup as a part of the doc update. I've concluded it's not worth splitting off into a separate patch. Let me know if you think otherwise. Previous iteration (v1) at <https://lore.kernel.org/r/alpine.DEB.2.21.2605242242330.1450@angie.orcam.me.uk/>. Maciej [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: Type: text/plain; CHARSET=US-ASCII; NAME=swarm-bad.log, Size: 1526 bytes --] ^[[H^[[J\rswarm.orcam.me.uk login: \0þ\0þ\0\0\0\0\0\0þ\0\0ð\0\0ð\0þ\0\0ð\0\0ð\0þ\0þ\0\0ð\0\0ð\0þ\0\0\0À\0þ\0þ\0þ\0\0ð\0þ\0\0ð\0\0\0\0\0\0\0ø\0\0ð\0þ\0\0ð\0\0ð\0þ\0\0\0À\0\0ð\0þ\0\0\0\0\0\0\0ø\0\0ð\0\0ð\0\0\0À\0\0ð\0þ\0\0ð\0þ\0þ\0\0\0\0\0\0þ\0\0ð\0\0ð\0\0\0À\0\0ð\0\0\0\0\0\0\0ø\0\0ð\0\0ð\0\0ð\0þ\0þ\0þ\0\0ð\0þ\0þ\0\0ð\0þ\0\0\0\0\0\0þ\0þ\0þ\0\0\0\0\0\0þ\0þ\0þ\0þ\0\0ð\0þ\0þ\0þ\0\0ð\0þ\0þ\0\0ð\0þ\0þ\0þ\0\0ð\0þ\0\0\0\0\0\0\0ð\0\0ð\0\0\0À\0þ\0þ\0þ\0þ\0þ\0þ\0þ\0þ\0\0\0\0\0\0\0ð\0\0ð\0\0ð\0\0ð\0þ\0þ\0\0\0\0\0\0þ\0þ\0\0ð\0þ\0\0\0À\0þ\0þ\0\0ð\0þ\0\0ð\0\0\0\0\0\0\0ð\0\0ð\0\0\0À\0\0ð\0þ\0þ\0\0ð\0þ\0þ\0þ\0\0ð\0þ\0þ\0\0\0\0\0\0þ\0\0ð\0\0\0À\0þ\0\0\0À\0þ\0þ\0þ\0þ\0\0ð\0þ\0\0\0À\0\0ð\0þ\0\0\0\0\0\0\0ø\0\0ð\0\0ð\0\0ð\0þ\0þ\0þ\0þ\0\0ð\0\0ð\0þ\0\0\0\0\0\0\0ø\0\0ð\0þ\0þ\0þ\0\0ð\0\0\0\0\0\0\0ø\0\0ð\0þ\0þ\0þ\0þ\0þ\0\0ð\0þ\0þ\0\0ð\0þ\0þ\0þ\0þ\0þ\0þ\0\0\0À\0þ\0þ\0þ\0þ\0\0ð\0þ\0þ\0\0ð\0þ\0þ\0þ\0\0\0\0\0\0\0ð\0þ\0\0\0\0\0\0þ\0þ\0\0\0\0\0\0þ\0\0ð\0\0ð\0þ\0\0ð\0\0ð\0þ\0þ\0\0ð\0\0ð\0þ\0\0\0À\0þ\0þ\0þ\0\0ð\0þ\0\0ð\0\0\0\0\0\0\0ø\0\0ð\0þ\0\0ð\0\0ð\0þ\0\0\0À\0\0ð\0þ\0\0\0\0\0\0\0ð\0\0ð\0\0ð\0\0\0À\0\0ð\0þ\0\0ð\0þ\0þ\0þ\0\0\0À\0\0ð\0\0ð\0\0\0À\0\0ð\0\0\0\0\0\0\0ð\0\0ð\0\0ð\0\0ð\0þ\0þ\0þ\0\0ð\0þ\0þ\0\0ð\0þ\0\0\0\0\0\0þ\0þ\0þ\0\0\0\0\0\0þ\0þ\0þ\0þ\0\0ð\0þ\0þ\0þ\0\0ð\0þ\0þ\0\0ð\0þ\0þ\0þ\0\0ð\0þ\0\0\0\0\0\0\0ø\0\0ð\0\0\0À\0þ\0þ\0þ\0þ\0þ\0þ\0þ\0þ\0\0\0\0\0\0\0ø\0\0ð\0\0ð\0\0ð\0þ\0þ\0\0\0\0\0\0þ\0þ\0\0ð\0þ\0\0\0À\0þ\0þ\0\0ð\0þ\0\0ð\0\0\0\0\0\0\0ø\0\0ð\0\0\0À\0\0ð\0þ\0þ\0\0ð\0þ\0þ\0þ\0\0ð\0þ\0þ\0\0\0\0\0\0þ\0\0ð\0\0\0À\0þ\0\0\0À\0þ\0þ\0þ\0þ\0\0ð\0þ\0\0\0À\0\0ð\0þ\0\0\0\0\0\0\0ð\0\0ð\0\0ð\0\0ð\0þ\0þ\0þ\0þ\0\0ð\0\0ð\0þ\0\0\0\0\0\0\0ð\0\0ð\0þ\0þ\0þ\0\0ð\0\0\0\0\0\0\0ø\0\0ð\0þ\0þ\0þ\0þ\0þ\0\0ð\0þ\0þ\0\0ð\0þ\0þ\0þ\0þ\0þ\0þ\0\0\0À\0þ\0þ\0þ\0þ\0\0ð\0þ\0þ\0\0ð\0þ\0þ\0þ\0\0\0\0\0\0\0ð\0þ\0\0\0\0\0\0þ\0þ\0\0\0\0\0\0þ\0\0ð\0\0ð\0þ\0\0ð\0\0ð\0þ\0þ\0\0ð\0\0ð\0þ\0\0\0À\0þ\0þ\0þ\0\0ð\0þ\0\0ð\0\0\0\0\0\0\0ð\0\0ð\0\0ð\0þ\0þ\0þ\0þ\0þ\0\0\0\0\0\0\0ø\0\0ð\0þ\0þ\0þ\0þ\0þ\0þ\0þ\0\0ð\0\0ð\0þ\0þ\0þ\0\0ð\0þ\0\0\0\0\0\0\0ø\0\0ð\0\0\0\0\0\0þ\0þ\0\0ð\0\0ð\0þ\0þ\0þ\0þ\0þ\0\0\0À\0þ\0þ\0þ\0\0ð\0þ\0þ\0\0ð\0þ\0þ\0\0ð\0þ\0þ\0þ\0\0ð\0þ\0þ\0\0ð\0þ\0\0\0\0\0\0\0ø\0\0ð\0\0\0À\0þ\0þ\0þ\0þ\0\0ð\0þ\0\0ð\0\0ð\0þ\0\0\0À\0þ\0þ\0\0\0\0\0\0\0ð\0\0ð\0þ\0\0ð\0þ\0þ\0\0ð\0þ\0þ\0\0\0\0\0\0\0ð\0\0ð\0\0\0À\0þ\0þ\0\0\0\0\0\0þ\0þ\0þ\0\0ð\0þ\0þ\0þ\0\0ð\0þ\0\0\0\0\0\0\0ø\0\0ð\0\0ð\0\0ð\0þ\0þ\0þ\0þ\0þ\0\0ð\0þ\0þ\0\0\0À\0þ\0þ\0þ\0þ\0\0ð\0þ\0\0ð\0þ\0þ\0þ\0þ\0\0ð\0þ\0\0\0À\0þ\0þ\0þ\0þ\0\0\0\0\0\0\0ð\0þ\0\0\0\0\0^[[H^[[J\rswarm.orcam.me.uk login: [-- Attachment #3: Type: text/plain, Size: 229 bytes --] ^[[H^[[Jswarm.orcam.me.uk login: INIT: Id "S0" respawning too fast: disabled for 5 minutes INIT: Id "S1" respawning too fast: disabled for 5 minutes INIT: no more processes left in this runlevel ^[[H^[[Jswarm.orcam.me.uk login: ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2 1/1] serial: core: Don't clobber the baud rate on hangup via B0 2026-10-12 1:49 [PATCH v2 0/1] serial: core: Don't clobber the baud rate on hangup via B0 Maciej W. Rozycki @ 2026-10-12 1:49 ` Maciej W. Rozycki 2026-10-12 2:10 ` sashiko-bot 0 siblings, 1 reply; 5+ messages in thread From: Maciej W. Rozycki @ 2026-10-12 1:49 UTC (permalink / raw) To: Greg Kroah-Hartman, Jiri Slaby Cc: Hugo Villeneuve, Kees Cook, Lukas Wunner, John Ogness, Jiayuan Chen, Gerhard Engleder, linux-serial, linux-kernel Requesting hangup via the B0 baud rate has this nasty side effect of clobbering the previously set rate for the port unless it happens to be 9600bps or the port minimum rate if higher. Consequently if there's a consumer still active at the other end, then it receives garbled data from any other producers outputting messages to the port, such as init(8) or the kernel console. There doesn't appear to be any particular reason for this clobbering other than that we have been doing it since 2.1.131, so take a saner approach and keep track of the most recently set baud rate and then use it whenever hangup has been requested. Signed-off-by: Maciej W. Rozycki <macro@orcam.me.uk> --- Changes from v1, <https://lore.kernel.org/r/alpine.DEB.2.21.2605242242330.1450@angie.orcam.me.uk/>: - Cache the most recently set actual baud rate in port->hup_speed and use it whenever the B0 rate has been requested since the old termios may be NULL or have the B0 rate recorded. --- drivers/tty/serial/serial_core.c | 16 +++++++++------- include/linux/serial_core.h | 1 + 2 files changed, 10 insertions(+), 7 deletions(-) linux-serial-uart-get-baud-rate-hup.diff Index: linux-macro/drivers/tty/serial/serial_core.c =================================================================== --- linux-macro.orig/drivers/tty/serial/serial_core.c +++ linux-macro/drivers/tty/serial/serial_core.c @@ -461,15 +461,15 @@ EXPORT_SYMBOL(uart_update_timeout); * @max: maximum acceptable baud rate * * Decode the termios structure into a numeric baud rate, taking account of the - * magic 38400 baud rate (with spd_* flags), and mapping the %B0 rate to 9600 - * baud or min argument, whichever is greater. + * magic 38400 baud rate (with spd_* flags), and mapping the %B0 rate to the + * previously used rate provided by @port. * * 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"). + * The @termios and @port structures are 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"). * * Locking: caller dependent */ @@ -514,10 +514,12 @@ uart_get_baud_rate(struct uart_port *por * Special case: B0 rate. */ if (baud == 0) - return max(min, 9600); + return port->hup_speed; - if (baud >= min && baud <= max) + if (baud >= min && baud <= max) { + port->hup_speed = baud; return baud; + } /* * If the range cannot be met then try again with Index: linux-macro/include/linux/serial_core.h =================================================================== --- linux-macro.orig/include/linux/serial_core.h +++ linux-macro/include/linux/serial_core.h @@ -569,6 +569,7 @@ struct uart_port { bool hw_stopped; /* sw-assisted CTS flow state */ bool cons_flow; /* user specified console flow control */ + speed_t hup_speed; /* speed to use for B0 rate (hang-up) */ unsigned int mctrl; /* current modem ctrl settings */ unsigned int frame_time; /* frame timing in ns */ unsigned int type; /* port type */ ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 1/1] serial: core: Don't clobber the baud rate on hangup via B0 2026-10-12 1:49 ` [PATCH v2 1/1] " Maciej W. Rozycki @ 2026-10-12 2:10 ` sashiko-bot 2026-10-12 14:37 ` Maciej W. Rozycki 0 siblings, 1 reply; 5+ messages in thread From: sashiko-bot @ 2026-10-12 2:10 UTC (permalink / raw) To: Maciej W. Rozycki; +Cc: linux-serial Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] tty: division by zero in uart_get_divisor() on uninitialized port->hup_speed - [Medium] tty: serial: data race on port->hup_speed in uart_get_baud_rate() -- commit 3c831138bb5fb81f1a02fc66246315af11bf6bbc Author: Maciej W. Rozycki <macro@orcam.me.uk> serial: core: Don't clobber the baud rate on hangup via B0 Requesting hangup via the B0 baud rate clobbers the previously set rate unless it is 9600bps or the port minimum. This commit tracks the most recently set baud rate in port->hup_speed and uses it whenever the B0 rate is requested. > diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c > index 319a4b427f3a3..3390e4d142127 100644 > --- a/drivers/tty/serial/serial_core.c > +++ b/drivers/tty/serial/serial_core.c [ ... ] > @@ -514,10 +514,12 @@ uart_get_baud_rate(struct uart_port *port, struct ktermios *termios, > * Special case: B0 rate. > */ > if (baud == 0) > - return max(min, 9600); > + return port->hup_speed; [Severity: High] Does this risk a division by zero panic? If a new struct uart_port is initialized, port->hup_speed defaults to 0. If a B0 rate is requested before any valid rate is set (such as via console=ttyS0,0 or an early tcsetattr with B0), this returns 0. This 0 is then passed down to helpers like uart_get_divisor(), which calculates the divisor as: quot = DIV_ROUND_CLOSEST(port->uartclk, 16 * baud); It can also reach uart_update_timeout(), which calculates the frame time: port->frame_time = (unsigned int)DIV64_U64_ROUND_UP(temp, baud); In both cases, passing a 0 baud rate will result in a divide-by-zero hardware exception. > > - if (baud >= min && baud <= max) > + if (baud >= min && baud <= max) { > + port->hup_speed = baud; [Severity: Medium] Could this result in a data race on port->hup_speed? In the standard uart_set_termios() path, uart_get_baud_rate() is called holding only port->mutex and termios_rwsem, not port->lock. At the same time, atomic console write paths execute asynchronously from hardirq or NMI holding only the nbcon lock. For example: drivers/tty/serial/8250/8250_port.c:serial8250_console_restore() { ... baud = serial8250_get_baud_rate(port, &termios, NULL); ... } This calls into uart_get_baud_rate() without holding port->mutex. A concurrent tcsetattr() and atomic console write can trigger an unprotected data race on port->hup_speed. A torn read or write on this variable could yield a garbage baud rate and lead to hardware misconfiguration. > return baud; > + } > > /* > * If the range cannot be met then try again with -- Sashiko AI review · https://sashiko.dev/#/patchset/alpine.DEB.2.21.2610120150230.55767@angie.orcam.me.uk?part=1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 1/1] serial: core: Don't clobber the baud rate on hangup via B0 2026-10-12 2:10 ` sashiko-bot @ 2026-10-12 14:37 ` Maciej W. Rozycki 2026-10-13 13:16 ` Greg Kroah-Hartman 0 siblings, 1 reply; 5+ messages in thread From: Maciej W. Rozycki @ 2026-10-12 14:37 UTC (permalink / raw) To: sashiko-reviews Cc: Greg Kroah-Hartman, Jiri Slaby, Hugo Villeneuve, Kees Cook, Lukas Wunner, John Ogness, Jiayuan Chen, Gerhard Engleder, linux-serial, linux-kernel On Mon, 12 Oct 2026, sashiko-bot@kernel.org wrote: > > @@ -514,10 +514,12 @@ uart_get_baud_rate(struct uart_port *port, struct ktermios *termios, > > * Special case: B0 rate. > > */ > > if (baud == 0) > > - return max(min, 9600); > > + return port->hup_speed; > > [Severity: High] > Does this risk a division by zero panic? > > If a new struct uart_port is initialized, port->hup_speed defaults to 0. > If a B0 rate is requested before any valid rate is set (such as via > console=ttyS0,0 or an early tcsetattr with B0), this returns 0. Yeah, this has glimpsed in my mind and then I lost it. Indeed we need to keep the fallback 9600/min logic after all for the initial setup. Easily fixed since it's not in the hot path. > > > > - if (baud >= min && baud <= max) > > + if (baud >= min && baud <= max) { > > + port->hup_speed = baud; > > [Severity: Medium] > Could this result in a data race on port->hup_speed? > > In the standard uart_set_termios() path, uart_get_baud_rate() is called > holding only port->mutex and termios_rwsem, not port->lock. > > At the same time, atomic console write paths execute asynchronously from > hardirq or NMI holding only the nbcon lock. For example: > > drivers/tty/serial/8250/8250_port.c:serial8250_console_restore() { > ... > baud = serial8250_get_baud_rate(port, &termios, NULL); > ... > } > > This calls into uart_get_baud_rate() without holding port->mutex. A > concurrent tcsetattr() and atomic console write can trigger an unprotected > data race on port->hup_speed. A torn read or write on this variable could > yield a garbage baud rate and lead to hardware misconfiguration. This seems like the only exception across all the serial drivers in the use of uart_get_baud_rate(). Since this is always called to restore the most recent baud rate setting only, perhaps the path of least resistance would be just to refrain from updating port->hup_speed if called with the port lock held? Otherwise it would require rewriting all the 60+ call sites to use the port lock to guard the whole sequence from a call to uart_get_baud_rate() through to the write of the settings obtained to the port registers, which seems an overkill to me and asking for trouble with some odd hardware. I'll experiment with the idea outlined or will be happy with any hints otherwise. Maciej ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 1/1] serial: core: Don't clobber the baud rate on hangup via B0 2026-10-12 14:37 ` Maciej W. Rozycki @ 2026-10-13 13:16 ` Greg Kroah-Hartman 0 siblings, 0 replies; 5+ messages in thread From: Greg Kroah-Hartman @ 2026-10-13 13:16 UTC (permalink / raw) To: Maciej W. Rozycki Cc: sashiko-reviews, Jiri Slaby, Hugo Villeneuve, Kees Cook, Lukas Wunner, John Ogness, Jiayuan Chen, Gerhard Engleder, linux-serial, linux-kernel On Mon, Oct 12, 2026 at 03:37:01PM +0100, Maciej W. Rozycki wrote: > On Mon, 12 Oct 2026, sashiko-bot@kernel.org wrote: > > > > @@ -514,10 +514,12 @@ uart_get_baud_rate(struct uart_port *port, struct ktermios *termios, > > > * Special case: B0 rate. > > > */ > > > if (baud == 0) > > > - return max(min, 9600); > > > + return port->hup_speed; > > > > [Severity: High] > > Does this risk a division by zero panic? > > > > If a new struct uart_port is initialized, port->hup_speed defaults to 0. > > If a B0 rate is requested before any valid rate is set (such as via > > console=ttyS0,0 or an early tcsetattr with B0), this returns 0. > > Yeah, this has glimpsed in my mind and then I lost it. Indeed we need to > keep the fallback 9600/min logic after all for the initial setup. Easily > fixed since it's not in the hot path. > > > > > > > - if (baud >= min && baud <= max) > > > + if (baud >= min && baud <= max) { > > > + port->hup_speed = baud; > > > > [Severity: Medium] > > Could this result in a data race on port->hup_speed? > > > > In the standard uart_set_termios() path, uart_get_baud_rate() is called > > holding only port->mutex and termios_rwsem, not port->lock. > > > > At the same time, atomic console write paths execute asynchronously from > > hardirq or NMI holding only the nbcon lock. For example: > > > > drivers/tty/serial/8250/8250_port.c:serial8250_console_restore() { > > ... > > baud = serial8250_get_baud_rate(port, &termios, NULL); > > ... > > } > > > > This calls into uart_get_baud_rate() without holding port->mutex. A > > concurrent tcsetattr() and atomic console write can trigger an unprotected > > data race on port->hup_speed. A torn read or write on this variable could > > yield a garbage baud rate and lead to hardware misconfiguration. > > This seems like the only exception across all the serial drivers in the > use of uart_get_baud_rate(). Since this is always called to restore the > most recent baud rate setting only, perhaps the path of least resistance > would be just to refrain from updating port->hup_speed if called with the > port lock held? > > Otherwise it would require rewriting all the 60+ call sites to use the > port lock to guard the whole sequence from a call to uart_get_baud_rate() > through to the write of the settings obtained to the port registers, which > seems an overkill to me and asking for trouble with some odd hardware. > > I'll experiment with the idea outlined or will be happy with any hints > otherwise. That idea seems sane, thanks for working on this. greg k-h ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-10-13 13:16 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-10-12 1:49 [PATCH v2 0/1] serial: core: Don't clobber the baud rate on hangup via B0 Maciej W. Rozycki 2026-10-12 1:49 ` [PATCH v2 1/1] " Maciej W. Rozycki 2026-10-12 2:10 ` sashiko-bot 2026-10-12 14:37 ` Maciej W. Rozycki 2026-10-13 13:16 ` Greg Kroah-Hartman
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox