diff mbox series

[v4,1/2] serial: sifive: lock port in startup()/shutdown() callbacks

Message ID 20250405044338.397237-1-ryotkkr98@gmail.com (mailing list archive)
State Superseded
Headers show
Series serial: sifive: Convert sifive console to nbcon | expand

Checks

Context Check Description
bjorn/pre-ci_am success Success
bjorn/build-rv32-defconfig success build-rv32-defconfig
bjorn/build-rv64-clang-allmodconfig success build-rv64-clang-allmodconfig
bjorn/build-rv64-gcc-allmodconfig success build-rv64-gcc-allmodconfig
bjorn/build-rv64-nommu-k210-defconfig success build-rv64-nommu-k210-defconfig
bjorn/build-rv64-nommu-k210-virt success build-rv64-nommu-k210-virt
bjorn/checkpatch warning checkpatch
bjorn/dtb-warn-rv64 success dtb-warn-rv64
bjorn/header-inline success header-inline
bjorn/kdoc success kdoc
bjorn/module-param success module-param
bjorn/verify-fixes success verify-fixes
bjorn/verify-signedoff success verify-signedoff

Commit Message

Ryo Takakura April 5, 2025, 4:43 a.m. UTC
startup()/shutdown() callbacks access SIFIVE_SERIAL_IE_OFFS.
The register is also accessed from write() callback.

If console were printing and startup()/shutdown() callback
gets called, its access to the register could be overwritten.

Add port->lock to startup()/shutdown() callbacks to make sure
their access to SIFIVE_SERIAL_IE_OFFS is synchronized against
write() callback.

Signed-off-by: Ryo Takakura <ryotkkr98@gmail.com>
Cc: stable@vger.kernel.org
---
 drivers/tty/serial/sifive.c | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Greg Kroah-Hartman April 5, 2025, 7:35 a.m. UTC | #1
On Sat, Apr 05, 2025 at 01:43:38PM +0900, Ryo Takakura wrote:
> startup()/shutdown() callbacks access SIFIVE_SERIAL_IE_OFFS.
> The register is also accessed from write() callback.
> 
> If console were printing and startup()/shutdown() callback
> gets called, its access to the register could be overwritten.
> 
> Add port->lock to startup()/shutdown() callbacks to make sure
> their access to SIFIVE_SERIAL_IE_OFFS is synchronized against
> write() callback.
> 
> Signed-off-by: Ryo Takakura <ryotkkr98@gmail.com>
> Cc: stable@vger.kernel.org

What commit id does this fix?

Why does patch 1/2 need to go to stable, but patch 2/2 does not?  Please
do not mix changes like this in the same series, otherwise we have to
split them up manually when we apply them to the different branches,
right?

thanks,

greg k-h
Ryo Takakura April 5, 2025, 11:23 a.m. UTC | #2
Hi Greg, thanks for the comments!

On Sat, 5 Apr 2025 08:35:44 +0100, Greg KH wrote:
>On Sat, Apr 05, 2025 at 01:43:38PM +0900, Ryo Takakura wrote:
>> startup()/shutdown() callbacks access SIFIVE_SERIAL_IE_OFFS.
>> The register is also accessed from write() callback.
>> 
>> If console were printing and startup()/shutdown() callback
>> gets called, its access to the register could be overwritten.
>> 
>> Add port->lock to startup()/shutdown() callbacks to make sure
>> their access to SIFIVE_SERIAL_IE_OFFS is synchronized against
>> write() callback.
>> 
>> Signed-off-by: Ryo Takakura <ryotkkr98@gmail.com>
>> Cc: stable@vger.kernel.org
>
>What commit id does this fix?

I believe the issue existed ever since the driver was added by commit 
45c054d0815b ("tty: serial: add driver for the SiFive UART").

>Why does patch 1/2 need to go to stable, but patch 2/2 does not?  Please

The patch 2/2 has nothing to do with existing issue and its only the 
patch 1/2 that needs to go to stable as discussed [0].

>do not mix changes like this in the same series, otherwise we have to
>split them up manually when we apply them to the different branches,
>right?

I see, I'll keep this in mind.
Let me resend the two separately with 'Fixes:' tag for the patch 1/2. 

Sincerely,
Ryo Takakura

>thanks,
>
>greg k-h

[0] https://lore.kernel.org/lkml/84sen2fo4b.fsf@jogness.linutronix.de/
diff mbox series

Patch

diff --git a/drivers/tty/serial/sifive.c b/drivers/tty/serial/sifive.c
index 5904a2d4c..054a8e630 100644
--- a/drivers/tty/serial/sifive.c
+++ b/drivers/tty/serial/sifive.c
@@ -563,8 +563,11 @@  static void sifive_serial_break_ctl(struct uart_port *port, int break_state)
 static int sifive_serial_startup(struct uart_port *port)
 {
 	struct sifive_serial_port *ssp = port_to_sifive_serial_port(port);
+	unsigned long flags;
 
+	uart_port_lock_irqsave(&ssp->port, &flags);
 	__ssp_enable_rxwm(ssp);
+	uart_port_unlock_irqrestore(&ssp->port, flags);
 
 	return 0;
 }
@@ -572,9 +575,12 @@  static int sifive_serial_startup(struct uart_port *port)
 static void sifive_serial_shutdown(struct uart_port *port)
 {
 	struct sifive_serial_port *ssp = port_to_sifive_serial_port(port);
+	unsigned long flags;
 
+	uart_port_lock_irqsave(&ssp->port, &flags);
 	__ssp_disable_rxwm(ssp);
 	__ssp_disable_txwm(ssp);
+	uart_port_unlock_irqrestore(&ssp->port, flags);
 }
 
 /**