* [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
* 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
* [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 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
* 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
* [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 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