On Fri, Oct 15, 2021 at 01:14:20PM +0200, Johan Hovold wrote: > Commit 868f3ee6e452 ("serial: 8250: Add 8250 port clock update method") > added a hack to support SoCs where the UART reference clock can > change behind the back of the driver but failed to add the proper > locking. > > First, make sure to take a reference to the tty struct to avoid > dereferencing a NULL pointer if the clock change races with a hangup. > > Second, the termios semaphore must be held during the update to prevent > a racing termios change. Nice catch! Thanks, Johan, for fixing this! Acked-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> > Fixes: 868f3ee6e452 ("serial: 8250: Add 8250 port clock update method") > Fixes: c8dff3aa8241 ("serial: 8250: Skip uninitialized TTY port baud rate update") > Cc: stable@xxxxxxxxxxxxxxx # 5.9 > Cc: Serge Semin <Sergey.Semin@xxxxxxxxxxxxxxxxxxxx> > Signed-off-by: Johan Hovold <johan@xxxxxxxxxx> > --- > drivers/tty/serial/8250/8250_port.c | 21 +++++++++++++++++---- > 1 file changed, 17 insertions(+), 4 deletions(-) > > diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c > index 66374704747e..e4dd82fd7c2a 100644 > --- a/drivers/tty/serial/8250/8250_port.c > +++ b/drivers/tty/serial/8250/8250_port.c > @@ -2696,21 +2696,32 @@ static unsigned int serial8250_get_baud_rate(struct uart_port *port, > void serial8250_update_uartclk(struct uart_port *port, unsigned int uartclk) > { > struct uart_8250_port *up = up_to_u8250p(port); > + struct tty_port *tport = &port->state->port; > unsigned int baud, quot, frac = 0; > struct ktermios *termios; > + struct tty_struct *tty; > unsigned long flags; > > - mutex_lock(&port->state->port.mutex); > + tty = tty_port_tty_get(tport); > + if (!tty) { > + mutex_lock(&tport->mutex); > + port->uartclk = uartclk; > + mutex_unlock(&tport->mutex); > + return; > + } > + > + down_write(&tty->termios_rwsem); > + mutex_lock(&tport->mutex); > > if (port->uartclk == uartclk) > goto out_lock; > > port->uartclk = uartclk; > > - if (!tty_port_initialized(&port->state->port)) > + if (!tty_port_initialized(tport)) > goto out_lock; > > - termios = &port->state->port.tty->termios; > + termios = &tty->termios; > > baud = serial8250_get_baud_rate(port, termios, NULL); > quot = serial8250_get_divisor(port, baud, &frac); > @@ -2727,7 +2738,9 @@ void serial8250_update_uartclk(struct uart_port *port, unsigned int uartclk) > serial8250_rpm_put(up); > > out_lock: > - mutex_unlock(&port->state->port.mutex); > + mutex_unlock(&tport->mutex); > + up_write(&tty->termios_rwsem); > + tty_kref_put(tty); > } > EXPORT_SYMBOL_GPL(serial8250_update_uartclk); > > -- > 2.32.0 > -- With Best Regards, Andy Shevchenko