Re: [PATCH v2 1/1] serial: core: Don't clobber the baud rate on hangup via B0

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

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