Linux Serial subsystem development
 help / color / mirror / Atom feed
* [PATCH] serial: xilinx_uartps: publish console_port with release semantics
@ 2026-09-22  1:21 Jaidev Shastri via B4 Relay
  2026-09-22  1:31 ` sashiko-bot
  2026-09-23 12:08 ` Greg Kroah-Hartman
  0 siblings, 2 replies; 3+ messages in thread
From: Jaidev Shastri via B4 Relay @ 2026-09-22  1:21 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jiri Slaby, Michal Simek
  Cc: linux-kernel, linux-serial, linux-arm-kernel, Jaidev Shastri

From: Jaidev Shastri <jaidevshastri@vt.edu>

cdns_uart_probe() assigns the first probed port to console_port with a
plain store once the port is set up. cdns_uart_console_write() and
cdns_uart_console_setup() read it with plain loads.

Store it with smp_store_release() and read it with smp_load_acquire().

Found with MBCheck, a static herd7-based memory consistency checker.

Signed-off-by: Jaidev Shastri <jaidevshastri@vt.edu>
---
 drivers/tty/serial/xilinx_uartps.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index a072b75db..59e0d388d 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -1371,7 +1371,8 @@ static struct uart_port *console_port;
 static void cdns_uart_console_write(struct console *co, const char *s,
 				unsigned int count)
 {
-	struct uart_port *port = console_port;
+	/* Pairs with the smp_store_release() in cdns_uart_probe(). */
+	struct uart_port *port = smp_load_acquire(&console_port);
 	unsigned long flags;
 	unsigned int imr, ctrl;
 	int locked = 1;
@@ -1414,7 +1415,8 @@ static void cdns_uart_console_write(struct console *co, const char *s,
  */
 static int cdns_uart_console_setup(struct console *co, char *options)
 {
-	struct uart_port *port = console_port;
+	/* Pairs with the smp_store_release() in cdns_uart_probe(). */
+	struct uart_port *port = smp_load_acquire(&console_port);
 
 	int baud = 9600;
 	int bits = 8;
@@ -1812,7 +1814,8 @@ static int cdns_uart_probe(struct platform_device *pdev)
 	 */
 	if (!console_port) {
 		cdns_uart_console.index = id;
-		console_port = port;
+		/* Pairs with the smp_load_acquire() in the console callbacks. */
+		smp_store_release(&console_port, port);
 	}
 #endif
 	if (cdns_uart_data->port->rs485.flags & SER_RS485_ENABLED)

---
base-commit: 93f51579e7df248780214094418f205253383cc5
change-id: 20260921-mb-uartps-ce1b1ca29a83

Best regards,
--  
Jaidev Shastri <jaidevshastri@vt.edu>



^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] serial: xilinx_uartps: publish console_port with release semantics
  2026-09-22  1:21 [PATCH] serial: xilinx_uartps: publish console_port with release semantics Jaidev Shastri via B4 Relay
@ 2026-09-22  1:31 ` sashiko-bot
  2026-09-23 12:08 ` Greg Kroah-Hartman
  1 sibling, 0 replies; 3+ messages in thread
From: sashiko-bot @ 2026-09-22  1:31 UTC (permalink / raw)
  To: Jaidev Shastri; +Cc: linux-serial

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] serial: xilinx_uartps: mixed plain and atomic accesses to console_port
--

commit 6429f77a5af6cf9c0baafae636fbc28eb10cbf3c
Author: Jaidev Shastri <jaidevshastri@vt.edu>
serial: xilinx_uartps: publish console_port with release semantics

cdns_uart_probe() assigns the first probed port to console_port with a plain
store once the port is set up, while cdns_uart_console_write() and
cdns_uart_console_setup() read it with plain loads. This commit stores it
with smp_store_release() and reads it with smp_load_acquire() to address
memory consistency issues.

> diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
> index a072b75dbaf23..59e0d388d7c38 100644
> --- a/drivers/tty/serial/xilinx_uartps.c
> +++ b/drivers/tty/serial/xilinx_uartps.c

[ ... ]

> @@ -1812,7 +1814,8 @@ static int cdns_uart_probe(struct platform_device *pdev)
>  	 */
>  	if (!console_port) {
        ^^^^

[Severity: Medium]
Does this plain read of console_port race with concurrent probes, violating the
patch's stated goal of using acquire/release semantics?

>  		cdns_uart_console.index = id;
> -		console_port = port;
> +		/* Pairs with the smp_load_acquire() in the console callbacks. */
> +		smp_store_release(&console_port, port);
>  	}

Are there other plain accesses to console_port that were left unfixed and could
cause regressions by breaking the strict release/acquire semantics intended by
this patch?

For example, further down in cdns_uart_probe():

	if (console_port == port &&
	    !console_is_registered(cdns_uart_uart_driver.cons)) {
		console_port = NULL;
		cdns_uart_console.index = -1;
	}

And in cdns_uart_remove():

	if (console_port == port)
		console_port = NULL;

Could these plain stores and reads race with smp_load_acquire() in the console
callbacks, violating the Linux Kernel Memory Model?

Mixing atomic barrier operations with plain C accesses can lead to data races
or compiler optimizations breaking strict memory ordering. This might happen
during concurrent asynchronous device probes or removals interacting with
console callbacks reading the global state.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260921-mb-uartps-v1-1-9fc2f3d4b1a5@vt.edu?part=1

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] serial: xilinx_uartps: publish console_port with release semantics
  2026-09-22  1:21 [PATCH] serial: xilinx_uartps: publish console_port with release semantics Jaidev Shastri via B4 Relay
  2026-09-22  1:31 ` sashiko-bot
@ 2026-09-23 12:08 ` Greg Kroah-Hartman
  1 sibling, 0 replies; 3+ messages in thread
From: Greg Kroah-Hartman @ 2026-09-23 12:08 UTC (permalink / raw)
  To: jaidevshastri
  Cc: Jiri Slaby, Michal Simek, linux-kernel, linux-serial,
	linux-arm-kernel

On Mon, Sep 21, 2026 at 09:21:13PM -0400, Jaidev Shastri via B4 Relay wrote:
> From: Jaidev Shastri <jaidevshastri@vt.edu>
> 
> cdns_uart_probe() assigns the first probed port to console_port with a
> plain store once the port is set up. cdns_uart_console_write() and
> cdns_uart_console_setup() read it with plain loads.
> 
> Store it with smp_store_release() and read it with smp_load_acquire().
> 
> Found with MBCheck, a static herd7-based memory consistency checker.

And how was this tested?  The shashiko review bot shows where your tool
looks wrong...


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-09-23 12:08 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-22  1:21 [PATCH] serial: xilinx_uartps: publish console_port with release semantics Jaidev Shastri via B4 Relay
2026-09-22  1:31 ` sashiko-bot
2026-09-23 12:08 ` 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