linux-kernel - [PATCH 3/3] serial: 8250_dw: drop bogus uartclk optimisation

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 15 Oct 2021 13:14:22 +0200
From:   Johan Hovold <johan@...nel.org>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:     Jiri Slaby <jirislaby@...nel.org>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Serge Semin <Sergey.Semin@...kalelectronics.ru>,
        linux-serial@...r.kernel.org, linux-kernel@...r.kernel.org,
        Johan Hovold <johan@...nel.org>
Subject: [PATCH 3/3] serial: 8250_dw: drop bogus uartclk optimisation

The driver was updating the port uartclk before setting the new rate in
an attempt to avoid having the clock notifier redundantly update the
divisors.

The set_termios() callback is however called under the termios semaphore
and tty-port mutex so the worker scheduled by the clock notifier will
block in serial8250_update_uartclk() until the uartclk and divisors have
been updated anyway.

Drop the unnecessary swaps and incorrect comment and simply update the
uartclk field if the clock-rate change was successful.

Signed-off-by: Johan Hovold <johan@...nel.org>
---
 drivers/tty/serial/8250/8250_dw.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index a3a0154da567..52e03dd0eb3b 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -338,15 +338,12 @@ static void dw8250_set_termios(struct uart_port *p, struct ktermios *termios,
 	rate = clk_round_rate(d->clk, newrate);
 	if (rate > 0) {
 		/*
-		 * Premilinary set the uartclk to the new clock rate so the
-		 * clock update event handler caused by the clk_set_rate()
-		 * calling wouldn't actually update the UART divisor since
-		 * we about to do this anyway.
+		 * Note that any clock-notifer worker will block in
+		 * serial8250_update_uartclk() until we are done.
 		 */
-		swap(p->uartclk, rate);
 		ret = clk_set_rate(d->clk, newrate);
-		if (ret)
-			swap(p->uartclk, rate);
+		if (!ret)
+			p->uartclk = rate;
 	}
 	clk_prepare_enable(d->clk);
 
-- 
2.32.0