drivers/tty/serial/8250/8250_dw.c | 2 +- drivers/tty/serial/8250/8250_port.c | 7 +++++-- include/linux/serial_core.h | 13 +++++++++++++ 3 files changed, 19 insertions(+), 3 deletions(-)
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().
The root cause and two fix candidates were discussed in [1]. Following
Ilpo's suggestion, this series adds a dedicated lock-guard variant whose
destructor is the sysrq-aware unlock helper, and switches the two
affected IRQ handlers (serial8250_handle_irq and dw8250_handle_irq) to
use it. The plain guard(uart_port_lock_irqsave) keeps its current
semantics for the many callers that do not process RX.
Patch 1 introduces guard(uart_port_lock_sysrq_irqsave) in serial_core.h.
Patch 2 switches serial8250_handle_irq() and updates the Context comment
on serial8250_handle_irq_locked() so future HW-specific 8250
wrappers know which unlock variant is required.
Patch 3 switches dw8250_handle_irq() to the same guard.
Verified on RTL8196E with CONFIG_MAGIC_SYSRQ_SERIAL=y: BREAK + 'h' on
the console UART now produces the SysRq help dump in dmesg; the brk
counter in /proc/tty/driver/serial increments per BREAK as expected.
Build tested on tty-next (base 16e95bfb79b5).
[1] https://lore.kernel.org/linux-serial/5efe9e03-4d86-43a0-9ec2-e610ff31095d@free.fr/
Jacques Nilo (3):
serial: core: introduce guard(uart_port_lock_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 | 13 +++++++++++++
3 files changed, 19 insertions(+), 3 deletions(-)
base-commit: 16e95bfb79b5d9d01dc7651d98caf3c2ace331cd
--
2.43.0
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
On Wed, 13 May 2026, 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]. Thanks for the report and working on a fix. This issue hit me hard last week when chasing a bug with one of my systems where my debug hacks caused me to become unable to become root and reboot the system properly. To my surprise I was unable to access any of the magic SysRq features either and consequently I had to power-cycle the system remotely via a PDU (it's some 1600km/1000mi away). I've now verified that 1/3 and 2/3 bring the feature back with patched 7.0.0 and said x86 PC. No way to verify 3/3 though. Maciej
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
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.0On Wed, 13 May 2026, Jacques Nilo wrote: > 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. Tested-by: Maciej W. Rozycki <macro@orcam.me.uk> Maciej
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.
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
On Wed, 13 May 2026, Jacques Nilo wrote: > 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. Tested-by: Maciej W. Rozycki <macro@orcam.me.uk> Maciej
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
© 2016 - 2026 Red Hat, Inc.