Linux kernel -stable discussions
 help / color / mirror / Atom feed
* [PATCH 2/3] serial: 8250: dispatch SysRq character in serial8250_handle_irq()
       [not found] ` <cover.1778592805.git.jnilo@free.fr>
@ 2026-05-12 13:46   ` Jacques Nilo
  2026-05-13 11:49     ` Ilpo Järvinen
  2026-05-12 13:46   ` [PATCH 3/3] serial: 8250_dw: dispatch SysRq character in dw8250_handle_irq() Jacques Nilo
  2026-05-13 13:30   ` [PATCH v2 0/3] serial: 8250: fix BREAK+SysRq dispatch on guard()-locked IRQ handlers Jacques Nilo
  2 siblings, 1 reply; 10+ messages in thread
From: Jacques Nilo @ 2026-05-12 13:46 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jiri Slaby
  Cc: linux-serial, linux-kernel, Ilpo Järvinen, Johan Hovold,
	Jacques Nilo, stable

serial8250_handle_irq() captures a SysRq character into port->sysrq_ch
inside serial8250_handle_irq_locked() via uart_prepare_sysrq_char()
(reached from serial8250_read_char()). Dispatch of that captured
character to handle_sysrq() is expected to happen at port-unlock time,
through uart_unlock_and_check_sysrq[_irqrestore]().

After commit 8324a54f604d ("serial: 8250: Add
serial8250_handle_irq_locked()") the function was reduced to a wrapper
that takes the port lock via guard(uart_port_lock_irqsave) whose
destructor is plain uart_port_unlock_irqrestore(). The sysrq-aware
unlock helper is no longer called, so port->sysrq_ch is captured but
never dispatched: BREAK + SysRq key is consumed silently.

This was the very condition Johan Hovold's 853a9ae29e978 ("serial:
8250: fix handle_irq locking", 2021) introduced
uart_unlock_and_check_sysrq_irqrestore() to address.

Switch to the new guard(uart_port_lock_sysrq_irqsave), whose destructor
is the sysrq-aware unlock helper, restoring the pre-split behaviour.
Update the Context: comment on serial8250_handle_irq_locked() so future
HW-specific 8250 wrappers know to use the same guard or the explicit
sysrq-aware unlock.

Verified on RTL8196E with CONFIG_MAGIC_SYSRQ_SERIAL=y: BREAK + 'h' on
the console UART produces the SysRq help dump in dmesg and the brk
counter in /proc/tty/driver/serial increments correctly.

Fixes: 8324a54f604d ("serial: 8250: Add serial8250_handle_irq_locked()")
Cc: stable@vger.kernel.org
Signed-off-by: Jacques Nilo <jnilo@free.fr>
---
 drivers/tty/serial/8250/8250_port.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index e4e6a53eb..64f3487e8 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -1786,7 +1786,10 @@ static bool handle_rx_dma(struct uart_8250_port *up, unsigned int iir)
 }
 
 /*
- * Context: port's lock must be held by the caller.
+ * Context: port's lock must be held by the caller. The caller must
+ * release it via guard(uart_port_lock_sysrq_irqsave) or
+ * uart_unlock_and_check_sysrq_irqrestore(), which captures SysRq
+ * character on unlock.
  */
 void serial8250_handle_irq_locked(struct uart_port *port, unsigned int iir)
 {
@@ -1839,7 +1842,7 @@ int serial8250_handle_irq(struct uart_port *port, unsigned int iir)
 	if (iir & UART_IIR_NO_INT)
 		return 0;
 
-	guard(uart_port_lock_irqsave)(port);
+	guard(uart_port_lock_sysrq_irqsave)(port);
 	serial8250_handle_irq_locked(port, iir);
 
 	return 1;
-- 
2.43.0


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

* [PATCH 3/3] serial: 8250_dw: dispatch SysRq character in dw8250_handle_irq()
       [not found] ` <cover.1778592805.git.jnilo@free.fr>
  2026-05-12 13:46   ` [PATCH 2/3] serial: 8250: dispatch SysRq character in serial8250_handle_irq() Jacques Nilo
@ 2026-05-12 13:46   ` Jacques Nilo
  2026-05-13 11:50     ` Ilpo Järvinen
  2026-05-13 13:30   ` [PATCH v2 0/3] serial: 8250: fix BREAK+SysRq dispatch on guard()-locked IRQ handlers Jacques Nilo
  2 siblings, 1 reply; 10+ messages in thread
From: Jacques Nilo @ 2026-05-12 13:46 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jiri Slaby
  Cc: linux-serial, linux-kernel, Ilpo Järvinen, Johan Hovold,
	Jacques Nilo, stable

dw8250_handle_irq() calls serial8250_handle_irq_locked() with the port
lock held via guard(uart_port_lock_irqsave). The guard destructor is
plain uart_port_unlock_irqrestore(), so a SysRq character captured into
port->sysrq_ch by uart_prepare_sysrq_char() is dropped without ever
being dispatched to handle_sysrq().

This is the same regression pattern as in serial8250_handle_irq(),
introduced when 883c5a2bc934 ("serial: 8250_dw: Rework
dw8250_handle_irq() locking and IIR handling") moved the function to
the guard()-based locking scheme without using the sysrq-aware unlock
helper.

Switch to guard(uart_port_lock_sysrq_irqsave) so that captured
sysrq_ch is dispatched on scope exit, matching the fix in
serial8250_handle_irq().

Fixes: 883c5a2bc934 ("serial: 8250_dw: Rework dw8250_handle_irq() locking and IIR handling")
Cc: stable@vger.kernel.org
Signed-off-by: Jacques Nilo <jnilo@free.fr>
---
 drivers/tty/serial/8250/8250_dw.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index 55e40c10f..237543fa7 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -416,7 +416,7 @@ static int dw8250_handle_irq(struct uart_port *p)
 	unsigned int quirks = d->pdata->quirks;
 	unsigned int status;
 
-	guard(uart_port_lock_irqsave)(p);
+	guard(uart_port_lock_sysrq_irqsave)(p);
 
 	switch (FIELD_GET(DW_UART_IIR_IID, iir)) {
 	case UART_IIR_NO_INT:
-- 
2.43.0


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

* Re: [PATCH 2/3] serial: 8250: dispatch SysRq character in serial8250_handle_irq()
  2026-05-12 13:46   ` [PATCH 2/3] serial: 8250: dispatch SysRq character in serial8250_handle_irq() Jacques Nilo
@ 2026-05-13 11:49     ` Ilpo Järvinen
  0 siblings, 0 replies; 10+ messages in thread
From: Ilpo Järvinen @ 2026-05-13 11:49 UTC (permalink / raw)
  To: Jacques Nilo
  Cc: Greg Kroah-Hartman, Jiri Slaby, linux-serial, LKML, Johan Hovold,
	stable

[-- Attachment #1: Type: text/plain, Size: 2858 bytes --]

On Tue, 12 May 2026, Jacques Nilo wrote:

> serial8250_handle_irq() captures a SysRq character into port->sysrq_ch
> inside serial8250_handle_irq_locked() via uart_prepare_sysrq_char()
> (reached from serial8250_read_char()). Dispatch of that captured
> character to handle_sysrq() is expected to happen at port-unlock time,
> through uart_unlock_and_check_sysrq[_irqrestore]().
> 
> After commit 8324a54f604d ("serial: 8250: Add
> serial8250_handle_irq_locked()") the function was reduced to a wrapper
> that takes the port lock via guard(uart_port_lock_irqsave) whose
> destructor is plain uart_port_unlock_irqrestore(). The sysrq-aware
> unlock helper is no longer called, so port->sysrq_ch is captured but
> never dispatched: BREAK + SysRq key is consumed silently.
> 
> This was the very condition Johan Hovold's 853a9ae29e978 ("serial:
> 8250: fix handle_irq locking", 2021) introduced
> uart_unlock_and_check_sysrq_irqrestore() to address.
> 
> Switch to the new guard(uart_port_lock_sysrq_irqsave), whose destructor
> is the sysrq-aware unlock helper, restoring the pre-split behaviour.
> Update the Context: comment on serial8250_handle_irq_locked() so future
> HW-specific 8250 wrappers know to use the same guard or the explicit
> sysrq-aware unlock.
> 
> Verified on RTL8196E with CONFIG_MAGIC_SYSRQ_SERIAL=y: BREAK + 'h' on
> the console UART produces the SysRq help dump in dmesg and the brk
> counter in /proc/tty/driver/serial increments correctly.
> 
> Fixes: 8324a54f604d ("serial: 8250: Add serial8250_handle_irq_locked()")
> Cc: stable@vger.kernel.org
> Signed-off-by: Jacques Nilo <jnilo@free.fr>
> ---
>  drivers/tty/serial/8250/8250_port.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
> index e4e6a53eb..64f3487e8 100644
> --- a/drivers/tty/serial/8250/8250_port.c
> +++ b/drivers/tty/serial/8250/8250_port.c
> @@ -1786,7 +1786,10 @@ static bool handle_rx_dma(struct uart_8250_port *up, unsigned int iir)
>  }
>  
>  /*
> - * Context: port's lock must be held by the caller.
> + * Context: port's lock must be held by the caller. The caller must
> + * release it via guard(uart_port_lock_sysrq_irqsave) or
> + * uart_unlock_and_check_sysrq_irqrestore(), which captures SysRq
> + * character on unlock.
>   */
>  void serial8250_handle_irq_locked(struct uart_port *port, unsigned int iir)
>  {
> @@ -1839,7 +1842,7 @@ int serial8250_handle_irq(struct uart_port *port, unsigned int iir)
>  	if (iir & UART_IIR_NO_INT)
>  		return 0;
>  
> -	guard(uart_port_lock_irqsave)(port);
> +	guard(uart_port_lock_sysrq_irqsave)(port);
>  	serial8250_handle_irq_locked(port, iir);
>  
>  	return 1;
> 

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

-- 
 i.

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

* Re: [PATCH 3/3] serial: 8250_dw: dispatch SysRq character in dw8250_handle_irq()
  2026-05-12 13:46   ` [PATCH 3/3] serial: 8250_dw: dispatch SysRq character in dw8250_handle_irq() Jacques Nilo
@ 2026-05-13 11:50     ` Ilpo Järvinen
  0 siblings, 0 replies; 10+ messages in thread
From: Ilpo Järvinen @ 2026-05-13 11:50 UTC (permalink / raw)
  To: Jacques Nilo
  Cc: Greg Kroah-Hartman, Jiri Slaby, linux-serial, LKML, Johan Hovold,
	stable

[-- Attachment #1: Type: text/plain, Size: 1705 bytes --]

On Tue, 12 May 2026, Jacques Nilo wrote:

> dw8250_handle_irq() calls serial8250_handle_irq_locked() with the port
> lock held via guard(uart_port_lock_irqsave). The guard destructor is
> plain uart_port_unlock_irqrestore(), so a SysRq character captured into
> port->sysrq_ch by uart_prepare_sysrq_char() is dropped without ever
> being dispatched to handle_sysrq().
> 
> This is the same regression pattern as in serial8250_handle_irq(),
> introduced when 883c5a2bc934 ("serial: 8250_dw: Rework
> dw8250_handle_irq() locking and IIR handling") moved the function to
> the guard()-based locking scheme without using the sysrq-aware unlock
> helper.
> 
> Switch to guard(uart_port_lock_sysrq_irqsave) so that captured
> sysrq_ch is dispatched on scope exit, matching the fix in
> serial8250_handle_irq().
> 
> Fixes: 883c5a2bc934 ("serial: 8250_dw: Rework dw8250_handle_irq() locking and IIR handling")
> Cc: stable@vger.kernel.org
> Signed-off-by: Jacques Nilo <jnilo@free.fr>
> ---
>  drivers/tty/serial/8250/8250_dw.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
> index 55e40c10f..237543fa7 100644
> --- a/drivers/tty/serial/8250/8250_dw.c
> +++ b/drivers/tty/serial/8250/8250_dw.c
> @@ -416,7 +416,7 @@ static int dw8250_handle_irq(struct uart_port *p)
>  	unsigned int quirks = d->pdata->quirks;
>  	unsigned int status;
>  
> -	guard(uart_port_lock_irqsave)(p);
> +	guard(uart_port_lock_sysrq_irqsave)(p);
>  
>  	switch (FIELD_GET(DW_UART_IIR_IID, iir)) {
>  	case UART_IIR_NO_INT:
> 

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

-- 
 i.

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

* [PATCH v2 0/3] serial: 8250: fix BREAK+SysRq dispatch on guard()-locked IRQ handlers
       [not found] ` <cover.1778592805.git.jnilo@free.fr>
  2026-05-12 13:46   ` [PATCH 2/3] serial: 8250: dispatch SysRq character in serial8250_handle_irq() Jacques Nilo
  2026-05-12 13:46   ` [PATCH 3/3] serial: 8250_dw: dispatch SysRq character in dw8250_handle_irq() Jacques Nilo
@ 2026-05-13 13:30   ` Jacques Nilo
  2026-05-13 13:30     ` [PATCH v2 1/3] serial: core: introduce guard(uart_port_lock_check_sysrq_irqsave) Jacques Nilo
                       ` (3 more replies)
  2 siblings, 4 replies; 10+ messages in thread
From: Jacques Nilo @ 2026-05-13 13:30 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jiri Slaby
  Cc: Ilpo Järvinen, Andy Shevchenko, linux-serial, linux-kernel,
	stable, Jacques Nilo

This series fixes a silent regression where a SysRq character entered as
BREAK + key on the serial console is consumed by the kernel but never
dispatched to handle_sysrq(). Same description as v1 [1].

v1 -> v2 (per Ilpo's review [2]):

 - Renamed the new lock guard from uart_port_lock_sysrq_irqsave to
   uart_port_lock_check_sysrq_irqsave, preserving the "check" semantics
   of the destructor's underlying helper
   uart_unlock_and_check_sysrq_irqrestore(). Mechanical rename across
   patches 2/3 and 3/3; Ilpo's Reviewed-by trailers from v1 carried
   forward.

 - Patch 1/3 commit message reflowed: the "guard(...)" form is spelled
   out, the "lock side is identical" sentence moved up next to the
   variant introduction, the now-redundant naming-rationale sentence
   removed, and "opt in by using" tightened to "must use".

 - Added Cc: stable@vger.kernel.org to patch 1/3 (prerequisite for the
   stable backport of 2/3 and 3/3); no Fixes: tag, since 1/3 adds new
   API rather than fixing existing code.

 - Collapsed the DEFINE_LOCK_GUARD_1 destructor expression to a single
   line, which fits within the expected indentation.

No re-test of the BREAK + 'h' path was performed for v2 since the
diff against v1 is purely a textual rename plus the commit-message
reflow above; the v1 RTL8196E validation (BREAK + 'h' on the console
UART producing the SysRq help dump, brk counter incrementing in
/proc/tty/driver/serial) continues to apply unchanged. Built and
booted on tty-next (base 16e95bfb79b5).

[1] https://lore.kernel.org/linux-serial/cover.1778592805.git.jnilo@free.fr/
[2] https://lore.kernel.org/linux-serial/3439217b-90b5-5d21-e777-d238b3ffc1a0@linux.intel.com/

Jacques Nilo (3):
  serial: core: introduce guard(uart_port_lock_check_sysrq_irqsave)
  serial: 8250: dispatch SysRq character in serial8250_handle_irq()
  serial: 8250_dw: dispatch SysRq character in dw8250_handle_irq()

 drivers/tty/serial/8250/8250_dw.c   |  2 +-
 drivers/tty/serial/8250/8250_port.c |  7 +++++--
 include/linux/serial_core.h         | 12 ++++++++++++
 3 files changed, 18 insertions(+), 3 deletions(-)


base-commit: 16e95bfb79b5d9d01dc7651d98caf3c2ace331cd
-- 
2.43.0


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

* [PATCH v2 1/3] serial: core: introduce guard(uart_port_lock_check_sysrq_irqsave)
  2026-05-13 13:30   ` [PATCH v2 0/3] serial: 8250: fix BREAK+SysRq dispatch on guard()-locked IRQ handlers Jacques Nilo
@ 2026-05-13 13:30     ` Jacques Nilo
  2026-05-13 13:35       ` Ilpo Järvinen
  2026-05-13 13:30     ` [PATCH v2 2/3] serial: 8250: dispatch SysRq character in serial8250_handle_irq() Jacques Nilo
                       ` (2 subsequent siblings)
  3 siblings, 1 reply; 10+ messages in thread
From: Jacques Nilo @ 2026-05-13 13:30 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jiri Slaby
  Cc: Ilpo Järvinen, Andy Shevchenko, linux-serial, linux-kernel,
	stable, Jacques Nilo

uart_handle_break() and uart_prepare_sysrq_char() (in
include/linux/serial_core.h) capture a SysRq character into
port->sysrq_ch while the port lock is held and rely on the unlock
helper -- uart_unlock_and_check_sysrq_irqrestore() -- to dispatch the
captured character to handle_sysrq() on scope exit.

The existing guard(uart_port_lock_irqsave) cannot be used by IRQ
handlers that process RX, because its destructor calls plain
uart_port_unlock_irqrestore() and silently drops port->sysrq_ch.

Add a dedicated guard(uart_port_lock_check_sysrq_irqsave) variant
whose destructor is the sysrq-aware unlock helper. The lock side is
identical to uart_port_lock_irqsave -- only the unlock-time behaviour
differs. Callers that may capture SysRq characters must use
guard(uart_port_lock_check_sysrq_irqsave); the existing
guard(uart_port_lock_irqsave) keeps its current plain-unlock semantics
for the many callers that do not process RX.

The new macro is placed after the CONFIG_MAGIC_SYSRQ_SERIAL block so
both definitions of uart_unlock_and_check_sysrq_irqrestore() (sysrq
enabled and disabled) are visible at expansion time. When
CONFIG_MAGIC_SYSRQ_SERIAL=n the destructor degenerates to plain
uart_port_unlock_irqrestore(), so there is no overhead.

No functional change on its own; users are converted in the following
patches.

Cc: stable@vger.kernel.org
Signed-off-by: Jacques Nilo <jnilo@free.fr>
---
 include/linux/serial_core.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 4f7bbdd90..d1404c97d 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -1286,6 +1286,18 @@ static inline void uart_unlock_and_check_sysrq_irqrestore(struct uart_port *port
 }
 #endif	/* CONFIG_MAGIC_SYSRQ_SERIAL */
 
+/*
+ * Variant of guard(uart_port_lock_irqsave) for IRQ handlers that may capture
+ * a SysRq character via uart_prepare_sysrq_char(). The destructor uses the
+ * sysrq-aware unlock helper so that a captured port->sysrq_ch is dispatched
+ * to handle_sysrq() on scope exit. The plain guard variant silently drops
+ * sysrq_ch and must not be used by callers that process RX.
+ */
+DEFINE_LOCK_GUARD_1(uart_port_lock_check_sysrq_irqsave, struct uart_port,
+                    uart_port_lock_irqsave(_T->lock, &_T->flags),
+                    uart_unlock_and_check_sysrq_irqrestore(_T->lock, _T->flags),
+                    unsigned long flags);
+
 /*
  * We do the SysRQ and SAK checking like this...
  */
-- 
2.43.0


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

* [PATCH v2 2/3] serial: 8250: dispatch SysRq character in serial8250_handle_irq()
  2026-05-13 13:30   ` [PATCH v2 0/3] serial: 8250: fix BREAK+SysRq dispatch on guard()-locked IRQ handlers Jacques Nilo
  2026-05-13 13:30     ` [PATCH v2 1/3] serial: core: introduce guard(uart_port_lock_check_sysrq_irqsave) Jacques Nilo
@ 2026-05-13 13:30     ` Jacques Nilo
  2026-05-13 13:30     ` [PATCH v2 3/3] serial: 8250_dw: dispatch SysRq character in dw8250_handle_irq() Jacques Nilo
  2026-05-13 17:51     ` [PATCH v2 0/3] serial: 8250: fix BREAK+SysRq dispatch on guard()-locked IRQ handlers Andy Shevchenko
  3 siblings, 0 replies; 10+ messages in thread
From: Jacques Nilo @ 2026-05-13 13:30 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jiri Slaby
  Cc: Ilpo Järvinen, Andy Shevchenko, linux-serial, linux-kernel,
	stable, Jacques Nilo

serial8250_handle_irq() captures a SysRq character into port->sysrq_ch
inside serial8250_handle_irq_locked() via uart_prepare_sysrq_char()
(reached from serial8250_read_char()). Dispatch of that captured
character to handle_sysrq() is expected to happen at port-unlock time,
through uart_unlock_and_check_sysrq[_irqrestore]().

After commit 8324a54f604d ("serial: 8250: Add
serial8250_handle_irq_locked()") the function was reduced to a wrapper
that takes the port lock via guard(uart_port_lock_irqsave) whose
destructor is plain uart_port_unlock_irqrestore(). The sysrq-aware
unlock helper is no longer called, so port->sysrq_ch is captured but
never dispatched: BREAK + SysRq key is consumed silently.

This was the very condition Johan Hovold's 853a9ae29e978 ("serial:
8250: fix handle_irq locking", 2021) introduced
uart_unlock_and_check_sysrq_irqrestore() to address.

Switch to the new guard(uart_port_lock_check_sysrq_irqsave), whose
destructor is the sysrq-aware unlock helper, restoring the pre-split
behaviour. Update the Context: comment on serial8250_handle_irq_locked()
so future HW-specific 8250 wrappers know to use the same guard or the
explicit sysrq-aware unlock.

Verified on RTL8196E with CONFIG_MAGIC_SYSRQ_SERIAL=y: BREAK + 'h' on
the console UART produces the SysRq help dump in dmesg and the brk
counter in /proc/tty/driver/serial increments correctly.

Fixes: 8324a54f604d ("serial: 8250: Add serial8250_handle_irq_locked()")
Cc: stable@vger.kernel.org
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Signed-off-by: Jacques Nilo <jnilo@free.fr>
---
 drivers/tty/serial/8250/8250_port.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index e4e6a53eb..59203bbfb 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -1786,7 +1786,10 @@ static bool handle_rx_dma(struct uart_8250_port *up, unsigned int iir)
 }
 
 /*
- * Context: port's lock must be held by the caller.
+ * Context: port's lock must be held by the caller. The caller must
+ * release it via guard(uart_port_lock_check_sysrq_irqsave) or
+ * uart_unlock_and_check_sysrq_irqrestore(), which captures SysRq
+ * character on unlock.
  */
 void serial8250_handle_irq_locked(struct uart_port *port, unsigned int iir)
 {
@@ -1839,7 +1842,7 @@ int serial8250_handle_irq(struct uart_port *port, unsigned int iir)
 	if (iir & UART_IIR_NO_INT)
 		return 0;
 
-	guard(uart_port_lock_irqsave)(port);
+	guard(uart_port_lock_check_sysrq_irqsave)(port);
 	serial8250_handle_irq_locked(port, iir);
 
 	return 1;
-- 
2.43.0


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

* [PATCH v2 3/3] serial: 8250_dw: dispatch SysRq character in dw8250_handle_irq()
  2026-05-13 13:30   ` [PATCH v2 0/3] serial: 8250: fix BREAK+SysRq dispatch on guard()-locked IRQ handlers Jacques Nilo
  2026-05-13 13:30     ` [PATCH v2 1/3] serial: core: introduce guard(uart_port_lock_check_sysrq_irqsave) Jacques Nilo
  2026-05-13 13:30     ` [PATCH v2 2/3] serial: 8250: dispatch SysRq character in serial8250_handle_irq() Jacques Nilo
@ 2026-05-13 13:30     ` Jacques Nilo
  2026-05-13 17:51     ` [PATCH v2 0/3] serial: 8250: fix BREAK+SysRq dispatch on guard()-locked IRQ handlers Andy Shevchenko
  3 siblings, 0 replies; 10+ messages in thread
From: Jacques Nilo @ 2026-05-13 13:30 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jiri Slaby
  Cc: Ilpo Järvinen, Andy Shevchenko, linux-serial, linux-kernel,
	stable, Jacques Nilo

dw8250_handle_irq() calls serial8250_handle_irq_locked() with the port
lock held via guard(uart_port_lock_irqsave). The guard destructor is
plain uart_port_unlock_irqrestore(), so a SysRq character captured into
port->sysrq_ch by uart_prepare_sysrq_char() is dropped without ever
being dispatched to handle_sysrq().

This is the same regression pattern as in serial8250_handle_irq(),
introduced when 883c5a2bc934 ("serial: 8250_dw: Rework
dw8250_handle_irq() locking and IIR handling") moved the function to
the guard()-based locking scheme without using the sysrq-aware unlock
helper.

Switch to guard(uart_port_lock_check_sysrq_irqsave) so that captured
sysrq_ch is dispatched on scope exit, matching the fix in
serial8250_handle_irq().

Fixes: 883c5a2bc934 ("serial: 8250_dw: Rework dw8250_handle_irq() locking and IIR handling")
Cc: stable@vger.kernel.org
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Signed-off-by: Jacques Nilo <jnilo@free.fr>
---
 drivers/tty/serial/8250/8250_dw.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index 55e40c10f..9d552b224 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -416,7 +416,7 @@ static int dw8250_handle_irq(struct uart_port *p)
 	unsigned int quirks = d->pdata->quirks;
 	unsigned int status;
 
-	guard(uart_port_lock_irqsave)(p);
+	guard(uart_port_lock_check_sysrq_irqsave)(p);
 
 	switch (FIELD_GET(DW_UART_IIR_IID, iir)) {
 	case UART_IIR_NO_INT:
-- 
2.43.0


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

* Re: [PATCH v2 1/3] serial: core: introduce guard(uart_port_lock_check_sysrq_irqsave)
  2026-05-13 13:30     ` [PATCH v2 1/3] serial: core: introduce guard(uart_port_lock_check_sysrq_irqsave) Jacques Nilo
@ 2026-05-13 13:35       ` Ilpo Järvinen
  0 siblings, 0 replies; 10+ messages in thread
From: Ilpo Järvinen @ 2026-05-13 13:35 UTC (permalink / raw)
  To: Jacques Nilo
  Cc: Greg Kroah-Hartman, Jiri Slaby, Andy Shevchenko, linux-serial,
	LKML, stable

[-- Attachment #1: Type: text/plain, Size: 2806 bytes --]

On Wed, 13 May 2026, Jacques Nilo wrote:

> uart_handle_break() and uart_prepare_sysrq_char() (in
> include/linux/serial_core.h) capture a SysRq character into
> port->sysrq_ch while the port lock is held and rely on the unlock
> helper -- uart_unlock_and_check_sysrq_irqrestore() -- to dispatch the
> captured character to handle_sysrq() on scope exit.
> 
> The existing guard(uart_port_lock_irqsave) cannot be used by IRQ
> handlers that process RX, because its destructor calls plain
> uart_port_unlock_irqrestore() and silently drops port->sysrq_ch.
> 
> Add a dedicated guard(uart_port_lock_check_sysrq_irqsave) variant
> whose destructor is the sysrq-aware unlock helper. The lock side is
> identical to uart_port_lock_irqsave -- only the unlock-time behaviour
> differs. Callers that may capture SysRq characters must use
> guard(uart_port_lock_check_sysrq_irqsave); the existing
> guard(uart_port_lock_irqsave) keeps its current plain-unlock semantics
> for the many callers that do not process RX.
> 
> The new macro is placed after the CONFIG_MAGIC_SYSRQ_SERIAL block so
> both definitions of uart_unlock_and_check_sysrq_irqrestore() (sysrq
> enabled and disabled) are visible at expansion time. When
> CONFIG_MAGIC_SYSRQ_SERIAL=n the destructor degenerates to plain
> uart_port_unlock_irqrestore(), so there is no overhead.
> 
> No functional change on its own; users are converted in the following
> patches.
> 
> Cc: stable@vger.kernel.org
> Signed-off-by: Jacques Nilo <jnilo@free.fr>
> ---
>  include/linux/serial_core.h | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
> index 4f7bbdd90..d1404c97d 100644
> --- a/include/linux/serial_core.h
> +++ b/include/linux/serial_core.h
> @@ -1286,6 +1286,18 @@ static inline void uart_unlock_and_check_sysrq_irqrestore(struct uart_port *port
>  }
>  #endif	/* CONFIG_MAGIC_SYSRQ_SERIAL */
>  
> +/*
> + * Variant of guard(uart_port_lock_irqsave) for IRQ handlers that may capture
> + * a SysRq character via uart_prepare_sysrq_char(). The destructor uses the
> + * sysrq-aware unlock helper so that a captured port->sysrq_ch is dispatched
> + * to handle_sysrq() on scope exit. The plain guard variant silently drops
> + * sysrq_ch and must not be used by callers that process RX.
> + */
> +DEFINE_LOCK_GUARD_1(uart_port_lock_check_sysrq_irqsave, struct uart_port,
> +                    uart_port_lock_irqsave(_T->lock, &_T->flags),
> +                    uart_unlock_and_check_sysrq_irqrestore(_T->lock, _T->flags),
> +                    unsigned long flags);
> +
>  /*
>   * We do the SysRQ and SAK checking like this...
>   */
> 

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

-- 
 i.

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

* Re: [PATCH v2 0/3] serial: 8250: fix BREAK+SysRq dispatch on guard()-locked IRQ handlers
  2026-05-13 13:30   ` [PATCH v2 0/3] serial: 8250: fix BREAK+SysRq dispatch on guard()-locked IRQ handlers Jacques Nilo
                       ` (2 preceding siblings ...)
  2026-05-13 13:30     ` [PATCH v2 3/3] serial: 8250_dw: dispatch SysRq character in dw8250_handle_irq() Jacques Nilo
@ 2026-05-13 17:51     ` Andy Shevchenko
  3 siblings, 0 replies; 10+ messages in thread
From: Andy Shevchenko @ 2026-05-13 17:51 UTC (permalink / raw)
  To: Jacques Nilo
  Cc: Greg Kroah-Hartman, Jiri Slaby, Ilpo Järvinen, linux-serial,
	linux-kernel, stable

On Wed, May 13, 2026 at 03:30:22PM +0200, Jacques Nilo wrote:
> This series fixes a silent regression where a SysRq character entered as
> BREAK + key on the serial console is consumed by the kernel but never
> dispatched to handle_sysrq(). Same description as v1 [1].

I have read the v1 discussion and v2 makes sense to me and looks good
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

-- 
With Best Regards,
Andy Shevchenko



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

end of thread, other threads:[~2026-05-13 17:51 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <5efe9e03-4d86-43a0-9ec2-e610ff31095d@free.fr>
     [not found] ` <cover.1778592805.git.jnilo@free.fr>
2026-05-12 13:46   ` [PATCH 2/3] serial: 8250: dispatch SysRq character in serial8250_handle_irq() Jacques Nilo
2026-05-13 11:49     ` Ilpo Järvinen
2026-05-12 13:46   ` [PATCH 3/3] serial: 8250_dw: dispatch SysRq character in dw8250_handle_irq() Jacques Nilo
2026-05-13 11:50     ` Ilpo Järvinen
2026-05-13 13:30   ` [PATCH v2 0/3] serial: 8250: fix BREAK+SysRq dispatch on guard()-locked IRQ handlers Jacques Nilo
2026-05-13 13:30     ` [PATCH v2 1/3] serial: core: introduce guard(uart_port_lock_check_sysrq_irqsave) Jacques Nilo
2026-05-13 13:35       ` Ilpo Järvinen
2026-05-13 13:30     ` [PATCH v2 2/3] serial: 8250: dispatch SysRq character in serial8250_handle_irq() Jacques Nilo
2026-05-13 13:30     ` [PATCH v2 3/3] serial: 8250_dw: dispatch SysRq character in dw8250_handle_irq() Jacques Nilo
2026-05-13 17:51     ` [PATCH v2 0/3] serial: 8250: fix BREAK+SysRq dispatch on guard()-locked IRQ handlers Andy Shevchenko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox