Linux Serial subsystem development
 help / color / mirror / Atom feed
* [PATCH v3 1/2] serial: qcom-geni: trace: Add tracepoint support for Qualcomm GENI serial
From: Praveen Talari @ 2026-05-18 17:56 UTC (permalink / raw)
  To: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
	Greg Kroah-Hartman, Jiri Slaby, Konrad Dybcio
  Cc: linux-kernel, linux-trace-kernel, linux-arm-msm, linux-serial,
	Mukesh Kumar Savaliya, Aniket Randive, chandana.chiluveru,
	jyothi.seerapu, Praveen Talari
In-Reply-To: <20260518-add-tracepoints-for-qcom-geni-serial-v3-0-b4addb151376@oss.qualcomm.com>

Add tracepoint support to the Qualcomm GENI serial driver to provide
runtime visibility into driver behavior without requiring invasive debug
patches.

The trace events cover UART termios configuration, clock setup, modem
control state, interrupt status, and TX/RX data, making it easier to
diagnose communication issues in the field.

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Signed-off-by: Praveen Talari <praveen.talari@oss.qualcomm.com>
---
v2->v3:
- Removed \n from geni_serial_tx_data and geni_serial_rx_data events.
- Resolved aligment issues in geni_serial_data, geni_serial_tx_data and
  geni_serial_rx_data events.

v1->v2:
- Removed multiple TX/RX trace events, instead used
  DECLARE_EVENT_CLASS and DEFINE_EVENT.
---
 include/trace/events/qcom_geni_serial.h | 164 ++++++++++++++++++++++++++++++++
 1 file changed, 164 insertions(+)

diff --git a/include/trace/events/qcom_geni_serial.h b/include/trace/events/qcom_geni_serial.h
new file mode 100644
index 000000000000..417ec01f9fc8
--- /dev/null
+++ b/include/trace/events/qcom_geni_serial.h
@@ -0,0 +1,164 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM qcom_geni_serial
+
+#if !defined(_TRACE_QCOM_GENI_SERIAL_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_QCOM_GENI_SERIAL_H
+
+#include <linux/device.h>
+#include <linux/tracepoint.h>
+
+TRACE_EVENT(geni_serial_set_termios,
+	    TP_PROTO(struct device *dev, unsigned int baud,
+		     unsigned int bits_per_char, u32 tx_trans_cfg,
+		     u32 tx_parity_cfg, u32 rx_trans_cfg,
+		     u32 rx_parity_cfg, u32 stop_bit_len),
+	    TP_ARGS(dev, baud, bits_per_char, tx_trans_cfg, tx_parity_cfg,
+		    rx_trans_cfg, rx_parity_cfg, stop_bit_len),
+
+	    TP_STRUCT__entry(__string(name, dev_name(dev))
+			     __field(unsigned int, baud)
+			     __field(unsigned int, bits_per_char)
+			     __field(u32, tx_trans_cfg)
+			     __field(u32, tx_parity_cfg)
+			     __field(u32, rx_trans_cfg)
+			     __field(u32, rx_parity_cfg)
+			     __field(u32, stop_bit_len)
+	    ),
+
+	    TP_fast_assign(__assign_str(name);
+			   __entry->baud = baud;
+			   __entry->bits_per_char = bits_per_char;
+			   __entry->tx_trans_cfg = tx_trans_cfg;
+			   __entry->tx_parity_cfg = tx_parity_cfg;
+			   __entry->rx_trans_cfg = rx_trans_cfg;
+			   __entry->rx_parity_cfg = rx_parity_cfg;
+			   __entry->stop_bit_len = stop_bit_len;
+	    ),
+
+	    TP_printk("%s: baud=%u bpc=%u tx_trans=0x%08x tx_par=0x%08x rx_trans=0x%08x rx_par=0x%08x stop=%u",
+		      __get_str(name), __entry->baud, __entry->bits_per_char,
+		      __entry->tx_trans_cfg, __entry->tx_parity_cfg,
+		      __entry->rx_trans_cfg, __entry->rx_parity_cfg,
+		      __entry->stop_bit_len)
+);
+
+TRACE_EVENT(geni_serial_clk_cfg,
+	    TP_PROTO(struct device *dev, unsigned int desired_rate,
+		     unsigned long clk_rate, unsigned int clk_div,
+		     unsigned int clk_idx),
+	    TP_ARGS(dev, desired_rate, clk_rate, clk_div, clk_idx),
+
+	    TP_STRUCT__entry(__string(name, dev_name(dev))
+			     __field(unsigned int, desired_rate)
+			     __field(unsigned long, clk_rate)
+			     __field(unsigned int, clk_div)
+			     __field(unsigned int, clk_idx)
+	    ),
+
+	    TP_fast_assign(__assign_str(name);
+			   __entry->desired_rate = desired_rate;
+			   __entry->clk_rate = clk_rate;
+			   __entry->clk_div = clk_div;
+			   __entry->clk_idx = clk_idx;
+	    ),
+
+	    TP_printk("%s: desired_rate=%u clk_rate=%lu clk_div=%u clk_idx=%u",
+		      __get_str(name), __entry->desired_rate, __entry->clk_rate,
+		      __entry->clk_div, __entry->clk_idx)
+);
+
+TRACE_EVENT(geni_serial_irq,
+	    TP_PROTO(struct device *dev, u32 m_irq, u32 s_irq,
+		     u32 dma_tx, u32 dma_rx),
+	    TP_ARGS(dev, m_irq, s_irq, dma_tx, dma_rx),
+
+	    TP_STRUCT__entry(__string(name, dev_name(dev))
+			     __field(u32, m_irq)
+			     __field(u32, s_irq)
+			     __field(u32, dma_tx)
+			     __field(u32, dma_rx)
+	    ),
+
+	    TP_fast_assign(__assign_str(name);
+			   __entry->m_irq = m_irq;
+			   __entry->s_irq = s_irq;
+			   __entry->dma_tx = dma_tx;
+			   __entry->dma_rx = dma_rx;
+	    ),
+
+	    TP_printk("%s: m_irq=0x%08x s_irq=0x%08x dma_tx=0x%08x dma_rx=0x%08x",
+		      __get_str(name), __entry->m_irq, __entry->s_irq,
+		      __entry->dma_tx, __entry->dma_rx)
+);
+
+DECLARE_EVENT_CLASS(geni_serial_data,
+		    TP_PROTO(struct device *dev, const u8 *buf, unsigned int len),
+		    TP_ARGS(dev, buf, len),
+
+		    TP_STRUCT__entry(__string(name, dev_name(dev))
+				     __field(unsigned int, len)
+				     __dynamic_array(u8, data, len)
+		    ),
+
+		    TP_fast_assign(__assign_str(name);
+				   __entry->len = len;
+				   memcpy(__get_dynamic_array(data), buf, len);
+		    ),
+
+		    TP_printk("%s: len=%u data=%s",
+			      __get_str(name), __entry->len,
+			      __print_hex(__get_dynamic_array(data), __entry->len))
+);
+
+DEFINE_EVENT(geni_serial_data, geni_serial_tx_data,
+	     TP_PROTO(struct device *dev, const u8 *buf, unsigned int len),
+	     TP_ARGS(dev, buf, len)
+);
+
+DEFINE_EVENT(geni_serial_data, geni_serial_rx_data,
+	     TP_PROTO(struct device *dev, const u8 *buf, unsigned int len),
+	     TP_ARGS(dev, buf, len)
+);
+
+TRACE_EVENT(geni_serial_set_mctrl,
+	    TP_PROTO(struct device *dev, unsigned int mctrl,
+		     u32 uart_manual_rfr),
+	    TP_ARGS(dev, mctrl, uart_manual_rfr),
+
+	    TP_STRUCT__entry(__string(name, dev_name(dev))
+			     __field(unsigned int, mctrl)
+			     __field(u32, uart_manual_rfr)
+	    ),
+
+	    TP_fast_assign(__assign_str(name);
+			   __entry->mctrl = mctrl;
+			   __entry->uart_manual_rfr = uart_manual_rfr;
+	    ),
+
+	    TP_printk("%s: mctrl=0x%04x uart_manual_rfr=0x%08x",
+		      __get_str(name), __entry->mctrl, __entry->uart_manual_rfr)
+);
+
+TRACE_EVENT(geni_serial_get_mctrl,
+	    TP_PROTO(struct device *dev, unsigned int mctrl, u32 geni_ios),
+	    TP_ARGS(dev, mctrl, geni_ios),
+
+	    TP_STRUCT__entry(__string(name, dev_name(dev))
+			     __field(unsigned int, mctrl)
+			     __field(u32, geni_ios)
+	    ),
+
+	    TP_fast_assign(__assign_str(name);
+			   __entry->mctrl = mctrl;
+			   __entry->geni_ios = geni_ios;
+	    ),
+
+	    TP_printk("%s: mctrl=0x%04x geni_ios=0x%08x",
+		      __get_str(name), __entry->mctrl, __entry->geni_ios)
+);
+
+#endif /* _TRACE_QCOM_GENI_SERIAL_H */
+
+/* This part must be outside protection */
+#include <trace/define_trace.h>

-- 
2.34.1


^ permalink raw reply related

* [PATCH v3 0/2] Add tracepoints support for Qualcomm GENI Serial drivers
From: Praveen Talari @ 2026-05-18 17:56 UTC (permalink / raw)
  To: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
	Greg Kroah-Hartman, Jiri Slaby, Konrad Dybcio
  Cc: linux-kernel, linux-trace-kernel, linux-arm-msm, linux-serial,
	Mukesh Kumar Savaliya, Aniket Randive, chandana.chiluveru,
	jyothi.seerapu, Praveen Talari

Add tracepoints to the Qualcomm GENI (Generic Interface) serial driver.
These trace events enable runtime debugging and performance analysis of
UART operations.

The trace events cover UART termios configuration, clock setup, manual
control state, interrupt status, and actual transmitted/received data in
hexadecimal format.

Usage examples:

Enable all serial traces:
  echo 1 > /sys/kernel/debug/tracing/events/qcom_geni_serial/enable
  cat /sys/kernel/debug/tracing/trace_pipe

Example trace output:

2517.938432: geni_serial_clk_cfg: a94000.serial: desired_rate=1843200
     clk_rate=7372800 clk_div=4 clk_idx=0
2517.938753: geni_serial_irq: a94000.serial: m_irq=0x88800000
     s_irq=0x08000111 dma_tx=0x00000000 dma_rx=0x00000000
2517.938803: geni_serial_set_termios: a94000.serial: baud=115200 bpc=8
     tx_trans=0x00000002 tx_par=0x00000000 rx_trans=0x00000000
rx_par=0x00000000 stop=0
2517.938807: geni_serial_set_mctrl: a94000.serial: mctrl=0x8006
     uart_manual_rfr=0x00000000
2517.938818: geni_serial_get_mctrl: a94000.serial: mctrl=0x0160
     geni_ios=0x00000001
2517.939165: geni_serial_irq: a94000.serial: m_irq=0x00400000
     s_irq=0x00000000 dma_tx=0x00000000 dma_rx=0x00000000
2517.939592: geni_serial_tx_data: a94000.serial: tx_len=8 data=61 62 63
     64 65 66 67 68
2517.940610: geni_serial_irq: a94000.serial: m_irq=0x00000001
     s_irq=0x00000000 dma_tx=0x00000003 dma_rx=0x00000000
2517.942174: geni_serial_irq: a94000.serial: m_irq=0x08000000
     s_irq=0x08000100 dma_tx=0x00000000 dma_rx=0x00000003
2517.942323: geni_serial_rx_data: a94000.serial: rx_len=8 data=61 62 63
     64 65 66 67 68
2517.942680: geni_serial_set_mctrl: a94000.serial: mctrl=0x8000
     uart_manual_rfr=0x80000002

Signed-off-by: Praveen Talari <praveen.talari@oss.qualcomm.com>
---
Changes in v3:
- Removed \n from geni_serial_tx_data and geni_serial_rx_data events.
- Resolved aligment issues in geni_serial_data, geni_serial_tx_data and
  geni_serial_rx_data events.
- Link to v2: https://lore.kernel.org/r/20260512-add-tracepoints-for-qcom-geni-serial-v2-0-a5726421b3af@oss.qualcomm.com

Changes in v2:
- removed multiple trace events for TX/RX events, instead used
  DECLARE_EVENT_CLASS and DEFINE_EVENT.
- Link to v1: https://lore.kernel.org/r/20260506-add-tracepoints-for-qcom-geni-serial-v1-0-544b22612e08@oss.qualcomm.com

---
Praveen Talari (2):
      serial: qcom-geni: trace: Add tracepoint support for Qualcomm GENI serial
      serial: qcom-geni: Add tracepoints for Qualcomm GENI serial driver

 drivers/tty/serial/qcom_geni_serial.c   |  27 +++++-
 include/trace/events/qcom_geni_serial.h | 164 ++++++++++++++++++++++++++++++++
 2 files changed, 187 insertions(+), 4 deletions(-)
---
base-commit: 1f5ffc672165ff851063a5fd044b727ab2517ae3
change-id: 20260427-add-tracepoints-for-qcom-geni-serial-948777218b7b

Best regards,
-- 
Praveen Talari <praveen.talari@oss.qualcomm.com>


^ permalink raw reply

* Re: [PATCH 00/13] Add DMA support for LINFlexD UART driver
From: Enric Balletbo i Serra @ 2026-05-18 14:14 UTC (permalink / raw)
  To: Jared Kangas
  Cc: Larisa Grigore, gregkh, jirislaby, robh, krzk+dt, conor+dt,
	sumit.semwal, christian.koenig, chester62515, cosmin.stoica,
	adrian.nitu, stefan-gabriel.mirea, Mihaela.Martinas, linux-kernel,
	linux-serial, devicetree, linux-media, dri-devel, linaro-mm-sig,
	s32, imx, clizzi, aruizrui, echanude
In-Reply-To: <aaGkGwbk-sh0YJqj@jkangas-thinkpadp1gen3.rmtuswa.csb>

Hi all,

Any chance these series can be considered? They still apply on top of
the mainline kernel.

On Fri, Feb 27, 2026 at 3:03 PM Jared Kangas <jkangas@redhat.com> wrote:
>
> Hi Larisa,
>
> On Mon, Feb 16, 2026 at 04:01:52PM +0100, Larisa Grigore wrote:
> > This patchset enhances the LINFlexD UART driver and its device tree bindings to
> > support DMA transfers, configurable clock inputs, dynamic baudrate changes, and
> > termios features. It also includes a series of fixes and improvements to ensure
> > reliable operation across various modes and configurations.
> >
> > The changes added can be summarized as follows:
> > 1. Fixes with respect to FIFO handling, locking, interrupt related registers and
> > INITM mode transition.
>
> Tested this series with the default devicetree configuration by booting
> the board to a login prompt about 200 times. Without the series applied,
> I was seeing a bug roughly every 30-50 boots where the kernel would
> would hang in linflex_console_putchar() waiting for DTFTFF. In my tests
> with the series applied, I didn't see any regressions and the bug no
> longer appeared. Thanks for the fix!
>
> Tested-by: Jared Kangas <jkangas@redhat.com> # S32G3, interrupt-driven
>

FWIW I also reproduced the issue Jared faced. Current state of the
LinFLEX serial driver in mainline seems a bit buggy and I can confirm
that these fix the problem.

Tested-by: Enric Balletbo i Serra <eballetb@redhat.com>

> > 2. Removal of the earlycon workaround, as proper FIFO handling and INITM
> > transitions now ensure stable behavior.
> > 3. Support for configurable stop bits and dynamic baudrate changes based on
> > clock inputs and termios settings.
> > 4. Optional DMA support for RX and TX paths, preventing character loss during
> > high-throughput operations like copy-paste. Cyclic DMA is used for RX to avoid
> > gaps between transactions.
> >
> > Larisa Grigore (8):
> >   serial: linflexuart: Clean SLEEP bit in LINCR1 after suspend
> >   serial: linflexuart: Check FIFO full before writing
> >   serial: linflexuart: Correctly clear UARTSR in buffer mode
> >   serial: linflexuart: Update RXEN/TXEN outside INITM mode
> >   serial: linflexuart: Ensure FIFO is empty when entering INITM
> >   serial: linflexuart: Revert earlycon workaround
> >   serial: linflexuart: Add support for configurable stop bits
> >   serial: linflexuart: Add DMA support
> >
> > Radu Pirea (5):
> >   serial: linflexuart: Fix locking in set_termios
> >   dt-bindings: serial: fsl-linflexuart: add clock input properties
> >   dt-bindings: serial: fsl-linflexuart: add dma properties
> >   serial: linflexuart: Add support for changing baudrate
> >   serial: linflexuart: Avoid stopping DMA during receive operations
> >
> >  .../bindings/serial/fsl,s32-linflexuart.yaml  |  31 +
> >  drivers/tty/serial/fsl_linflexuart.c          | 972 +++++++++++++++---
> >  2 files changed, 846 insertions(+), 157 deletions(-)
> >
> > --
> > 2.47.0
> >
>


^ permalink raw reply

* Re: [PATCH v2 1/2] serial: qcom-geni: trace: Add tracepoint support for Qualcomm GENI serial
From: Konrad Dybcio @ 2026-05-18 13:48 UTC (permalink / raw)
  To: Praveen Talari, Steven Rostedt, Masami Hiramatsu,
	Mathieu Desnoyers, Greg Kroah-Hartman, Jiri Slaby
  Cc: linux-kernel, linux-trace-kernel, linux-arm-msm, linux-serial,
	Mukesh Kumar Savaliya, Aniket Randive, chandana.chiluveru,
	jyothi.seerapu
In-Reply-To: <20260512-add-tracepoints-for-qcom-geni-serial-v2-1-a5726421b3af@oss.qualcomm.com>

On 5/12/26 7:14 PM, Praveen Talari wrote:
> Add tracepoint support to the Qualcomm GENI serial driver to provide
> runtime visibility into driver behavior without requiring invasive debug
> patches.
> 
> The trace events cover UART termios configuration, clock setup, modem
> control state, interrupt status, and TX/RX data, making it easier to
> diagnose communication issues in the field.
> 
> Signed-off-by: Praveen Talari <praveen.talari@oss.qualcomm.com>
> ---
> v1->v2:
> - Removed multiple TX/RX trace events, instead used
>   DECLARE_EVENT_CLASS and DEFINE_EVENT.
> ---
>  include/trace/events/qcom_geni_serial.h | 172 ++++++++++++++++++++++++++++++++
>  1 file changed, 172 insertions(+)
> 
> diff --git a/include/trace/events/qcom_geni_serial.h b/include/trace/events/qcom_geni_serial.h
> new file mode 100644
> index 000000000000..5e23827881d0
> --- /dev/null
> +++ b/include/trace/events/qcom_geni_serial.h

Oh, I only noticed now that this isn't in a subsystem/driver-
local directory.. I suppose it's up to the other maintainers
whether they like that

Konrad

^ permalink raw reply

* Re: [PATCH v2 2/2] serial: qcom-geni: Add tracepoints for Qualcomm GENI serial driver
From: Konrad Dybcio @ 2026-05-18 13:41 UTC (permalink / raw)
  To: Praveen Talari, Steven Rostedt, Masami Hiramatsu,
	Mathieu Desnoyers, Greg Kroah-Hartman, Jiri Slaby
  Cc: linux-kernel, linux-trace-kernel, linux-arm-msm, linux-serial,
	Mukesh Kumar Savaliya, Aniket Randive, chandana.chiluveru,
	jyothi.seerapu
In-Reply-To: <20260512-add-tracepoints-for-qcom-geni-serial-v2-2-a5726421b3af@oss.qualcomm.com>

On 5/12/26 7:14 PM, Praveen Talari wrote:
> Add tracing to the Qualcomm GENI serial driver to improve runtime
> observability.
> 
> Trace hooks are added at key points including termios and clock
> configuration, manual control get/set, interrupt handling, and data
> TX/RX paths.
> 
> Usage examples:
> 
> Enable all serial traces:
>   echo 1 > /sys/kernel/debug/tracing/events/qcom_geni_serial/enable
>   cat /sys/kernel/debug/tracing/trace_pipe
> 
> Example trace output:
> 
> 2517.938432: geni_serial_clk_cfg: a94000.serial: desired_rate=1843200
>      clk_rate=7372800 clk_div=4 clk_idx=0
> 2517.938753: geni_serial_irq: a94000.serial: m_irq=0x88800000
>      s_irq=0x08000111 dma_tx=0x00000000 dma_rx=0x00000000
> 2517.938803: geni_serial_set_termios: a94000.serial: baud=115200 bpc=8
>      tx_trans=0x00000002 tx_par=0x00000000 rx_trans=0x00000000
> rx_par=0x00000000 stop=0
> 2517.938807: geni_serial_set_mctrl: a94000.serial: mctrl=0x8006
>      uart_manual_rfr=0x00000000
> 2517.938818: geni_serial_get_mctrl: a94000.serial: mctrl=0x0160
>      geni_ios=0x00000001
> 2517.939165: geni_serial_irq: a94000.serial: m_irq=0x00400000
>      s_irq=0x00000000 dma_tx=0x00000000 dma_rx=0x00000000
> 2517.939592: geni_serial_tx_data: a94000.serial: tx_len=8 data=61 62 63
>      64 65 66 67 68
> 2517.940610: geni_serial_irq: a94000.serial: m_irq=0x00000001
>      s_irq=0x00000000 dma_tx=0x00000003 dma_rx=0x00000000
> 2517.942174: geni_serial_irq: a94000.serial: m_irq=0x08000000
>      s_irq=0x08000100 dma_tx=0x00000000 dma_rx=0x00000003
> 2517.942323: geni_serial_rx_data: a94000.serial: rx_len=8 data=61 62 63
>      64 65 66 67 68
> 2517.942680: geni_serial_set_mctrl: a94000.serial: mctrl=0x8000
>      uart_manual_rfr=0x80000002

I think the example (or at least the data that it produces) could go
under the --- line, there's plenty of docs regarding tracing on
docs.kernel.org

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

Konrad

^ permalink raw reply

* Re: [PATCH v2 1/2] serial: qcom-geni: trace: Add tracepoint support for Qualcomm GENI serial
From: Konrad Dybcio @ 2026-05-18 13:40 UTC (permalink / raw)
  To: Praveen Talari, Steven Rostedt, Masami Hiramatsu,
	Mathieu Desnoyers, Greg Kroah-Hartman, Jiri Slaby
  Cc: linux-kernel, linux-trace-kernel, linux-arm-msm, linux-serial,
	Mukesh Kumar Savaliya, Aniket Randive, chandana.chiluveru,
	jyothi.seerapu
In-Reply-To: <20260512-add-tracepoints-for-qcom-geni-serial-v2-1-a5726421b3af@oss.qualcomm.com>

On 5/12/26 7:14 PM, Praveen Talari wrote:
> Add tracepoint support to the Qualcomm GENI serial driver to provide
> runtime visibility into driver behavior without requiring invasive debug
> patches.
> 
> The trace events cover UART termios configuration, clock setup, modem
> control state, interrupt status, and TX/RX data, making it easier to
> diagnose communication issues in the field.
> 
> Signed-off-by: Praveen Talari <praveen.talari@oss.qualcomm.com>
> ---

[...]

> +DEFINE_EVENT(geni_serial_data, geni_serial_tx_data,
> +
> +	TP_PROTO(struct device *dev, const u8 *buf, unsigned int len),
> +
> +	TP_ARGS(dev, buf, len)
> +
> +);
> +
> +DEFINE_EVENT(geni_serial_data, geni_serial_rx_data,
> +
> +	TP_PROTO(struct device *dev, const u8 *buf, unsigned int len),
> +
> +	TP_ARGS(dev, buf, len)
> +
> +);

stray \ns above

otherwise lgtm

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

Konrad

^ permalink raw reply

* Re: [PATCH 01/15] serial: 8250: split Moxa PCIe serial board support out of 8250_pci
From: Crescent Hsieh @ 2026-05-18 12:53 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: gregkh, jirislaby, ilpo.jarvinen, fangpingfp.cheng, linux-kernel,
	linux-serial
In-Reply-To: <CAHp75VfFMLnLDP0V3U=4zG4Ayj71-ZgVkJsVtgNE=52tGQ963w@mail.gmail.com>

On Mon, May 04, 2026 at 04:27:44PM +0300, Andy Shevchenko wrote:
> On Mon, May 4, 2026 at 11:49 AM Crescent Hsieh
> <crescentcy.hsieh@moxa.com> wrote:
> > +static void mxpcie8250_remove(struct pci_dev *pdev)
> > +{
> > +       struct mxpcie8250 *priv = dev_get_drvdata(&pdev->dev);
> 
> platform_get_drvdata() IIRC

In a previous patchset, dev_get_drvdata() was suggested for similar
cases [1].

Since 8250_mxpcie is a PCI driver, could you clarify what you would
prefer here for retrieving the private data? I was not sure whether the
intention was to use dev_get_drvdata(), pci_get_drvdata(), or something
else, since platform_get_drvdata() does not seem to match this driver
type.

[1]
https://lore.kernel.org/all/CAHp75VcPanVWaLi39Wf-pq8nA+xbeJUs=v1BACz-+Sns0BVyWg@mail.gmail.com/

---
Sincerely,
Crescent Hsieh

^ permalink raw reply

* Re: [PATCH] drm: Use named initializers for arrays of i2c_device_data
From: Uwe Kleine-König (The Capable Hub) @ 2026-05-18 11:07 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jiri Slaby, Tapio Reijonen, Dan Carpenter,
	Xichao Zhao, Bartosz Golaszewski, Hugo Villeneuve
  Cc: linux-kernel, linux-serial
In-Reply-To: <20260518101456.632410-2-u.kleine-koenig@baylibre.com>

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

Hello,

I messed up the Subject, of course this should have been:

	[PATCH] tty: serial: Use named initializers for arrays of i2c_device_data

please fix up accordingly iff you apply this version. If I should resend
for this reason, please tell me.

Best regards
Uwe

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply

* [PATCH] drm: Use named initializers for arrays of i2c_device_data
From: Uwe Kleine-König (The Capable Hub) @ 2026-05-18 10:14 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jiri Slaby, Tapio Reijonen, Dan Carpenter,
	Xichao Zhao, Bartosz Golaszewski, Hugo Villeneuve
  Cc: linux-kernel, linux-serial

While being less compact, using named initializers allows to more easily
see which members of the structs are assigned which value without having
to lookup the declaration of the struct. And it's also more robust
against changes to the struct definition.

The mentioned robustness is relevant for a planned change to struct
i2c_device_id that replaces .driver_data by an anonymous union.

While touching all these arrays, unify usage of whitespace in the list
terminator.

This patch doesn't modify the compiled arrays, only their representation
in source form benefits. The former was confirmed with x86 and arm64
builds.

Signed-off-by: Uwe Kleine-König (The Capable Hub) <u.kleine-koenig@baylibre.com>
---
Hello,

the mentioned change to i2c_device_id is the following:

	diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
	index 23ff24080dfd..aebd3a5e90af 100644
	--- a/include/linux/mod_devicetable.h
	+++ b/include/linux/mod_devicetable.h
	@@ -477,7 +477,11 @@ struct rpmsg_device_id {

	 struct i2c_device_id {
		char name[I2C_NAME_SIZE];
	-	kernel_ulong_t driver_data;	/* Data private to the driver */
	+	union {
	+		/* Data private to the driver */
	+		kernel_ulong_t driver_data;
	+		const void *driver_data_ptr;
	+	};
	 };

	 /* pci_epf */

and this requires that .driver_data is assigned via a named initializer
for static data. This requirement isn't a bad one because named
initializers are also much better readable than list initializers.

The union added to struct i2c_device_id enables further cleanups like:

	diff --git a/drivers/regulator/ad5398.c b/drivers/regulator/ad5398.c
	index 0123ca8157a8..dfb0b07500a7 100644
	--- a/drivers/regulator/ad5398.c
	+++ b/drivers/regulator/ad5398.c
	@@ -207,8 +207,8 @@ struct ad5398_current_data_format {
	 static const struct ad5398_current_data_format df_10_4_120 = {10, 4, 0, 120000};

	 static const struct i2c_device_id ad5398_id[] = {
	-	{ .name = "ad5398", .driver_data = (kernel_ulong_t)&df_10_4_120 },
	-	{ .name = "ad5821", .driver_data = (kernel_ulong_t)&df_10_4_120 },
	+	{ .name = "ad5398", .driver_data_ptr = &df_10_4_120 },
	+	{ .name = "ad5821", .driver_data_ptr = &df_10_4_120 },
	 	{ }
	 };
	 MODULE_DEVICE_TABLE(i2c, ad5398_id);
	@@ -219,8 +219,7 @@ static int ad5398_probe(struct i2c_client *client)
	 	struct regulator_init_data *init_data = dev_get_platdata(&client->dev);
	 	struct regulator_config config = { };
	 	struct ad5398_chip_info *chip;
	-	const struct ad5398_current_data_format *df =
	-	                (struct ad5398_current_data_format *)id->driver_data;
	+	const struct ad5398_current_data_format *df = id->driver_data;

	 	chip = devm_kzalloc(&client->dev, sizeof(*chip), GFP_KERNEL);
	 	if (!chip)

that are an improvement for readability (again!) and it keeps some
properties of the pointers (here: being const) without having to pay
attention for that. (I didn't find a serial driver that benefits, so
this is "only" a regulator driver example.)

My additional motivation for this effort is CHERI[1]. This is a hardware
extension that uses 128 bit pointers but unsigned long is still 64 bit.
So with CHERI you cannot store pointers in unsigned long variables.

Best regards
Uwe

[1] https://cheri-alliance.org/discover-cheri/
    https://lwn.net/Articles/1037974/

 drivers/tty/serial/max310x.c       |  8 ++++----
 drivers/tty/serial/sc16is7xx_i2c.c | 14 +++++++-------
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/tty/serial/max310x.c b/drivers/tty/serial/max310x.c
index ac7d3f197c3a..742b8c23bacf 100644
--- a/drivers/tty/serial/max310x.c
+++ b/drivers/tty/serial/max310x.c
@@ -1671,10 +1671,10 @@ static void max310x_i2c_remove(struct i2c_client *client)
 }
 
 static const struct i2c_device_id max310x_i2c_id_table[] = {
-	{ "max3107",	(kernel_ulong_t)&max3107_devtype, },
-	{ "max3108",	(kernel_ulong_t)&max3108_devtype, },
-	{ "max3109",	(kernel_ulong_t)&max3109_devtype, },
-	{ "max14830",	(kernel_ulong_t)&max14830_devtype, },
+	{ .name = "max3107", .driver_data = (kernel_ulong_t)&max3107_devtype },
+	{ .name = "max3108", .driver_data = (kernel_ulong_t)&max3108_devtype },
+	{ .name = "max3109", .driver_data = (kernel_ulong_t)&max3109_devtype },
+	{ .name = "max14830", .driver_data = (kernel_ulong_t)&max14830_devtype },
 	{ }
 };
 MODULE_DEVICE_TABLE(i2c, max310x_i2c_id_table);
diff --git a/drivers/tty/serial/sc16is7xx_i2c.c b/drivers/tty/serial/sc16is7xx_i2c.c
index 699376c3b3a5..6c2a697556a6 100644
--- a/drivers/tty/serial/sc16is7xx_i2c.c
+++ b/drivers/tty/serial/sc16is7xx_i2c.c
@@ -39,13 +39,13 @@ static void sc16is7xx_i2c_remove(struct i2c_client *client)
 }
 
 static const struct i2c_device_id sc16is7xx_i2c_id_table[] = {
-	{ "sc16is74x",	(kernel_ulong_t)&sc16is74x_devtype, },
-	{ "sc16is740",	(kernel_ulong_t)&sc16is74x_devtype, },
-	{ "sc16is741",	(kernel_ulong_t)&sc16is74x_devtype, },
-	{ "sc16is750",	(kernel_ulong_t)&sc16is750_devtype, },
-	{ "sc16is752",	(kernel_ulong_t)&sc16is752_devtype, },
-	{ "sc16is760",	(kernel_ulong_t)&sc16is760_devtype, },
-	{ "sc16is762",	(kernel_ulong_t)&sc16is762_devtype, },
+	{ .name = "sc16is74x", .driver_data = (kernel_ulong_t)&sc16is74x_devtype },
+	{ .name = "sc16is740", .driver_data = (kernel_ulong_t)&sc16is74x_devtype },
+	{ .name = "sc16is741", .driver_data = (kernel_ulong_t)&sc16is74x_devtype },
+	{ .name = "sc16is750", .driver_data = (kernel_ulong_t)&sc16is750_devtype },
+	{ .name = "sc16is752", .driver_data = (kernel_ulong_t)&sc16is752_devtype },
+	{ .name = "sc16is760", .driver_data = (kernel_ulong_t)&sc16is760_devtype },
+	{ .name = "sc16is762", .driver_data = (kernel_ulong_t)&sc16is762_devtype },
 	{ }
 };
 MODULE_DEVICE_TABLE(i2c, sc16is7xx_i2c_id_table);

base-commit: 254f49634ee16a731174d2ae34bc50bd5f45e731
-- 
2.47.3


^ permalink raw reply related

* Re: [PATCH v2] tty: serial: atmel: Ignore chars when CREAD is cleared
From: Richard GENOUD @ 2026-05-18  7:20 UTC (permalink / raw)
  To: Rakesh Alasyam, gregkh
  Cc: richard.genoud, jirislaby, nicolas.ferre, alexandre.belloni,
	claudiu.beznea, linux-serial, linux-kernel, linux-arm-kernel
In-Reply-To: <20260511165913.36467-1-alasyamrakesh77@gmail.com>

Hi Rakesh,

Le 11/05/2026 à 18:59, Rakesh Alasyam a écrit :
> Ignore received characters when CREAD is cleared by adding RXRDY
> to ignore_status_mask.
> 
> This replaces an existing TODO in the driver.
> 
> Tested on hardware.
Could you be more precise?
Which board(s) did you test with? (e.g. sama5d3_explained, custom...)
Which SoC? (sam9g35, sama5d2...)
With DMA, PDC or none?

Regards,
Richard
> 
> Signed-off-by: Rakesh Alasyam <alasyamrakesh77@gmail.com>
> 
> ---
> 
> v2:
> - Add blank line before comment
> - Tested on hardware
> ---
>   drivers/tty/serial/atmel_serial.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
> index 5d8c1cfc1c60..5c756dc904b0 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -2184,7 +2184,8 @@ static void atmel_set_termios(struct uart_port *port,
>   		if (termios->c_iflag & IGNPAR)
>   			port->ignore_status_mask |= ATMEL_US_OVRE;
>   	}
> -	/* TODO: Ignore all characters if CREAD is set.*/
> +	if (!(termios->c_cflag & CREAD))
> +		port->ignore_status_mask |= ATMEL_US_RXRDY;
>   
>   	/* update the per-port timeout */
>   	uart_update_timeout(port, termios->c_cflag, baud);


^ permalink raw reply

* [PATCH v9] Bluetooth: hci_uart: fix UAFs and race conditions in close and init paths
From: w15303746062 @ 2026-05-18  2:49 UTC (permalink / raw)
  To: luiz.dentz, pmenzel, marcel, linux-bluetooth
  Cc: linux-serial, linux-kernel, greg, stable, Mingyu Wang
In-Reply-To: <CABBYNZ+r3gm37FW5WqE79bRp+x9UZsaCtyvfz_FdixqEucAxGw@mail.gmail.com>

From: Mingyu Wang <25181214217@stu.xidian.edu.cn>

Vulnerabilities leading to Use-After-Free (UAF) and Null Pointer
Dereference (NPD) conditions were observed in the lifecycle management
of hci_uart.

The primary issue arises because the workqueues (init_ready and
write_work) are only flushed/cancelled if the HCI_UART_PROTO_READY
flag is set during TTY close. If a hangup occurs before setup completes,
hci_uart_tty_close() skips the teardown of these workqueues and
proceeds to free the `hu` struct. When the scheduled work executes
later, it blindly dereferences the freed `hu` struct.

Furthermore, several data races and UAFs were identified in the teardown
sequence:
1. Calling hci_uart_flush() from hci_uart_close() without effectively
   disabling write_work causes a race condition where both can concurrently
   double-free hu->tx_skb. This happens because protocol timers can
   concurrently invoke hci_uart_tx_wakeup() and requeue write_work.
2. Calling hci_free_dev(hdev) before hu->proto->close(hu) causes a UAF
   when vendor specific protocol close callbacks dereference hu->hdev.
3. In the initialization error paths, failing to take the proto_lock
   write lock before clearing PROTO_READY leads to races with active
   readers. Additionally, hci_uart_tty_receive() accesses hu->hdev
   outside the read lock, leading to UAFs if the initialization error
   path frees hdev concurrently.

Fix these synchronization and lifecycle issues by:
1. Re-ordering hci_uart_tty_close() to clear HCI_UART_PROTO_READY first,
   followed immediately by a cancel_work_sync(&hu->write_work). Clearing
   the flag locks out concurrent protocol timers from successfully invoking
   hci_uart_tx_wakeup(), effectively rendering the cancellation permanent
   and preventing the tx_skb double-free.
2. Note: Clearing PROTO_READY early causes hci_uart_close() to skip
   hu->proto->flush(). This is perfectly safe in the tty_close path
   because hu->proto->close() executes shortly after, which intrinsically
   purges all protocol SKB queues and tears down the state.
3. Relocating hu->proto->close(hu) strictly prior to hci_free_dev(hdev)
   across all close and error paths to prevent vendor-level UAFs.
4. Moving the hdev->stat.byte_rx increment in hci_uart_tty_receive()
   inside the proto_lock read-side critical section to safely synchronize
   with device unregistration.
5. Adding cancel_work_sync(&hu->write_work) to hci_uart_close() to safely
   flush the workqueue before hci_uart_flush() is invoked via the HCI core.
6. Utilizing cancel_work_sync() instead of disable_work_sync() across
   all paths to prevent permanently breaking user-space retry capabilities.

Fixes: 3b799254cf6f ("Bluetooth: hci_uart: Cancel init work before unregistering")
Cc: stable@vger.kernel.org
Signed-off-by: Mingyu Wang <25181214217@stu.xidian.edu.cn>
---
Changes in v9:
- Addressed a critical flaw identified in v8 where premature cancellation of write_work allowed active protocol timers to immediately reschedule it. The teardown sequence in hci_uart_tty_close() now strictly clears HCI_UART_PROTO_READY *before* calling cancel_work_sync(&hu->write_work). This permanently locks out hci_uart_tx_wakeup(), completely resolving the lingering UAF and double-free races.
- Documented that skipping hu->proto->flush() via early flag clearance is intrinsically safe, as hu->proto->close() executes subsequently to purge all unacked/rel queues.

Changes in v8:
- Corrected the teardown sequence in hci_uart_tty_close() by unconditionally canceling write_work BEFORE hci_uart_close().
- Moved hu->hdev->stat.byte_rx increment inside the proto_lock read-side critical section in hci_uart_tty_receive() to prevent read-side UAF against concurrent registration failures.
- Added cancel_work_sync(&hu->write_work) inside hci_uart_close() to eliminate the race condition between write_work and hci_uart_flush() when the interface is brought down via the HCI core.

Changes in v7:
- Reverted disable_work_sync() back to cancel_work_sync() across all error and close paths to preserve user-space retry capabilities.
- Synchronized workqueue teardown safely by atomically clearing PROTO_READY / PROTO_INIT under proto_lock prior to calling cancel_work_sync().
- Fixed a Use-After-Free (UAF) vulnerability in the teardown sequence by relocating hu->proto->close(hu) strictly prior to hci_free_dev(hdev).
- Added cancel_work_sync(&hu->init_ready) at the very beginning of hci_uart_tty_close() to serialize teardown against active asynchronous registration.

Changes in v6:
- Fixed missing `hu->proto_lock` write lock in hci_uart_init_work() error path to prevent race with readers (reported by Sashiko).
- Added disable_work_sync() instead of cancel_work_sync() for `hu->write_work` in hci_uart_init_work() and hci_uart_register_dev() error paths.

Changes in v5:
- Relocated disable_work_sync() to the very top of hci_uart_tty_close(), 
  before hci_uart_close(), to ensure no new work is submitted during device teardown.

Changes in v4:
- Adopted Luiz's suggestion to use disable_work_sync() instead of 
  cancel_work_sync() in close path to prevent new work submissions.

Changes in v3:
- Added 'Cc: stable' tag as requested by the stable bot.

Changes in v2:
- Added KASAN/ODEBUG crash trace.

 drivers/bluetooth/hci_ldisc.c | 48 +++++++++++++++++++++++++++++------
 1 file changed, 40 insertions(+), 8 deletions(-)

diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
index 275ea865bc29..47f4902b40b4 100644
--- a/drivers/bluetooth/hci_ldisc.c
+++ b/drivers/bluetooth/hci_ldisc.c
@@ -194,7 +194,15 @@ void hci_uart_init_work(struct work_struct *work)
 	err = hci_register_dev(hu->hdev);
 	if (err < 0) {
 		BT_ERR("Can't register HCI device");
+
+		percpu_down_write(&hu->proto_lock);
 		clear_bit(HCI_UART_PROTO_READY, &hu->flags);
+		percpu_up_write(&hu->proto_lock);
+
+		/* Safely cancel work after clearing flags */
+		cancel_work_sync(&hu->write_work);
+
+		/* Close protocol before freeing hdev */
 		hu->proto->close(hu);
 		hdev = hu->hdev;
 		hu->hdev = NULL;
@@ -263,8 +271,12 @@ static int hci_uart_open(struct hci_dev *hdev)
 /* Close device */
 static int hci_uart_close(struct hci_dev *hdev)
 {
+	struct hci_uart *hu = hci_get_drvdata(hdev);
+
 	BT_DBG("hdev %p", hdev);
 
+	cancel_work_sync(&hu->write_work);
+
 	hci_uart_flush(hdev);
 	hdev->flush = NULL;
 	return 0;
@@ -531,6 +543,7 @@ static void hci_uart_tty_close(struct tty_struct *tty)
 {
 	struct hci_uart *hu = tty->disc_data;
 	struct hci_dev *hdev;
+	bool proto_ready;
 
 	BT_DBG("tty %p", tty);
 
@@ -540,24 +553,38 @@ static void hci_uart_tty_close(struct tty_struct *tty)
 	if (!hu)
 		return;
 
-	hdev = hu->hdev;
-	if (hdev)
-		hci_uart_close(hdev);
+	/* Wait for init_ready to finish to prevent registration races */
+	cancel_work_sync(&hu->init_ready);
 
-	if (test_bit(HCI_UART_PROTO_READY, &hu->flags)) {
+	proto_ready = test_bit(HCI_UART_PROTO_READY, &hu->flags);
+	if (proto_ready) {
 		percpu_down_write(&hu->proto_lock);
 		clear_bit(HCI_UART_PROTO_READY, &hu->flags);
 		percpu_up_write(&hu->proto_lock);
+	}
 
-		cancel_work_sync(&hu->init_ready);
-		cancel_work_sync(&hu->write_work);
+	/*
+	 * Unconditionally cancel write_work AFTER clearing PROTO_READY.
+	 * This ensures that concurrent protocol timers cannot requeue
+	 * write_work via hci_uart_tx_wakeup(), permanently preventing
+	 * double-free races and UAFs.
+	 */
+	cancel_work_sync(&hu->write_work);
+
+	hdev = hu->hdev;
+	if (hdev)
+		hci_uart_close(hdev); /* proto->flush is safely skipped */
 
+	if (proto_ready) {
 		if (hdev) {
 			if (test_bit(HCI_UART_REGISTERED, &hu->flags))
 				hci_unregister_dev(hdev);
-			hci_free_dev(hdev);
 		}
+		/* Close protocol before freeing hdev (intrinsically purges queues) */
 		hu->proto->close(hu);
+
+		if (hdev)
+			hci_free_dev(hdev);
 	}
 	clear_bit(HCI_UART_PROTO_SET, &hu->flags);
 
@@ -625,11 +652,12 @@ static void hci_uart_tty_receive(struct tty_struct *tty, const u8 *data,
 	 * tty caller
 	 */
 	hu->proto->recv(hu, data, count);
-	percpu_up_read(&hu->proto_lock);
 
 	if (hu->hdev)
 		hu->hdev->stat.byte_rx += count;
 
+	percpu_up_read(&hu->proto_lock);
+
 	tty_unthrottle(tty);
 }
 
@@ -695,6 +723,10 @@ static int hci_uart_register_dev(struct hci_uart *hu)
 		percpu_down_write(&hu->proto_lock);
 		clear_bit(HCI_UART_PROTO_INIT, &hu->flags);
 		percpu_up_write(&hu->proto_lock);
+		/* Cancel work after clearing flags */
+		cancel_work_sync(&hu->write_work);
+
+		/* Close protocol before freeing hdev */
 		hu->proto->close(hu);
 		hu->hdev = NULL;
 		hci_free_dev(hdev);
-- 
2.34.1


^ permalink raw reply related

* [PATCH v8] Bluetooth: hci_uart: fix UAFs and race conditions in close and init paths
From: w15303746062 @ 2026-05-18  1:36 UTC (permalink / raw)
  To: luiz.dentz, pmenzel, marcel, linux-bluetooth
  Cc: linux-serial, linux-kernel, greg, stable, Mingyu Wang
In-Reply-To: <CABBYNZ+r3gm37FW5WqE79bRp+x9UZsaCtyvfz_FdixqEucAxGw@mail.gmail.com>

From: Mingyu Wang <25181214217@stu.xidian.edu.cn>

Vulnerabilities leading to Use-After-Free (UAF) and Null Pointer
Dereference (NPD) conditions were observed in the lifecycle management
of hci_uart.

The primary issue arises because the workqueues (init_ready and
write_work) are only flushed/cancelled if the HCI_UART_PROTO_READY
flag is set during TTY close. If a hangup occurs before setup completes,
hci_uart_tty_close() skips the teardown of these workqueues and
proceeds to free the `hu` struct. When the scheduled work executes
later, it blindly dereferences the freed `hu` struct.

Furthermore, several data races and UAFs were identified in the teardown
sequence:
1. Calling hci_uart_flush() from hci_uart_close() without canceling
   write_work causes a race condition where both can concurrently
   double-free hu->tx_skb. This occurs both in TTY hangup and when the
   HCI device is closed via the HCI core.
2. Calling hci_free_dev(hdev) before hu->proto->close(hu) causes a UAF
   when vendor specific protocol close callbacks dereference hu->hdev.
3. In the initialization error paths, failing to take the proto_lock
   write lock before clearing PROTO_READY leads to races with active
   readers. Additionally, hci_uart_tty_receive() accesses hu->hdev
   outside the read lock, leading to UAFs if the initialization error
   path frees hdev concurrently.

Fix these synchronization and lifecycle issues by:
1. Re-ordering hci_uart_tty_close() to unconditionally cancel init_ready
   and write_work first. This prevents the double-free race in
   hci_uart_flush(), while preserving the HCI_UART_PROTO_READY flag so
   underlying hu->proto->flush() callbacks can still execute safely.
2. Relocating hu->proto->close(hu) strictly prior to hci_free_dev(hdev)
   across all close and error paths to prevent vendor-level UAFs.
3. Moving the hdev->stat.byte_rx increment in hci_uart_tty_receive()
   inside the proto_lock read-side critical section to safely synchronize
   with device unregistration.
4. Adding cancel_work_sync(&hu->write_work) to hci_uart_close() to safely
   flush the workqueue before hci_uart_flush() is invoked.
5. Utilizing cancel_work_sync() instead of disable_work_sync() after
   flags are cleared to safely flush workqueues without permanently
   breaking user-space retry capabilities.

Fixes: 3b799254cf6f ("Bluetooth: hci_uart: Cancel init work before unregistering")
Cc: stable@vger.kernel.org
Signed-off-by: Mingyu Wang <25181214217@stu.xidian.edu.cn>
---
Changes in v8:
- Corrected the teardown sequence in hci_uart_tty_close() by unconditionally canceling write_work BEFORE hci_uart_close(). This completely prevents the tx_skb double-free without prematurely clearing PROTO_READY, ensuring underlying hu->proto->flush(hu) runs correctly.
- Moved hu->hdev->stat.byte_rx increment inside the proto_lock read-side critical section in hci_uart_tty_receive() to prevent read-side UAF against concurrent registration failures.
- Added cancel_work_sync(&hu->write_work) inside hci_uart_close() to eliminate the race condition between write_work and hci_uart_flush() when the interface is brought down via the HCI core.

Changes in v7:
- Reverted disable_work_sync() back to cancel_work_sync() across all error and close paths to preserve user-space retry capabilities.
- Synchronized workqueue teardown safely by atomically clearing PROTO_READY / PROTO_INIT under proto_lock prior to calling cancel_work_sync().
- Fixed a Use-After-Free (UAF) vulnerability in the teardown sequence by relocating hu->proto->close(hu) strictly prior to hci_free_dev(hdev).
- Added cancel_work_sync(&hu->init_ready) at the very beginning of hci_uart_tty_close() to serialize teardown against active asynchronous registration.

Changes in v6:
- Fixed missing `hu->proto_lock` write lock in hci_uart_init_work() error path to prevent race with readers (reported by Sashiko).
- Added disable_work_sync() instead of cancel_work_sync() for `hu->write_work` in hci_uart_init_work() and hci_uart_register_dev() error paths.

Changes in v5:
- Relocated disable_work_sync() to the very top of hci_uart_tty_close(), 
  before hci_uart_close(), to ensure no new work is submitted during device teardown.

Changes in v4:
- Adopted Luiz's suggestion to use disable_work_sync() instead of 
  cancel_work_sync() in close path to prevent new work submissions.

Changes in v3:
- Added 'Cc: stable' tag as requested by the stable bot.

Changes in v2:
- Added KASAN/ODEBUG crash trace.

 drivers/bluetooth/hci_ldisc.c | 33 ++++++++++++++++++++++++++++-----
 1 file changed, 28 insertions(+), 5 deletions(-)

diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
index 275ea865bc29..cb56194daad1 100644
--- a/drivers/bluetooth/hci_ldisc.c
+++ b/drivers/bluetooth/hci_ldisc.c
@@ -194,7 +194,15 @@ void hci_uart_init_work(struct work_struct *work)
 	err = hci_register_dev(hu->hdev);
 	if (err < 0) {
 		BT_ERR("Can't register HCI device");
+
+		percpu_down_write(&hu->proto_lock);
 		clear_bit(HCI_UART_PROTO_READY, &hu->flags);
+		percpu_up_write(&hu->proto_lock);
+
+		/* Safely cancel work after clearing flags */
+		cancel_work_sync(&hu->write_work);
+
+		/* Close protocol before freeing hdev */
 		hu->proto->close(hu);
 		hdev = hu->hdev;
 		hu->hdev = NULL;
@@ -263,8 +271,12 @@ static int hci_uart_open(struct hci_dev *hdev)
 /* Close device */
 static int hci_uart_close(struct hci_dev *hdev)
 {
+	struct hci_uart *hu = hci_get_drvdata(hdev);
+
 	BT_DBG("hdev %p", hdev);
 
+	cancel_work_sync(&hu->write_work);
+
 	hci_uart_flush(hdev);
 	hdev->flush = NULL;
 	return 0;
@@ -540,6 +552,12 @@ static void hci_uart_tty_close(struct tty_struct *tty)
 	if (!hu)
 		return;
 
+	/* Wait for init_ready to finish to prevent registration races */
+	cancel_work_sync(&hu->init_ready);
+
+	/* Unconditionally cancel write_work BEFORE hci_uart_close() to prevent double-free */
+	cancel_work_sync(&hu->write_work);
+
 	hdev = hu->hdev;
 	if (hdev)
 		hci_uart_close(hdev);
@@ -549,15 +567,15 @@ static void hci_uart_tty_close(struct tty_struct *tty)
 		clear_bit(HCI_UART_PROTO_READY, &hu->flags);
 		percpu_up_write(&hu->proto_lock);
 
-		cancel_work_sync(&hu->init_ready);
-		cancel_work_sync(&hu->write_work);
-
 		if (hdev) {
 			if (test_bit(HCI_UART_REGISTERED, &hu->flags))
 				hci_unregister_dev(hdev);
-			hci_free_dev(hdev);
 		}
+		/* Close protocol before freeing hdev */
 		hu->proto->close(hu);
+
+		if (hdev)
+			hci_free_dev(hdev);
 	}
 	clear_bit(HCI_UART_PROTO_SET, &hu->flags);
 
@@ -625,11 +643,12 @@ static void hci_uart_tty_receive(struct tty_struct *tty, const u8 *data,
 	 * tty caller
 	 */
 	hu->proto->recv(hu, data, count);
-	percpu_up_read(&hu->proto_lock);
 
 	if (hu->hdev)
 		hu->hdev->stat.byte_rx += count;
 
+	percpu_up_read(&hu->proto_lock);
+
 	tty_unthrottle(tty);
 }
 
@@ -695,6 +714,10 @@ static int hci_uart_register_dev(struct hci_uart *hu)
 		percpu_down_write(&hu->proto_lock);
 		clear_bit(HCI_UART_PROTO_INIT, &hu->flags);
 		percpu_up_write(&hu->proto_lock);
+		/* Cancel work after clearing flags */
+		cancel_work_sync(&hu->write_work);
+
+		/* Close protocol before freeing hdev */
 		hu->proto->close(hu);
 		hu->hdev = NULL;
 		hci_free_dev(hdev);
-- 
2.34.1


^ permalink raw reply related

* [PATCH] ARM: Move Footbridge-specific header into mach-footbridge
From: Ethan Nelson-Moore @ 2026-05-17  4:57 UTC (permalink / raw)
  To: linux-arm-kernel, linux-serial, linux-watchdog
  Cc: Ethan Nelson-Moore, Russell King, Arnd Bergmann,
	Greg Kroah-Hartman, Miquel Raynal, Richard Weinberger,
	Vignesh Raghavendra, Jiri Slaby, Wim Van Sebroeck, Guenter Roeck

arch/arm/include/asm/hardware/dec21285.h is specific to the DC21285
Footbridge chip and should not be in the global ARM include directory.
Move it into mach-footbridge where it belongs. It was included twice in
arch/arm/mach-footbridge/common.c; remove one of the includes.
Also remove the file path from the header (it is bad style and would
become outdated) and add missing include guards.

Tested by compiling footbridge_defconfig and netwinder_defconfig,
modified to additionally enable CONFIG_MTD_DC21285 and
CONFIG_DEBUG_FOOTBRIDGE_COM1 or CONFIG_DEBUG_DC21285_PORT, respectively
(these are the only Footbridge-related options not enabled by the
defconfigs).

Signed-off-by: Ethan Nelson-Moore <enelsonmoore@gmail.com>
---
This patch depends on my previous patch "ARM: clean up machine-specific
PCI code and move it into mach-footbridge" because it touches the same
area of arch/arm/mach-footbridge/dc21285.c.

 MAINTAINERS                                               | 1 -
 arch/arm/include/debug/dc21285.S                          | 2 +-
 arch/arm/mach-footbridge/common.c                         | 3 +--
 arch/arm/mach-footbridge/dc21285-timer.c                  | 2 +-
 arch/arm/mach-footbridge/dc21285.c                        | 2 +-
 arch/arm/mach-footbridge/dma-isa.c                        | 2 +-
 arch/arm/mach-footbridge/ebsa285.c                        | 2 +-
 .../hardware => mach-footbridge/include/mach}/dec21285.h  | 8 +++++---
 arch/arm/mach-footbridge/isa-irq.c                        | 2 +-
 arch/arm/mach-footbridge/isa.c                            | 2 +-
 arch/arm/mach-footbridge/netwinder-hw.c                   | 2 +-
 drivers/char/nwflash.c                                    | 2 +-
 drivers/mtd/maps/dc21285.c                                | 2 +-
 drivers/tty/serial/21285.c                                | 2 +-
 drivers/watchdog/wdt285.c                                 | 2 +-
 15 files changed, 18 insertions(+), 18 deletions(-)
 rename arch/arm/{include/asm/hardware => mach-footbridge/include/mach}/dec21285.h (98%)

diff --git a/MAINTAINERS b/MAINTAINERS
index c2c6d79275c6..37ecfe4bc4e4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2811,7 +2811,6 @@ M:	Russell King <linux@armlinux.org.uk>
 L:	linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
 S:	Maintained
 W:	http://www.armlinux.org.uk/
-F:	arch/arm/include/asm/hardware/dec21285.h
 F:	arch/arm/mach-footbridge/
 
 ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
diff --git a/arch/arm/include/debug/dc21285.S b/arch/arm/include/debug/dc21285.S
index 4ec0e5e31704..c0eb58ba7d7e 100644
--- a/arch/arm/include/debug/dc21285.S
+++ b/arch/arm/include/debug/dc21285.S
@@ -7,7 +7,7 @@
  *  Moved from linux/arch/arm/kernel/debug.S by Ben Dooks
 */
 
-#include <asm/hardware/dec21285.h>
+#include <mach/dec21285.h>
 
 #include <mach/hardware.h>
 	/* For EBSA285 debugging */
diff --git a/arch/arm/mach-footbridge/common.c b/arch/arm/mach-footbridge/common.c
index 85c598708c10..d3076bf03875 100644
--- a/arch/arm/mach-footbridge/common.c
+++ b/arch/arm/mach-footbridge/common.c
@@ -20,7 +20,6 @@
 #include <asm/mach-types.h>
 #include <asm/setup.h>
 #include <asm/system_misc.h>
-#include <asm/hardware/dec21285.h>
 
 #include <asm/mach/irq.h>
 #include <asm/mach/map.h>
@@ -30,7 +29,7 @@
 
 #include <mach/hardware.h>
 #include <mach/irqs.h>
-#include <asm/hardware/dec21285.h>
+#include <mach/dec21285.h>
 
 static int dc21285_get_irq(void)
 {
diff --git a/arch/arm/mach-footbridge/dc21285-timer.c b/arch/arm/mach-footbridge/dc21285-timer.c
index 2908c9ef3c9b..f5d0024783e3 100644
--- a/arch/arm/mach-footbridge/dc21285-timer.c
+++ b/arch/arm/mach-footbridge/dc21285-timer.c
@@ -14,7 +14,7 @@
 
 #include <asm/irq.h>
 
-#include <asm/hardware/dec21285.h>
+#include <mach/dec21285.h>
 #include <asm/mach/time.h>
 #include <asm/system_info.h>
 
diff --git a/arch/arm/mach-footbridge/dc21285.c b/arch/arm/mach-footbridge/dc21285.c
index 5a68b6739ecf..923c808e8ba1 100644
--- a/arch/arm/mach-footbridge/dc21285.c
+++ b/arch/arm/mach-footbridge/dc21285.c
@@ -19,7 +19,7 @@
 
 #include <asm/irq.h>
 #include <asm/mach/pci.h>
-#include <asm/hardware/dec21285.h>
+#include <mach/dec21285.h>
 
 #include "pci.h"
 
diff --git a/arch/arm/mach-footbridge/dma-isa.c b/arch/arm/mach-footbridge/dma-isa.c
index 937f5376d5e7..300cdf6ef223 100644
--- a/arch/arm/mach-footbridge/dma-isa.c
+++ b/arch/arm/mach-footbridge/dma-isa.c
@@ -19,7 +19,7 @@
 
 #include <asm/dma.h>
 #include <asm/mach/dma.h>
-#include <asm/hardware/dec21285.h>
+#include <mach/dec21285.h>
 
 #define ISA_DMA_MASK		0
 #define ISA_DMA_MODE		1
diff --git a/arch/arm/mach-footbridge/ebsa285.c b/arch/arm/mach-footbridge/ebsa285.c
index 1cb7d674bc81..93ab333e3027 100644
--- a/arch/arm/mach-footbridge/ebsa285.c
+++ b/arch/arm/mach-footbridge/ebsa285.c
@@ -10,7 +10,7 @@
 #include <linux/slab.h>
 #include <linux/leds.h>
 
-#include <asm/hardware/dec21285.h>
+#include <mach/dec21285.h>
 #include <asm/mach-types.h>
 
 #include <asm/mach/arch.h>
diff --git a/arch/arm/include/asm/hardware/dec21285.h b/arch/arm/mach-footbridge/include/mach/dec21285.h
similarity index 98%
rename from arch/arm/include/asm/hardware/dec21285.h
rename to arch/arm/mach-footbridge/include/mach/dec21285.h
index 894f2a635cbb..35d10e2dcade 100644
--- a/arch/arm/include/asm/hardware/dec21285.h
+++ b/arch/arm/mach-footbridge/include/mach/dec21285.h
@@ -1,11 +1,13 @@
 /* SPDX-License-Identifier: GPL-2.0-only */
 /*
- *  arch/arm/include/asm/hardware/dec21285.h
- *
  *  Copyright (C) 1998 Russell King
  *
  *  DC21285 registers
  */
+
+#ifndef __MACH_DEC21285_H
+#define __MACH_DEC21285_H
+
 #define DC21285_PCI_IACK		0x79000000
 #define DC21285_ARMCSR_BASE		0x42000000
 #define DC21285_PCI_TYPE_0_CONFIG	0x7b000000
@@ -135,4 +137,4 @@
 #define TIMER_CNTL_DIV256	(2 << 2)
 #define TIMER_CNTL_CNTEXT	(3 << 2)
 
-
+#endif /* __MACH_DEC21285_H */
diff --git a/arch/arm/mach-footbridge/isa-irq.c b/arch/arm/mach-footbridge/isa-irq.c
index 842ddb4121ef..f9231e84028d 100644
--- a/arch/arm/mach-footbridge/isa-irq.c
+++ b/arch/arm/mach-footbridge/isa-irq.c
@@ -21,7 +21,7 @@
 #include <asm/mach/irq.h>
 
 #include <mach/hardware.h>
-#include <asm/hardware/dec21285.h>
+#include <mach/dec21285.h>
 #include <asm/irq.h>
 #include <asm/mach-types.h>
 
diff --git a/arch/arm/mach-footbridge/isa.c b/arch/arm/mach-footbridge/isa.c
index 84caccddce44..a028920e8f12 100644
--- a/arch/arm/mach-footbridge/isa.c
+++ b/arch/arm/mach-footbridge/isa.c
@@ -8,7 +8,7 @@
 #include <linux/serial_8250.h>
 
 #include <asm/irq.h>
-#include <asm/hardware/dec21285.h>
+#include <mach/dec21285.h>
 
 #include "common.h"
 
diff --git a/arch/arm/mach-footbridge/netwinder-hw.c b/arch/arm/mach-footbridge/netwinder-hw.c
index c024eefd4978..bd21c455a495 100644
--- a/arch/arm/mach-footbridge/netwinder-hw.c
+++ b/arch/arm/mach-footbridge/netwinder-hw.c
@@ -16,7 +16,7 @@
 #include <linux/slab.h>
 #include <linux/leds.h>
 
-#include <asm/hardware/dec21285.h>
+#include <mach/dec21285.h>
 #include <asm/mach-types.h>
 #include <asm/setup.h>
 #include <asm/system_misc.h>
diff --git a/drivers/char/nwflash.c b/drivers/char/nwflash.c
index 9f52f0306ef7..21ac9b2df42e 100644
--- a/drivers/char/nwflash.c
+++ b/drivers/char/nwflash.c
@@ -29,7 +29,7 @@
 #include <linux/mutex.h>
 #include <linux/jiffies.h>
 
-#include <asm/hardware/dec21285.h>
+#include <mach/dec21285.h>
 #include <asm/io.h>
 #include <asm/mach-types.h>
 #include <linux/uaccess.h>
diff --git a/drivers/mtd/maps/dc21285.c b/drivers/mtd/maps/dc21285.c
index 70a3db3ab856..8bcb40489f4f 100644
--- a/drivers/mtd/maps/dc21285.c
+++ b/drivers/mtd/maps/dc21285.c
@@ -17,7 +17,7 @@
 #include <linux/mtd/partitions.h>
 
 #include <asm/io.h>
-#include <asm/hardware/dec21285.h>
+#include <mach/dec21285.h>
 #include <asm/mach-types.h>
 
 
diff --git a/drivers/tty/serial/21285.c b/drivers/tty/serial/21285.c
index 4de0c975ebdc..f20c2092e4a5 100644
--- a/drivers/tty/serial/21285.c
+++ b/drivers/tty/serial/21285.c
@@ -18,7 +18,7 @@
 #include <asm/irq.h>
 #include <asm/mach-types.h>
 #include <asm/system_info.h>
-#include <asm/hardware/dec21285.h>
+#include <mach/dec21285.h>
 #include <mach/hardware.h>
 
 #define BAUD_BASE		(mem_fclk_21285/64)
diff --git a/drivers/watchdog/wdt285.c b/drivers/watchdog/wdt285.c
index 78681d9f7d53..347cb2892833 100644
--- a/drivers/watchdog/wdt285.c
+++ b/drivers/watchdog/wdt285.c
@@ -30,7 +30,7 @@
 
 #include <asm/mach-types.h>
 #include <asm/system_info.h>
-#include <asm/hardware/dec21285.h>
+#include <mach/dec21285.h>
 
 /*
  * Define this to stop the watchdog actually rebooting the machine.
-- 
2.43.0


^ permalink raw reply related

* [PATCH v7] Bluetooth: hci_uart: fix UAFs and race conditions in close and init paths
From: w15303746062 @ 2026-05-16  8:47 UTC (permalink / raw)
  To: luiz.dentz, pmenzel, marcel, linux-bluetooth
  Cc: linux-serial, linux-kernel, greg, stable, Mingyu Wang
In-Reply-To: <CABBYNZ+r3gm37FW5WqE79bRp+x9UZsaCtyvfz_FdixqEucAxGw@mail.gmail.com>

From: Mingyu Wang <25181214217@stu.xidian.edu.cn>

Vulnerabilities leading to Use-After-Free (UAF) and Null Pointer
Dereference (NPD) conditions were observed in the lifecycle management
of hci_uart.

The primary issue arises because the workqueues (init_ready and
write_work) are only flushed/cancelled if the HCI_UART_PROTO_READY
flag is set during TTY close. If a hangup occurs before setup completes,
hci_uart_tty_close() skips the teardown of these workqueues and
proceeds to free the `hu` struct. When the scheduled work executes
later, it blindly dereferences the freed `hu` struct.

Furthermore, several data races and UAFs were identified in the teardown
sequence:
1. Calling hci_uart_close(hdev) before cancel_work_sync(&hu->write_work)
   causes a race condition where hci_uart_flush() and write_work
   can concurrently double-free hu->tx_skb.
2. Calling hci_free_dev(hdev) before hu->proto->close(hu) causes a UAF
   when vendor specific protocol close callbacks dereference hu->hdev.
3. In the initialization error paths, failing to take the proto_lock
   write lock before clearing PROTO_READY leads to races with active
   readers (e.g., hci_uart_tty_receive).

Fix these synchronization and lifecycle issues by:
1. Re-ordering hci_uart_tty_close() to unconditionally cancel init_ready
   first, then atomically clear PROTO_READY under proto_lock, and safely
   cancel write_work before touching hdev.
2. Relocating hu->proto->close(hu) strictly prior to hci_free_dev(hdev)
   across all close and error paths to prevent vendor-level UAFs.
3. Utilizing cancel_work_sync() instead of disable_work_sync() after
   flags are cleared to safely flush workqueues without permanently
   breaking user-space retry capabilities.

Fixes: 3b799254cf6f ("Bluetooth: hci_uart: Cancel init work before unregistering")
Cc: stable@vger.kernel.org
Signed-off-by: Mingyu Wang <25181214217@stu.xidian.edu.cn>
---
Changes in v7:
- Reverted disable_work_sync() back to cancel_work_sync() across all error and close paths to preserve user-space retry capabilities, addressing the regression introduced in v4/v6 where work items were permanently disabled.
- Synchronized workqueue teardown safely by atomically clearing PROTO_READY / PROTO_INIT under proto_lock prior to calling cancel_work_sync(), preventing any concurrent work requeuing.
- Fixed a Use-After-Free (UAF) vulnerability in the teardown sequence by relocating hu->proto->close(hu) strictly prior to hci_free_dev(hdev) in all close and error paths, ensuring vendor specific callbacks safely access hu->hdev.
- Added cancel_work_sync(&hu->init_ready) at the very beginning of hci_uart_tty_close() to serialize teardown against active asynchronous registration, eliminating race-induced double-frees.

Changes in v6:
- Fixed missing `hu->proto_lock` write lock in hci_uart_init_work() error path to prevent race with readers (reported by Sashiko).
- Added disable_work_sync() instead of cancel_work_sync() for `hu->write_work` in hci_uart_init_work() and hci_uart_register_dev() error paths to completely block any concurrent re-queuing window before hdev is freed (reported by Sashiko).

Changes in v5:
- Relocated disable_work_sync() to the very top of hci_uart_tty_close(), 
  before hci_uart_close(), to ensure no new work is submitted during device teardown.

Changes in v4:
- Adopted Luiz's suggestion to use disable_work_sync() instead of 
  cancel_work_sync() in close path to prevent new work submissions.

Changes in v3:
- Added 'Cc: stable' tag as requested by the stable bot.

Changes in v2:
- Added KASAN/ODEBUG crash trace.

 drivers/bluetooth/hci_ldisc.c | 35 ++++++++++++++++++++++++++++-------
 1 file changed, 28 insertions(+), 7 deletions(-)

diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
index 275ea865bc29..46a080f77cb1 100644
--- a/drivers/bluetooth/hci_ldisc.c
+++ b/drivers/bluetooth/hci_ldisc.c
@@ -194,7 +194,15 @@ void hci_uart_init_work(struct work_struct *work)
 	err = hci_register_dev(hu->hdev);
 	if (err < 0) {
 		BT_ERR("Can't register HCI device");
+
+		percpu_down_write(&hu->proto_lock);
 		clear_bit(HCI_UART_PROTO_READY, &hu->flags);
+		percpu_up_write(&hu->proto_lock);
+
+		/* Safely cancel work after clearing flags */
+		cancel_work_sync(&hu->write_work);
+
+		/* Close protocol before freeing hdev */
 		hu->proto->close(hu);
 		hdev = hu->hdev;
 		hu->hdev = NULL;
@@ -531,6 +539,7 @@ static void hci_uart_tty_close(struct tty_struct *tty)
 {
 	struct hci_uart *hu = tty->disc_data;
 	struct hci_dev *hdev;
+	bool proto_ready;
 
 	BT_DBG("tty %p", tty);
 
@@ -540,24 +549,32 @@ static void hci_uart_tty_close(struct tty_struct *tty)
 	if (!hu)
 		return;
 
-	hdev = hu->hdev;
-	if (hdev)
-		hci_uart_close(hdev);
+	/* Wait for init_ready to finish to prevent registration races */
+	cancel_work_sync(&hu->init_ready);
 
-	if (test_bit(HCI_UART_PROTO_READY, &hu->flags)) {
+	proto_ready = test_bit(HCI_UART_PROTO_READY, &hu->flags);
+	if (proto_ready) {
 		percpu_down_write(&hu->proto_lock);
 		clear_bit(HCI_UART_PROTO_READY, &hu->flags);
 		percpu_up_write(&hu->proto_lock);
+	}
+	/* Unconditionally cancel write_work after clearing flags */
+	cancel_work_sync(&hu->write_work);
 
-		cancel_work_sync(&hu->init_ready);
-		cancel_work_sync(&hu->write_work);
+	hdev = hu->hdev;
+	if (hdev)
+		hci_uart_close(hdev);
 
+	if (proto_ready) {
 		if (hdev) {
 			if (test_bit(HCI_UART_REGISTERED, &hu->flags))
 				hci_unregister_dev(hdev);
-			hci_free_dev(hdev);
 		}
+		/* Close protocol before freeing hdev */
 		hu->proto->close(hu);
+
+		if (hdev)
+			hci_free_dev(hdev);
 	}
 	clear_bit(HCI_UART_PROTO_SET, &hu->flags);
 
@@ -695,6 +712,10 @@ static int hci_uart_register_dev(struct hci_uart *hu)
 		percpu_down_write(&hu->proto_lock);
 		clear_bit(HCI_UART_PROTO_INIT, &hu->flags);
 		percpu_up_write(&hu->proto_lock);
+		/* Cancel work after clearing flags */
+		cancel_work_sync(&hu->write_work);
+
+		/* Close protocol before freeing hdev */
 		hu->proto->close(hu);
 		hu->hdev = NULL;
 		hci_free_dev(hdev);
-- 
2.34.1


^ permalink raw reply related

* [PATCH v6] Bluetooth: hci_uart: fix UAFs and race conditions in close and init paths
From: w15303746062 @ 2026-05-16  5:30 UTC (permalink / raw)
  To: luiz.dentz, pmenzel, marcel, linux-bluetooth
  Cc: linux-serial, linux-kernel, greg, stable, Mingyu Wang
In-Reply-To: <CABBYNZ+r3gm37FW5WqE79bRp+x9UZsaCtyvfz_FdixqEucAxGw@mail.gmail.com>

From: Mingyu Wang <25181214217@stu.xidian.edu.cn>

Vulnerabilities leading to Use-After-Free (UAF) and Null Pointer
Dereference (NPD) conditions were observed in the lifecycle management
of hci_uart.

The primary issue arises because the workqueues (init_ready and write_work)
are only flushed/cancelled if the HCI_UART_PROTO_READY flag is set during
TTY close. If a hangup occurs before setup completes, hci_uart_tty_close()
skips the teardown of these workqueues and proceeds to free the `hu` struct.
When the scheduled work executes later, it blindly dereferences the freed
`hu` struct. This issue was triggered by our custom device emulation and
fuzzing framework (DevGen) on the v6.18 kernel.

The crash trace is as follows:
  ODEBUG: free active (active state 0) object: ffff88804024e870 object type: work_struct hint: hci_uart_write_work+0x0/0x940
  WARNING: CPU: 0 PID: 338273 at lib/debugobjects.c:612 debug_print_object+0x1a2/0x2b0
  ...
  Call Trace:
   <TASK>
   debug_check_no_obj_freed+0x3ec/0x520
   kfree+0x3f0/0x6c0
   hci_uart_tty_close+0x127/0x2a0
   k_ldisc_close+0x113/0x1a0
   tty_ldisc_kill+0x8e/0x150
   tty_ldisc_hangup+0x3c1/0x730
   __tty_hangup.part.0+0x3fd/0x8a0
   tty_ioctl+0x120f/0x1690
   __x64_sys_ioctl+0x18f/0x210
   do_syscall_64+0xcb/0xfa0
   entry_SYSCALL_64_after_hwframe+0x77/0x7f
   </TASK>

Additionally, sister bugs exist in the device registration error paths
within hci_uart_init_work() and hci_uart_register_dev(). If
hci_register_dev() fails, the code frees hdev and sets hu->hdev to NULL,
but leaves hu->write_work active if it was already scheduled during early
protocol setup phase. When the work executes later, it blindly fetches and
dereferences the NULL hu->hdev pointer, triggering an unconditioned kernel
panic.

Furthermore, the error path in hci_uart_init_work() clears the
HCI_UART_PROTO_READY flag and invokes hu->proto->close(hu) without
acquiring the hu->proto_lock write lock. Since concurrent callbacks like
hci_uart_tty_receive() execute under the read lock, this missing
synchronization allows the protocol state to be destroyed while active
readers are accessing it.

Fix these synchronization and lifecycle issues by:
1. Moving the workqueue teardown to the very beginning of
   hci_uart_tty_close() and using disable_work_sync() to unconditionally
   prevent new work submissions.
2. Wrapping the clearance of HCI_UART_PROTO_READY in hci_uart_init_work()
   with percpu_down_write/percpu_up_write of hu->proto_lock.
3. Explicitly invoking disable_work_sync(&hu->write_work) before resetting
   hu->hdev to NULL in both initialization error paths to safely prevent
   any concurrent re-queuing issues.

Fixes: 3b799254cf6f ("Bluetooth: hci_uart: Cancel init work before unregistering")
Cc: stable@vger.kernel.org
Signed-off-by: Mingyu Wang <25181214217@stu.xidian.edu.cn>
---
Changes in v6:
- Fixed missing `hu->proto_lock` write lock in hci_uart_init_work() error path to prevent race with readers (reported by Sashiko).
- Added disable_work_sync() instead of cancel_work_sync() for `hu->write_work` in hci_uart_init_work() and hci_uart_register_dev() error paths to completely block any concurrent re-queuing window before hdev is freed (reported by Sashiko).

Changes in v5:
- Relocated disable_work_sync() to the very top of hci_uart_tty_close(), 
  before hci_uart_close(), to ensure no new work is submitted during device teardown.

Changes in v4:
- Adopted Luiz's suggestion to use disable_work_sync() instead of 
  cancel_work_sync() in close path to prevent new work submissions.

Changes in v3:
- Added 'Cc: stable' tag as requested by the stable bot.

Changes in v2:
- Added KASAN/ODEBUG crash trace.

 drivers/bluetooth/hci_ldisc.c | 35 ++++++++++++++++++++++++++++++++---
 1 file changed, 32 insertions(+), 3 deletions(-)

diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
index 275ea865bc29..612fa2890cec 100644
--- a/drivers/bluetooth/hci_ldisc.c
+++ b/drivers/bluetooth/hci_ldisc.c
@@ -194,7 +194,22 @@ void hci_uart_init_work(struct work_struct *work)
 	err = hci_register_dev(hu->hdev);
 	if (err < 0) {
 		BT_ERR("Can't register HCI device");
+
+		/*
+		 * Acquire proto_lock before clearing PROTO_READY and closing
+		 * the protocol to synchronize with active readers.
+		 */
+		percpu_down_write(&hu->proto_lock);
 		clear_bit(HCI_UART_PROTO_READY, &hu->flags);
+		percpu_up_write(&hu->proto_lock);
+
+		/*
+		 * Disable any pending write_work that may have been queued
+		 * during the PROTO_INIT phase to prevent UAF/NPD when hdev
+		 * is freed.
+		 */
+		disable_work_sync(&hu->write_work);
+
 		hu->proto->close(hu);
 		hdev = hu->hdev;
 		hu->hdev = NULL;
@@ -540,6 +555,15 @@ static void hci_uart_tty_close(struct tty_struct *tty)
 	if (!hu)
 		return;
 
+	/*
+	 * Disable workqueues unconditionally before tearing down the
+	 * connection, as they might be active during the PROTO_INIT phase.
+	 * Using disable_work_sync() ensures no new submissions can occur
+	 * during or after hci_uart_close().
+	 */
+	disable_work_sync(&hu->init_ready);
+	disable_work_sync(&hu->write_work);
+
 	hdev = hu->hdev;
 	if (hdev)
 		hci_uart_close(hdev);
@@ -549,9 +573,6 @@ static void hci_uart_tty_close(struct tty_struct *tty)
 		clear_bit(HCI_UART_PROTO_READY, &hu->flags);
 		percpu_up_write(&hu->proto_lock);
 
-		cancel_work_sync(&hu->init_ready);
-		cancel_work_sync(&hu->write_work);
-
 		if (hdev) {
 			if (test_bit(HCI_UART_REGISTERED, &hu->flags))
 				hci_unregister_dev(hdev);
@@ -692,6 +713,14 @@ static int hci_uart_register_dev(struct hci_uart *hu)
 
 	if (hci_register_dev(hdev) < 0) {
 		BT_ERR("Can't register HCI device");
+
+		/*
+		 * Disable any pending write_work that may have been queued
+		 * during the proto->open() phase to prevent UAF/NPD when
+		 * hdev is freed.
+		 */
+		disable_work_sync(&hu->write_work);
+
 		percpu_down_write(&hu->proto_lock);
 		clear_bit(HCI_UART_PROTO_INIT, &hu->flags);
 		percpu_up_write(&hu->proto_lock);
-- 
2.34.1


^ permalink raw reply related

* [PATCH v5] Bluetooth: hci_uart: fix UAF in hci_uart_tty_close()
From: w15303746062 @ 2026-05-16  2:22 UTC (permalink / raw)
  To: luiz.dentz, pmenzel, marcel, linux-bluetooth
  Cc: linux-serial, linux-kernel, greg, stable, Mingyu Wang
In-Reply-To: <CABBYNZ+r3gm37FW5WqE79bRp+x9UZsaCtyvfz_FdixqEucAxGw@mail.gmail.com>

From: Mingyu Wang <25181214217@stu.xidian.edu.cn>

A Use-After-Free (UAF) vulnerability and a subsequent kernel panic were
observed in hci_uart_write_work() due to a race condition between the
initialization of the HCI UART line discipline and concurrent TTY hangup.

This issue was triggered by our custom device emulation and fuzzing
framework (DevGen) on the v6.18 kernel. Due to the highly timing-dependent
nature of this race condition (requiring a precise interleaving of
TIOCVHANGUP and protocol setup), Syzkaller failed to extract a reliable
standalone C reproducer (reproducer is too unreliable: 0.00).

The crash trace is as follows:
  ODEBUG: free active (active state 0) object: ffff88804024e870 object type: work_struct hint: hci_uart_write_work+0x0/0x940
  WARNING: CPU: 0 PID: 338273 at lib/debugobjects.c:612 debug_print_object+0x1a2/0x2b0
  ...
  Call Trace:
   <TASK>
   debug_check_no_obj_freed+0x3ec/0x520
   kfree+0x3f0/0x6c0
   hci_uart_tty_close+0x127/0x2a0
   k_ldisc_close+0x113/0x1a0
   tty_ldisc_kill+0x8e/0x150
   tty_ldisc_hangup+0x3c1/0x730
   __tty_hangup.part.0+0x3fd/0x8a0
   tty_ioctl+0x120f/0x1690
   __x64_sys_ioctl+0x18f/0x210
   do_syscall_64+0xcb/0xfa0
   entry_SYSCALL_64_after_hwframe+0x77/0x7f
   </TASK>

The issue arises because the workqueues (init_ready and write_work) are
only flushed/cancelled if the HCI_UART_PROTO_READY flag is set. However,
during the protocol initialization phase (HCI_UART_PROTO_INIT), the
underlying protocol may schedule work. If a hangup occurs before the setup
completes and the READY flag is set, hci_uart_tty_close() skips the
teardown of these workqueues and proceeds to free the `hu` struct. When
the scheduled work executes later, it blindly dereferences the freed `hu`
struct.

Fix this by moving the workqueue teardown to the very beginning of
hci_uart_tty_close(), outside the HCI_UART_PROTO_READY check and prior to
calling hci_uart_close(). Furthermore, use disable_work_sync() instead of
cancel_work_sync() to unconditionally disable the works. This ensures that
any pending works are cancelled and no new submissions can occur before or
during the teardown of the device connection. Note that hu->init_ready and
hu->write_work are initialized in hci_uart_tty_open(), so it is always safe
to call disable_work_sync() on them in hci_uart_tty_close(), even if the
protocol was never fully attached.

Fixes: 3b799254cf6f ("Bluetooth: hci_uart: Cancel init work before unregistering")
Cc: stable@vger.kernel.org
Signed-off-by: Mingyu Wang <25181214217@stu.xidian.edu.cn>
---
Changes in v5:
- Moved disable_work_sync() to the very top of hci_uart_tty_close(), 
  before hci_uart_close(), to prevent any concurrent re-queuing 
  during device shutdown and resolve Sashiko static analysis warnings.

Changes in v4:
- Adopted Luiz's suggestion to use disable_work_sync() instead of 
  cancel_work_sync() to prevent new work submissions during teardown.

Changes in v3:
- Added 'Cc: stable' tag as requested by the stable bot.

Changes in v2:
- Added KASAN/ODEBUG crash trace.

 drivers/bluetooth/hci_ldisc.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
index 275ea865bc29..ebdbcd567cd2 100644
--- a/drivers/bluetooth/hci_ldisc.c
+++ b/drivers/bluetooth/hci_ldisc.c
@@ -540,6 +540,15 @@ static void hci_uart_tty_close(struct tty_struct *tty)
 	if (!hu)
 		return;
 
+	/*
+	 * Disable workqueues unconditionally before tearing down the
+	 * connection, as they might be active during the PROTO_INIT phase.
+	 * Using disable_work_sync() ensures no new submissions can occur
+	 * during or after hci_uart_close().
+	 */
+	disable_work_sync(&hu->init_ready);
+	disable_work_sync(&hu->write_work);
+
 	hdev = hu->hdev;
 	if (hdev)
 		hci_uart_close(hdev);
@@ -549,9 +558,6 @@ static void hci_uart_tty_close(struct tty_struct *tty)
 		clear_bit(HCI_UART_PROTO_READY, &hu->flags);
 		percpu_up_write(&hu->proto_lock);
 
-		cancel_work_sync(&hu->init_ready);
-		cancel_work_sync(&hu->write_work);
-
 		if (hdev) {
 			if (test_bit(HCI_UART_REGISTERED, &hu->flags))
 				hci_unregister_dev(hdev);
-- 
2.34.1


^ permalink raw reply related

* Re:Re: [PATCH v4] Bluetooth: hci_uart: fix UAF in hci_uart_tty_close()
From: w15303746062 @ 2026-05-16  1:41 UTC (permalink / raw)
  To: Luiz Augusto von Dentz
  Cc: pmenzel, marcel, linux-bluetooth, linux-serial, linux-kernel,
	greg, stable, Mingyu Wang
In-Reply-To: <CABBYNZ+r3gm37FW5WqE79bRp+x9UZsaCtyvfz_FdixqEucAxGw@mail.gmail.com>


Hi Luiz,

Thank you for checking it with Sashiko.


At 2026-05-16 00:08:05, "Luiz Augusto von Dentz" <luiz.dentz@gmail.com> wrote:
>Hi,
>
>On Fri, May 15, 2026 at 10:06 AM <w15303746062@163.com> wrote:
>>
>> From: Mingyu Wang <25181214217@stu.xidian.edu.cn>
>>
>> A Use-After-Free (UAF) vulnerability and a subsequent kernel panic were
>> observed in hci_uart_write_work() due to a race condition between the
>> initialization of the HCI UART line discipline and concurrent TTY hangup.
>>
>> This issue was triggered by our custom device emulation and fuzzing
>> framework (DevGen) on the v6.18 kernel. Due to the highly timing-dependent
>> nature of this race condition (requiring a precise interleaving of
>> TIOCVHANGUP and protocol setup), Syzkaller failed to extract a reliable
>> standalone C reproducer (reproducer is too unreliable: 0.00).
>>
>> The crash trace is as follows:
>>   ODEBUG: free active (active state 0) object: ffff88804024e870 object type: work_struct hint: hci_uart_write_work+0x0/0x940
>>   WARNING: CPU: 0 PID: 338273 at lib/debugobjects.c:612 debug_print_object+0x1a2/0x2b0
>>   ...
>>   Call Trace:
>>    <TASK>
>>    debug_check_no_obj_freed+0x3ec/0x520
>>    kfree+0x3f0/0x6c0
>>    hci_uart_tty_close+0x127/0x2a0
>>    tty_ldisc_close+0x113/0x1a0
>>    tty_ldisc_kill+0x8e/0x150
>>    tty_ldisc_hangup+0x3c1/0x730
>>    __tty_hangup.part.0+0x3fd/0x8a0
>>    tty_ioctl+0x120f/0x1690
>>    __x64_sys_ioctl+0x18f/0x210
>>    do_syscall_64+0xcb/0xfa0
>>    entry_SYSCALL_64_after_hwframe+0x77/0x7f
>>    </TASK>
>>
>> The issue arises because the workqueues (init_ready and write_work) are
>> only flushed/cancelled if the HCI_UART_PROTO_READY flag is set. However,
>> during the protocol initialization phase (HCI_UART_PROTO_INIT), the
>> underlying protocol may schedule work. If a hangup occurs before the setup
>> completes and the READY flag is set, hci_uart_tty_close() skips the
>> teardown of these workqueues and proceeds to free the `hu` struct. When
>> the scheduled work executes later, it blindly dereferences the freed `hu`
>> struct.
>>
>> Fix this by moving the workqueue teardown outside the HCI_UART_PROTO_READY
>> check. Furthermore, use disable_work_sync() instead of cancel_work_sync()
>> to unconditionally disable the works. This ensures that any pending works
>> are cancelled and no new submissions can occur before the hci_uart
>> structure is freed. Note that hu->init_ready and hu->write_work are
>> initialized in hci_uart_tty_open(), so it is always safe to call
>> disable_work_sync() on them in hci_uart_tty_close(), even if the protocol
>> was never fully attached.
>>
>> Fixes: 3b799254cf6f ("Bluetooth: hci_uart: Cancel init work before unregistering")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Mingyu Wang <25181214217@stu.xidian.edu.cn>
>> ---
>> Changes in v4:
>> - Adopted Luiz's suggestion to use disable_work_sync() instead of
>>   cancel_work_sync() to prevent new work submissions during teardown.
>>
>> Changes in v3:
>> - Added 'Cc: stable' tag as requested by the stable bot.
>>
>> Changes in v2:
>> - Added KASAN/ODEBUG crash trace.
>>
>>  drivers/bluetooth/hci_ldisc.c | 12 +++++++++---
>>  1 file changed, 9 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
>> index 275ea865bc29..333c1e1503e8 100644
>> --- a/drivers/bluetooth/hci_ldisc.c
>> +++ b/drivers/bluetooth/hci_ldisc.c
>> @@ -544,14 +544,20 @@ static void hci_uart_tty_close(struct tty_struct *tty)
>>         if (hdev)
>>                 hci_uart_close(hdev);
>>
>> +       /*
>> +        * Disable workqueues unconditionally before freeing the hu
>> +        * struct, as they might be active during the PROTO_INIT phase.
>> +        * Using disable_work_sync() instead of cancel_work_sync()
>> +        * ensures no new submissions can occur.
>> +        */
>> +       disable_work_sync(&hu->init_ready);
>> +       disable_work_sync(&hu->write_work);
>
>Looks like sashiko has a problem with these being after hci_uart_close:

I see the issue now. Placing `disable_work_sync()` after `hci_uart_close()`
could still leave a tiny window where the workqueues might race with the 
teardown of the `hdev` structure. 

The safest and most logical approach is to pull the `disable_work_sync()`
calls to the very top of `hci_uart_tty_close()`, before `hci_uart_close()` 
or any other teardown logic begins. This will completely choke off any 
asynchronous operations before we touch the connection state or hardware.

I will update the patch and send out v5 immediately.

>
>https://sashiko.dev/#/patchset/20260515140548.393865-1-w15303746062%40163.com
>
>>         if (test_bit(HCI_UART_PROTO_READY, &hu->flags)) {
>>                 percpu_down_write(&hu->proto_lock);
>>                 clear_bit(HCI_UART_PROTO_READY, &hu->flags);
>>                 percpu_up_write(&hu->proto_lock);
>>
>> -               cancel_work_sync(&hu->init_ready);
>> -               cancel_work_sync(&hu->write_work);
>> -
>>                 if (hdev) {
>>                         if (test_bit(HCI_UART_REGISTERED, &hu->flags))
>>                                 hci_unregister_dev(hdev);
>> --
>> 2.34.1
>>
>
>
>-- 
>Luiz Augusto von Dentz

Best regards,
Mingyu

^ permalink raw reply

* [PATCH v2] serial: max310x: fix compile errors if CONFIG_SPI_MASTER is disabled
From: Hugo Villeneuve @ 2026-05-15 18:30 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jiri Slaby, Hugo Villeneuve
  Cc: hugo, kernel test robot, linux-kernel, linux-serial

From: Hugo Villeneuve <hvilleneuve@dimonoff.com>

Since commit 20ffe4b3330a8 ("serial: max310x: allow driver to be built with
SPI or I2C"), if I2C is enabled and SPI_MASTER is disabled, we have these
compile errors:

  drivers/tty/serial/max310x.c: In function 'max310x_uart_init':
  drivers/tty/serial/max310x.c: error: 'max310x_spi_driver' undeclared...
  drivers/tty/serial/max310x.c: In function ‘max310x_uart_init’:
  drivers/tty/serial/max310x.c: error: label ‘err_spi_register’
  defined but not used...
  drivers/tty/serial/max310x.c: error: ‘regcfg’ defined but not used

Fix by properly encapsulating i2c/spi code/variables in their respective
context with IS_ENABLED() macros for CONFIG_I2C and CONFIG_SPI_MASTER.

Fixes: 20ffe4b3330a8 ("serial: max310x: allow driver to be built with SPI or I2C")
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202605121847.N9DVLNg2-lkp@intel.com/
Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
---
note: not Cc-ing stable as the commit is still in tty-next, and even if the
errors originate from original commit that added I2C support, they were not
trigerred because the driver could not be selected/compiled if
CONFIG_SPI_MASTER was disabled.

Changes for v2:
- replace #ifdef with #if IS_ENABLED() to suppoirt both built-in and modules
  options
---
 drivers/tty/serial/max310x.c | 48 +++++++++++++++++++-----------------
 1 file changed, 26 insertions(+), 22 deletions(-)

diff --git a/drivers/tty/serial/max310x.c b/drivers/tty/serial/max310x.c
index 9f423b3b4201d..bad5329a0c84c 100644
--- a/drivers/tty/serial/max310x.c
+++ b/drivers/tty/serial/max310x.c
@@ -16,6 +16,7 @@
 #include <linux/device.h>
 #include <linux/gpio/driver.h>
 #include <linux/i2c.h>
+#include <linux/kconfig.h>
 #include <linux/module.h>
 #include <linux/mod_devicetable.h>
 #include <linux/property.h>
@@ -1507,6 +1508,21 @@ static const struct of_device_id __maybe_unused max310x_dt_ids[] = {
 };
 MODULE_DEVICE_TABLE(of, max310x_dt_ids);
 
+static const char *max310x_regmap_name(u8 port_id)
+{
+	switch (port_id) {
+	case 0:	return "port0";
+	case 1:	return "port1";
+	case 2:	return "port2";
+	case 3:	return "port3";
+	default:
+		WARN_ON(true);
+		return NULL;
+	}
+}
+
+#if IS_ENABLED(CONFIG_SPI_MASTER)
+
 static struct regmap_config regcfg = {
 	.reg_bits = 8,
 	.val_bits = 8,
@@ -1522,20 +1538,6 @@ static struct regmap_config regcfg = {
 	.max_raw_write = MAX310X_FIFO_SIZE,
 };
 
-static const char *max310x_regmap_name(u8 port_id)
-{
-	switch (port_id) {
-	case 0:	return "port0";
-	case 1:	return "port1";
-	case 2:	return "port2";
-	case 3:	return "port3";
-	default:
-		WARN_ON(true);
-		return NULL;
-	}
-}
-
-#ifdef CONFIG_SPI_MASTER
 static int max310x_spi_extended_reg_enable(struct device *dev, bool enable)
 {
 	struct max310x_port *s = dev_get_drvdata(dev);
@@ -1606,7 +1608,8 @@ static struct spi_driver max310x_spi_driver = {
 };
 #endif
 
-#ifdef CONFIG_I2C
+#if IS_ENABLED(CONFIG_I2C)
+
 static int max310x_i2c_extended_reg_enable(struct device *dev, bool enable)
 {
 	return 0;
@@ -1726,13 +1729,13 @@ static int __init max310x_uart_init(void)
 	if (ret)
 		return ret;
 
-#ifdef CONFIG_SPI_MASTER
+#if IS_ENABLED(CONFIG_SPI_MASTER)
 	ret = spi_register_driver(&max310x_spi_driver);
 	if (ret)
 		goto err_spi_register;
 #endif
 
-#ifdef CONFIG_I2C
+#if IS_ENABLED(CONFIG_I2C)
 	ret = i2c_add_driver(&max310x_i2c_driver);
 	if (ret)
 		goto err_i2c_register;
@@ -1740,12 +1743,13 @@ static int __init max310x_uart_init(void)
 
 	return 0;
 
-#ifdef CONFIG_I2C
+#if IS_ENABLED(CONFIG_I2C)
 err_i2c_register:
-	spi_unregister_driver(&max310x_spi_driver);
 #endif
-
+#if IS_ENABLED(CONFIG_SPI_MASTER)
+	spi_unregister_driver(&max310x_spi_driver);
 err_spi_register:
+#endif
 	uart_unregister_driver(&max310x_uart);
 
 	return ret;
@@ -1754,11 +1758,11 @@ module_init(max310x_uart_init);
 
 static void __exit max310x_uart_exit(void)
 {
-#ifdef CONFIG_I2C
+#if IS_ENABLED(CONFIG_I2C)
 	i2c_del_driver(&max310x_i2c_driver);
 #endif
 
-#ifdef CONFIG_SPI_MASTER
+#if IS_ENABLED(CONFIG_SPI_MASTER)
 	spi_unregister_driver(&max310x_spi_driver);
 #endif
 

base-commit: 16e95bfb79b5d9d01dc7651d98caf3c2ace331cd
-- 
2.47.3


^ permalink raw reply related

* Re: [PATCH v4] Bluetooth: hci_uart: fix UAF in hci_uart_tty_close()
From: Luiz Augusto von Dentz @ 2026-05-15 16:08 UTC (permalink / raw)
  To: w15303746062
  Cc: pmenzel, marcel, linux-bluetooth, linux-serial, linux-kernel,
	greg, stable, Mingyu Wang
In-Reply-To: <20260515140548.393865-1-w15303746062@163.com>

Hi,

On Fri, May 15, 2026 at 10:06 AM <w15303746062@163.com> wrote:
>
> From: Mingyu Wang <25181214217@stu.xidian.edu.cn>
>
> A Use-After-Free (UAF) vulnerability and a subsequent kernel panic were
> observed in hci_uart_write_work() due to a race condition between the
> initialization of the HCI UART line discipline and concurrent TTY hangup.
>
> This issue was triggered by our custom device emulation and fuzzing
> framework (DevGen) on the v6.18 kernel. Due to the highly timing-dependent
> nature of this race condition (requiring a precise interleaving of
> TIOCVHANGUP and protocol setup), Syzkaller failed to extract a reliable
> standalone C reproducer (reproducer is too unreliable: 0.00).
>
> The crash trace is as follows:
>   ODEBUG: free active (active state 0) object: ffff88804024e870 object type: work_struct hint: hci_uart_write_work+0x0/0x940
>   WARNING: CPU: 0 PID: 338273 at lib/debugobjects.c:612 debug_print_object+0x1a2/0x2b0
>   ...
>   Call Trace:
>    <TASK>
>    debug_check_no_obj_freed+0x3ec/0x520
>    kfree+0x3f0/0x6c0
>    hci_uart_tty_close+0x127/0x2a0
>    tty_ldisc_close+0x113/0x1a0
>    tty_ldisc_kill+0x8e/0x150
>    tty_ldisc_hangup+0x3c1/0x730
>    __tty_hangup.part.0+0x3fd/0x8a0
>    tty_ioctl+0x120f/0x1690
>    __x64_sys_ioctl+0x18f/0x210
>    do_syscall_64+0xcb/0xfa0
>    entry_SYSCALL_64_after_hwframe+0x77/0x7f
>    </TASK>
>
> The issue arises because the workqueues (init_ready and write_work) are
> only flushed/cancelled if the HCI_UART_PROTO_READY flag is set. However,
> during the protocol initialization phase (HCI_UART_PROTO_INIT), the
> underlying protocol may schedule work. If a hangup occurs before the setup
> completes and the READY flag is set, hci_uart_tty_close() skips the
> teardown of these workqueues and proceeds to free the `hu` struct. When
> the scheduled work executes later, it blindly dereferences the freed `hu`
> struct.
>
> Fix this by moving the workqueue teardown outside the HCI_UART_PROTO_READY
> check. Furthermore, use disable_work_sync() instead of cancel_work_sync()
> to unconditionally disable the works. This ensures that any pending works
> are cancelled and no new submissions can occur before the hci_uart
> structure is freed. Note that hu->init_ready and hu->write_work are
> initialized in hci_uart_tty_open(), so it is always safe to call
> disable_work_sync() on them in hci_uart_tty_close(), even if the protocol
> was never fully attached.
>
> Fixes: 3b799254cf6f ("Bluetooth: hci_uart: Cancel init work before unregistering")
> Cc: stable@vger.kernel.org
> Signed-off-by: Mingyu Wang <25181214217@stu.xidian.edu.cn>
> ---
> Changes in v4:
> - Adopted Luiz's suggestion to use disable_work_sync() instead of
>   cancel_work_sync() to prevent new work submissions during teardown.
>
> Changes in v3:
> - Added 'Cc: stable' tag as requested by the stable bot.
>
> Changes in v2:
> - Added KASAN/ODEBUG crash trace.
>
>  drivers/bluetooth/hci_ldisc.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
> index 275ea865bc29..333c1e1503e8 100644
> --- a/drivers/bluetooth/hci_ldisc.c
> +++ b/drivers/bluetooth/hci_ldisc.c
> @@ -544,14 +544,20 @@ static void hci_uart_tty_close(struct tty_struct *tty)
>         if (hdev)
>                 hci_uart_close(hdev);
>
> +       /*
> +        * Disable workqueues unconditionally before freeing the hu
> +        * struct, as they might be active during the PROTO_INIT phase.
> +        * Using disable_work_sync() instead of cancel_work_sync()
> +        * ensures no new submissions can occur.
> +        */
> +       disable_work_sync(&hu->init_ready);
> +       disable_work_sync(&hu->write_work);

Looks like sashiko has a problem with these being after hci_uart_close:

https://sashiko.dev/#/patchset/20260515140548.393865-1-w15303746062%40163.com

>         if (test_bit(HCI_UART_PROTO_READY, &hu->flags)) {
>                 percpu_down_write(&hu->proto_lock);
>                 clear_bit(HCI_UART_PROTO_READY, &hu->flags);
>                 percpu_up_write(&hu->proto_lock);
>
> -               cancel_work_sync(&hu->init_ready);
> -               cancel_work_sync(&hu->write_work);
> -
>                 if (hdev) {
>                         if (test_bit(HCI_UART_REGISTERED, &hu->flags))
>                                 hci_unregister_dev(hdev);
> --
> 2.34.1
>


-- 
Luiz Augusto von Dentz

^ permalink raw reply

* [PATCH v4] Bluetooth: hci_uart: fix UAF in hci_uart_tty_close()
From: w15303746062 @ 2026-05-15 14:05 UTC (permalink / raw)
  To: luiz.dentz, pmenzel, marcel, linux-bluetooth
  Cc: linux-serial, linux-kernel, greg, stable, Mingyu Wang
In-Reply-To: <CABBYNZLjreYY_BczAQr2G6L=iJjBYKksFp53CairG-6V0Cb0EA@mail.gmail.com>

From: Mingyu Wang <25181214217@stu.xidian.edu.cn>

A Use-After-Free (UAF) vulnerability and a subsequent kernel panic were
observed in hci_uart_write_work() due to a race condition between the
initialization of the HCI UART line discipline and concurrent TTY hangup.

This issue was triggered by our custom device emulation and fuzzing
framework (DevGen) on the v6.18 kernel. Due to the highly timing-dependent
nature of this race condition (requiring a precise interleaving of
TIOCVHANGUP and protocol setup), Syzkaller failed to extract a reliable
standalone C reproducer (reproducer is too unreliable: 0.00).

The crash trace is as follows:
  ODEBUG: free active (active state 0) object: ffff88804024e870 object type: work_struct hint: hci_uart_write_work+0x0/0x940
  WARNING: CPU: 0 PID: 338273 at lib/debugobjects.c:612 debug_print_object+0x1a2/0x2b0
  ...
  Call Trace:
   <TASK>
   debug_check_no_obj_freed+0x3ec/0x520
   kfree+0x3f0/0x6c0
   hci_uart_tty_close+0x127/0x2a0
   tty_ldisc_close+0x113/0x1a0
   tty_ldisc_kill+0x8e/0x150
   tty_ldisc_hangup+0x3c1/0x730
   __tty_hangup.part.0+0x3fd/0x8a0
   tty_ioctl+0x120f/0x1690
   __x64_sys_ioctl+0x18f/0x210
   do_syscall_64+0xcb/0xfa0
   entry_SYSCALL_64_after_hwframe+0x77/0x7f
   </TASK>

The issue arises because the workqueues (init_ready and write_work) are
only flushed/cancelled if the HCI_UART_PROTO_READY flag is set. However,
during the protocol initialization phase (HCI_UART_PROTO_INIT), the
underlying protocol may schedule work. If a hangup occurs before the setup
completes and the READY flag is set, hci_uart_tty_close() skips the
teardown of these workqueues and proceeds to free the `hu` struct. When
the scheduled work executes later, it blindly dereferences the freed `hu`
struct.

Fix this by moving the workqueue teardown outside the HCI_UART_PROTO_READY
check. Furthermore, use disable_work_sync() instead of cancel_work_sync()
to unconditionally disable the works. This ensures that any pending works
are cancelled and no new submissions can occur before the hci_uart
structure is freed. Note that hu->init_ready and hu->write_work are
initialized in hci_uart_tty_open(), so it is always safe to call
disable_work_sync() on them in hci_uart_tty_close(), even if the protocol
was never fully attached.

Fixes: 3b799254cf6f ("Bluetooth: hci_uart: Cancel init work before unregistering")
Cc: stable@vger.kernel.org
Signed-off-by: Mingyu Wang <25181214217@stu.xidian.edu.cn>
---
Changes in v4:
- Adopted Luiz's suggestion to use disable_work_sync() instead of 
  cancel_work_sync() to prevent new work submissions during teardown.

Changes in v3:
- Added 'Cc: stable' tag as requested by the stable bot.

Changes in v2:
- Added KASAN/ODEBUG crash trace.

 drivers/bluetooth/hci_ldisc.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
index 275ea865bc29..333c1e1503e8 100644
--- a/drivers/bluetooth/hci_ldisc.c
+++ b/drivers/bluetooth/hci_ldisc.c
@@ -544,14 +544,20 @@ static void hci_uart_tty_close(struct tty_struct *tty)
 	if (hdev)
 		hci_uart_close(hdev);
 
+	/*
+	 * Disable workqueues unconditionally before freeing the hu
+	 * struct, as they might be active during the PROTO_INIT phase.
+	 * Using disable_work_sync() instead of cancel_work_sync()
+	 * ensures no new submissions can occur.
+	 */
+	disable_work_sync(&hu->init_ready);
+	disable_work_sync(&hu->write_work);
+
 	if (test_bit(HCI_UART_PROTO_READY, &hu->flags)) {
 		percpu_down_write(&hu->proto_lock);
 		clear_bit(HCI_UART_PROTO_READY, &hu->flags);
 		percpu_up_write(&hu->proto_lock);
 
-		cancel_work_sync(&hu->init_ready);
-		cancel_work_sync(&hu->write_work);
-
 		if (hdev) {
 			if (test_bit(HCI_UART_REGISTERED, &hu->flags))
 				hci_unregister_dev(hdev);
-- 
2.34.1


^ permalink raw reply related

* Re:Re: [PATCH] Bluetooth: hci_uart: fix UAF in hci_uart_tty_close()
From: w15303746062 @ 2026-05-15 13:39 UTC (permalink / raw)
  To: Luiz Augusto von Dentz
  Cc: marcel, linux-bluetooth, linux-serial, linux-kernel, Mingyu Wang
In-Reply-To: <CABBYNZLjreYY_BczAQr2G6L=iJjBYKksFp53CairG-6V0Cb0EA@mail.gmail.com>


Hi Luiz,

Thank you for the review.

That is an excellent suggestion. You are absolutely right. Since the
`hu` structure is being torn down and freed immediately afterward, 
using `disable_work_sync()` provides a much stronger guarantee by 
preventing any concurrent threads from re-queuing the works, thus 
eliminating the risk of a lingering UAF.

Both `init_ready` and `write_work` are standard `struct work_struct`,
so `disable_work_sync()` applies perfectly here.

I will send out a v4 patch shortly adopting this change. 
Thank you for pointing this out!

Best regards,
Mingyu


At 2026-05-15 20:37:57, "Luiz Augusto von Dentz" <luiz.dentz@gmail.com> wrote:
>Hi,
>
>On Wed, May 13, 2026 at 2:46 AM <w15303746062@163.com> wrote:
>>
>> From: Mingyu Wang <25181214217@stu.xidian.edu.cn>
>>
>> A Use-After-Free (UAF) vulnerability and a subsequent General Protection
>> Fault (GPF) were observed in h5_recv() due to a race condition between
>> the initialization of the HCI UART line discipline and concurrent TTY
>> hangup via TIOCVHANGUP.
>>
>> The issue arises because the workqueues (init_ready and write_work) are
>> only cancelled if the HCI_UART_PROTO_READY flag is set. However, during
>> the protocol initialization phase (HCI_UART_PROTO_INIT), the underlying
>> protocol (e.g., H5) may schedule work (such as sending sync/config
>> packets). If a hangup occurs before the setup completes and the READY
>> flag is set, hci_uart_tty_close() skips the cancel_work_sync() calls
>> and proceeds to free the `hu` struct.
>>
>> When the delayed workqueue finally executes, it blindly dereferences
>> the freed `hu` struct, causing ODEBUG warnings and kernel panics.
>>
>> Fix this by moving the cancel_work_sync() calls outside the
>> HCI_UART_PROTO_READY check, ensuring that any pending works are
>> unconditionally cancelled before the hci_uart structure is freed.
>>
>> Signed-off-by: Mingyu Wang <25181214217@stu.xidian.edu.cn>
>> ---
>>  drivers/bluetooth/hci_ldisc.c | 10 +++++++---
>>  1 file changed, 7 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
>> index 275ea865bc29..566e1c525ee2 100644
>> --- a/drivers/bluetooth/hci_ldisc.c
>> +++ b/drivers/bluetooth/hci_ldisc.c
>> @@ -544,14 +544,18 @@ static void hci_uart_tty_close(struct tty_struct *tty)
>>         if (hdev)
>>                 hci_uart_close(hdev);
>>
>> +       /*
>> +        * Always cancel workqueues unconditionally before freeing the hu
>> +        * struct, as they might be active during the PROTO_INIT phase.
>> +        */
>> +       cancel_work_sync(&hu->init_ready);
>> +       cancel_work_sync(&hu->write_work);
>
>Can't we use disable_work_sync? If it frees up at the end, it's
>probably best to disable it so it doesn't allow new submissions.
>
>>         if (test_bit(HCI_UART_PROTO_READY, &hu->flags)) {
>>                 percpu_down_write(&hu->proto_lock);
>>                 clear_bit(HCI_UART_PROTO_READY, &hu->flags);
>>                 percpu_up_write(&hu->proto_lock);
>>
>> -               cancel_work_sync(&hu->init_ready);
>> -               cancel_work_sync(&hu->write_work);
>> -
>>                 if (hdev) {
>>                         if (test_bit(HCI_UART_REGISTERED, &hu->flags))
>>                                 hci_unregister_dev(hdev);
>> --
>> 2.34.1
>>
>>
>
>
>-- 
>Luiz Augusto von Dentz

^ permalink raw reply

* [PATCH] tty: serial: samsung: Remove redundant port lock acquisition in rx helpers
From: Tudor Ambarus @ 2026-05-15 12:41 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Alim Akhtar, Greg Kroah-Hartman, Jiri Slaby,
	Ben Dooks
  Cc: linux-arm-kernel, linux-samsung-soc, linux-kernel, linux-serial,
	john.ogness, peter.griffin, andre.draszik, jyescas, kernel-team,
	stable, John Ogness, Tudor Ambarus

Sashiko identified a deadlock when the console flow is engaged [1].

When console flow control is enabled (UPF_CONS_FLOW),
s3c24xx_serial_stop_tx() calls s3c24xx_serial_rx_enable() and
s3c24xx_serial_start_tx() calls s3c24xx_serial_rx_disable().

The serial core framework invokes the .stop_tx() and .start_tx()
callbacks with the port->lock spinlock already held. Furthermore, all
internal driver paths that invoke stop_tx (such as the DMA TX
completion handler s3c24xx_serial_tx_dma_complete() or the PIO TX IRQ
handler s3c24xx_serial_tx_irq()) also acquire port->lock prior to
calling it. (Note that s3c24xx_serial_start_tx() is only invoked by the
serial core).

However, s3c24xx_serial_rx_enable() and s3c24xx_serial_rx_disable()
unconditionally attempt to acquire port->lock again using
uart_port_lock_irqsave(). Since spinlocks are not recursive, this
causes a deadlock on the same CPU when console flow control is engaged.

Remove the redundant lock acquisition from both rx helper functions.

Cc: stable@vger.kernel.org
Fixes: b497549a035c ("[ARM] S3C24XX: Split serial driver into core and per-cpu drivers")
Reported-by: John Ogness <john.ogness@linutronix.de>
Closes: https://sashiko.dev/#/patchset/20260506121606.5805-1-john.ogness%40linutronix.de [1]
Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org>
---
 drivers/tty/serial/samsung_tty.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/drivers/tty/serial/samsung_tty.c b/drivers/tty/serial/samsung_tty.c
index 2f94fc798cff..63d0232dffc2 100644
--- a/drivers/tty/serial/samsung_tty.c
+++ b/drivers/tty/serial/samsung_tty.c
@@ -245,12 +245,9 @@ static bool s3c24xx_serial_txempty_nofifo(const struct uart_port *port)
 static void s3c24xx_serial_rx_enable(struct uart_port *port)
 {
 	struct s3c24xx_uart_port *ourport = to_ourport(port);
-	unsigned long flags;
 	int count = 10000;
 	u32 ucon, ufcon;
 
-	uart_port_lock_irqsave(port, &flags);
-
 	while (--count && !s3c24xx_serial_txempty_nofifo(port))
 		udelay(100);
 
@@ -263,23 +260,18 @@ static void s3c24xx_serial_rx_enable(struct uart_port *port)
 	wr_regl(port, S3C2410_UCON, ucon);
 
 	ourport->rx_enabled = 1;
-	uart_port_unlock_irqrestore(port, flags);
 }
 
 static void s3c24xx_serial_rx_disable(struct uart_port *port)
 {
 	struct s3c24xx_uart_port *ourport = to_ourport(port);
-	unsigned long flags;
 	u32 ucon;
 
-	uart_port_lock_irqsave(port, &flags);
-
 	ucon = rd_regl(port, S3C2410_UCON);
 	ucon &= ~S3C2410_UCON_RXIRQMODE;
 	wr_regl(port, S3C2410_UCON, ucon);
 
 	ourport->rx_enabled = 0;
-	uart_port_unlock_irqrestore(port, flags);
 }
 
 static void s3c24xx_serial_stop_tx(struct uart_port *port)

---
base-commit: 16e95bfb79b5d9d01dc7651d98caf3c2ace331cd
change-id: 20260515-samsung-tty-flow-control-deadlock-1d426171bf41

Best regards,
-- 
Tudor Ambarus <tudor.ambarus@linaro.org>


^ permalink raw reply related

* Re: [PATCH] Bluetooth: hci_uart: fix UAF in hci_uart_tty_close()
From: Luiz Augusto von Dentz @ 2026-05-15 12:37 UTC (permalink / raw)
  To: w15303746062
  Cc: marcel, linux-bluetooth, linux-serial, linux-kernel, Mingyu Wang
In-Reply-To: <20260513064547.352601-1-w15303746062@163.com>

Hi,

On Wed, May 13, 2026 at 2:46 AM <w15303746062@163.com> wrote:
>
> From: Mingyu Wang <25181214217@stu.xidian.edu.cn>
>
> A Use-After-Free (UAF) vulnerability and a subsequent General Protection
> Fault (GPF) were observed in h5_recv() due to a race condition between
> the initialization of the HCI UART line discipline and concurrent TTY
> hangup via TIOCVHANGUP.
>
> The issue arises because the workqueues (init_ready and write_work) are
> only cancelled if the HCI_UART_PROTO_READY flag is set. However, during
> the protocol initialization phase (HCI_UART_PROTO_INIT), the underlying
> protocol (e.g., H5) may schedule work (such as sending sync/config
> packets). If a hangup occurs before the setup completes and the READY
> flag is set, hci_uart_tty_close() skips the cancel_work_sync() calls
> and proceeds to free the `hu` struct.
>
> When the delayed workqueue finally executes, it blindly dereferences
> the freed `hu` struct, causing ODEBUG warnings and kernel panics.
>
> Fix this by moving the cancel_work_sync() calls outside the
> HCI_UART_PROTO_READY check, ensuring that any pending works are
> unconditionally cancelled before the hci_uart structure is freed.
>
> Signed-off-by: Mingyu Wang <25181214217@stu.xidian.edu.cn>
> ---
>  drivers/bluetooth/hci_ldisc.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
> index 275ea865bc29..566e1c525ee2 100644
> --- a/drivers/bluetooth/hci_ldisc.c
> +++ b/drivers/bluetooth/hci_ldisc.c
> @@ -544,14 +544,18 @@ static void hci_uart_tty_close(struct tty_struct *tty)
>         if (hdev)
>                 hci_uart_close(hdev);
>
> +       /*
> +        * Always cancel workqueues unconditionally before freeing the hu
> +        * struct, as they might be active during the PROTO_INIT phase.
> +        */
> +       cancel_work_sync(&hu->init_ready);
> +       cancel_work_sync(&hu->write_work);

Can't we use disable_work_sync? If it frees up at the end, it's
probably best to disable it so it doesn't allow new submissions.

>         if (test_bit(HCI_UART_PROTO_READY, &hu->flags)) {
>                 percpu_down_write(&hu->proto_lock);
>                 clear_bit(HCI_UART_PROTO_READY, &hu->flags);
>                 percpu_up_write(&hu->proto_lock);
>
> -               cancel_work_sync(&hu->init_ready);
> -               cancel_work_sync(&hu->write_work);
> -
>                 if (hdev) {
>                         if (test_bit(HCI_UART_REGISTERED, &hu->flags))
>                                 hci_unregister_dev(hdev);
> --
> 2.34.1
>
>


-- 
Luiz Augusto von Dentz

^ permalink raw reply

* Re: [PATCH 0/2] serial: 8250_dw: clock-notifier cleanup
From: Andy Shevchenko @ 2026-05-15 10:55 UTC (permalink / raw)
  To: Stepan Ionichev
  Cc: ilpo.jarvinen, gregkh, jirislaby, linux-serial, linux-kernel,
	stable
In-Reply-To: <20260514143746.23671-1-sozdayvek@gmail.com>

On Thu, May 14, 2026 at 07:37:44PM +0500, Stepan Ionichev wrote:
> Two-patch series addressing Andy's review of the leak-fix on v1.
> 
> Patch 1 keeps the same single-line leak fix as v1, but with:
> - the correct "serial: 8250_dw:" prefix (underscore),
> - a Fixes: tag pointing at the original clk_notifier introduction,
> - Cc: stable@ so the fix gets picked up by stable branches that
>   still carry the notifier code.
> 
> Patch 2 drops the clock-notifier infrastructure entirely from
> mainline, as suggested by Andy. The notifier was introduced for the
> Baikal-T1 SoC (shared baudclk between UART ports) and has no other
> in-tree user; Baikal-T1 support has been removed from the kernel.
> 
> If a future platform needs the cross-device baudclk-rate notification
> pattern again, it can be reintroduced in a more general form.

Seems legit, especially the second patch.
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>


-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply

* Re: [PATCH tty v4 2/6] serial: Replace driver usage of UPF_CONS_FLOW
From: Tudor Ambarus @ 2026-05-15 10:21 UTC (permalink / raw)
  To: John Ogness, Krzysztof Kozlowski, Greg Kroah-Hartman, Jiri Slaby,
	Alim Akhtar
  Cc: Andy Shevchenko, linux-kernel, David S. Miller,
	Ilpo Järvinen, Andy Shevchenko, Thomas Fourier, Kees Cook,
	linux-serial, linux-arm-kernel, linux-samsung-soc, sparclinux,
	Peter Griffin, André Draszik, Alexey Klimov, Juan Yescas
In-Reply-To: <87wlx56rcc.fsf@jogness.linutronix.de>



On 5/15/26 10:53 AM, John Ogness wrote:
> On 2026-05-13, Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>> (This email is particularly directed at the Samsung folks.)
>>>
>>> Responding to Sashiko:
>>>
>>> https://sashiko.dev/#/patchset/20260506121606.5805-1-john.ogness%40linutronix.de
>>>
>>> On 2026-05-06, Sashiko wrote:
>>>>> diff --git a/drivers/tty/serial/samsung_tty.c b/drivers/tty/serial/samsung_tty.c
>>>>> index e27806bf2cf3e..2f94fc798cffb 100644
>>>>> --- a/drivers/tty/serial/samsung_tty.c
>>>>> +++ b/drivers/tty/serial/samsung_tty.c
>>>>> @@ -319,7 +319,7 @@ static void s3c24xx_serial_stop_tx(struct uart_port *port)
>>>>>  	ourport->tx_enabled = 0;
>>>>>  	ourport->tx_in_progress = 0;
>>>>>  
>>>>> -	if (port->flags & UPF_CONS_FLOW)
>>>>> +	if (uart_cons_flow_enabled(port))
>>>>>  		s3c24xx_serial_rx_enable(port);
>>>>
>>>> This isn't a new bug introduced by this patch, but does this code
>>>> result in a recursive spinlock acquisition and deadlock when console
>>>> flow control is enabled?
>>>>
>>>> The serial core framework calls port->ops->stop_tx() with the
>>>> port->lock spinlock already held. When uart_cons_flow_enabled(port)
>>>> evaluates to true, s3c24xx_serial_stop_tx() invokes
>>>> s3c24xx_serial_rx_enable().
>>>>
>>>> Looking at s3c24xx_serial_rx_enable(), it unconditionally attempts to
>>>> acquire the exact same lock again:
>>>>
>>>> static void s3c24xx_serial_rx_enable(struct uart_port *port)
>>>> {
>>>> 	struct s3c24xx_uart_port *ourport = to_ourport(port);
>>>> 	unsigned long flags;
>>>> 	...
>>>> 	uart_port_lock_irqsave(port, &flags);
>>>> 	...
>>>> }
>>>>
>>>> Because spinlocks are not recursive in the kernel, it appears this
>>>> sequence would cause an immediate deadlock on the same CPU.
>>>
>>> Although out of scope for my series, the reported issue is real. Perhaps
>>> something for Krzysztof or Alim to look into.
>>>
>>>>> @@ -493,7 +493,7 @@ static void s3c24xx_serial_start_tx(struct uart_port *port)
>>>>>  	struct tty_port *tport = &port->state->port;
>>>>>  
>>>>>  	if (!ourport->tx_enabled) {
>>>>> -		if (port->flags & UPF_CONS_FLOW)
>>>>> +		if (uart_cons_flow_enabled(port))
>>>>>  			s3c24xx_serial_rx_disable(port);
>>>>
>>>> Similarly here, s3c24xx_serial_start_tx() is also called with
>>>> port->lock held, and s3c24xx_serial_rx_disable() will attempt to
>>>> acquire port->lock again:
>>>>
>>>> static void s3c24xx_serial_rx_disable(struct uart_port *port)
>>>> {
>>>> 	struct s3c24xx_uart_port *ourport = to_ourport(port);
>>>> 	unsigned long flags;
>>>> 	...
>>>> 	uart_port_lock_irqsave(port, &flags);
>>>> 	...
>>>> }
>>>>
>>>> Could this pre-existing locking issue in the samsung_tty driver be
>>>> addressed so that the rx enable/disable helpers do not try to take the
>>>> port lock when it is already held by the caller?
>>>
>>> Also legitimate. But out of scope for my series.
>>
>>
>> Thanks for letting us know. Deadlock did not happen so far, so something
>> is missing in Sashiko's report. :)
> 
> Nothing is missing. I am guessing you never use console flow
> control. The deadlock is clearly visible:
> 
> ->stop_tx() (always called with the port locked)
>   s3c24xx_serial_stop_tx()
>     s3c24xx_serial_rx_enable()
>       uart_port_lock_irqsave() (DEADLOCK!)
> 

Right.

The lock acquisitions in the rx helper functions are redundant and shall be
removed.

The serial core framework invokes the .stop_tx() and .start_tx() callbacks
with the port->lock spinlock already held. Furthermore, all internal driver
paths that invoke stop_tx/start_tx also acquire port->lock prior to calling
them.
    
However, s3c24xx_serial_rx_enable() and s3c24xx_serial_rx_disable()
unconditionally attempt to acquire port->lock again using
uart_port_lock_irqsave(). Since kernel spinlocks are not recursive, this
causes a deadlock on the same CPU when console flow control is engaged.

Just removing the redundant lock acquisitions shall fix it. I'll prepare
a patch.

diff --git a/drivers/tty/serial/samsung_tty.c b/drivers/tty/serial/samsung_tty.c
index e27806bf2cf3..17cd5bb100b1 100644
--- a/drivers/tty/serial/samsung_tty.c
+++ b/drivers/tty/serial/samsung_tty.c
+++ b/drivers/tty/serial/samsung_tty.c
@@ -245,12 +245,9 @@ static bool s3c24xx_serial_txempty_nofifo(const struct uart_port *port)
 static void s3c24xx_serial_rx_enable(struct uart_port *port)
 {
        struct s3c24xx_uart_port *ourport = to_ourport(port);
-       unsigned long flags;
        int count = 10000;
        u32 ucon, ufcon;
 
-       uart_port_lock_irqsave(port, &flags);
-
        while (--count && !s3c24xx_serial_txempty_nofifo(port))
                udelay(100);
 
@@ -263,23 +260,18 @@ static void s3c24xx_serial_rx_enable(struct uart_port *port)
        wr_regl(port, S3C2410_UCON, ucon);
 
        ourport->rx_enabled = 1;
-       uart_port_unlock_irqrestore(port, flags);
 }
 
 static void s3c24xx_serial_rx_disable(struct uart_port *port)
 {
        struct s3c24xx_uart_port *ourport = to_ourport(port);
-       unsigned long flags;
        u32 ucon;
 
-       uart_port_lock_irqsave(port, &flags);
-
        ucon = rd_regl(port, S3C2410_UCON);
        ucon &= ~S3C2410_UCON_RXIRQMODE;
        wr_regl(port, S3C2410_UCON, ucon);
 
        ourport->rx_enabled = 0;
-       uart_port_unlock_irqrestore(port, flags);
 }
 
 static void s3c24xx_serial_stop_tx(struct uart_port *port)

^ permalink raw reply


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