* Re: [PATCH tty] tty: n_gsm: fix use-after-free in gsm_queue vs gsm_cleanup_mux race
From: Zhenghang Xiao @ 2026-05-29 6:28 UTC (permalink / raw)
To: Greg KH; +Cc: jirislaby, linux-serial
In-Reply-To: <2026052743-probation-anything-aa72@gregkh>
Resending this email as plain text, since the previous reply was
rejected by the mailing list for containing an HTML part.
-------------------
Thanks for your reply!
> How to test
I built two identical arm64 kernels (v7.1.0-rc4), differing only by
this patch. Both kernel carry a msleep(20) instrumentation in
gsm_queue() to widen the race window. Then run PoC and the patched
kernel do not report kASAN again.
> What prevents dead from changing
Nothing prevents it and it doesn't need to. tty_ldisc_flush() is the
sync mechanism not dead check.
If gsmld_receive_buf() is already past the check, it's running under
buf->lock. The patch moves tty_ldisc_flush() before DLCI release. And
tty_buffer_flush() acquires the same buf->lock, so it blocks until the
in-flight receive completes. DLCIs are freed only after that.
The dead check handles the other direction, any receive that starts
after the flush sees dead == true and returns early. But I'm not sure
if silent drop here is fine or not.
Thanks!
Zhenghang
Greg KH <gregkh@linuxfoundation.org> 于2026年5月27日周三 16:17写道：
>
> On Tue, May 26, 2026 at 06:29:24PM +0800, Zhenghang Xiao wrote:
> > gsm_queue() reads gsm->dlci[address] into a local pointer in the
> > flush_to_ldisc workqueue without any lock. Concurrently,
> > gsm_cleanup_mux() (triggered by GSMIOC_SETCONF ioctl) frees DLCIs under
> > gsm->mutex — which the receive path never holds. The cached pointer in
> > gsm_queue() becomes dangling, and the subsequent dlci->data() call
> > dereferences freed memory.
> >
> > Fix this by:
> > 1. Checking gsm->dead at the start of gsmld_receive_buf() to reject
> > frame processing after cleanup has begun.
> > 2. Moving tty_ldisc_flush() before the DLCI release loop in
> > gsm_cleanup_mux(). tty_ldisc_flush() acquires the tty buffer lock
> > (buf->lock), which serializes against any in-flight flush_to_ldisc
> > work. After it returns, in-flight receive processing has completed,
> > and subsequent calls see gsm->dead and return early.
> >
> > Fixes: e1eaea46bb40 ("tty: n_gsm line discipline")
> > Signed-off-by: Zhenghang Xiao <kipreyyy@gmail.com>
> > ---
> > drivers/tty/n_gsm.c | 13 +++++++++++--
> > 1 file changed, 11 insertions(+), 2 deletions(-)
>
> Cool, how did you test this?
>
>
> >
> > diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
> > index c13e050de83b..8322fffbaeba 100644
> > --- a/drivers/tty/n_gsm.c
> > +++ b/drivers/tty/n_gsm.c
> > @@ -3156,12 +3156,18 @@ static void gsm_cleanup_mux(struct gsm_mux *gsm, bool disc)
> > gsm_unregister_devices(gsm_tty_driver, gsm->num);
> > gsm->has_devices = false;
> > }
> > + /*
> > + * Flush the ldisc before releasing DLCIs. tty_ldisc_flush() waits
> > + * for any in-flight flush_to_ldisc work to complete via buf->lock,
> > + * and the gsm->dead check added to gsmld_receive_buf() rejects any
> > + * future receive processing. This ensures gsm_queue() cannot access
> > + * a DLCI being freed.
> > + */
> > + tty_ldisc_flush(gsm->tty);
> > for (i = NUM_DLCI - 1; i >= 0; i--)
> > if (gsm->dlci[i])
> > gsm_dlci_release(gsm->dlci[i]);
> > mutex_unlock(&gsm->mutex);
> > - /* Now wipe the queues */
> > - tty_ldisc_flush(gsm->tty);
> >
> > guard(spinlock_irqsave)(&gsm->tx_lock);
> > list_for_each_entry_safe(txq, ntxq, &gsm->tx_ctrl_list, list)
> > @@ -3604,6 +3610,9 @@ static void gsmld_receive_buf(struct tty_struct *tty, const u8 *cp,
> > struct gsm_mux *gsm = tty->disc_data;
> > u8 flags = TTY_NORMAL;
> >
> > + if (gsm->dead)
> > + return;
> > +
>
> What prevents dead from changing right after you test this?
>
> thanks,
>
> greg k-h
^ permalink raw reply
* [PATCH v3] serial: 8250: fix use-after-free in IRQ chain handling
From: Qiliang Yuan @ 2026-05-29 4:36 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby, Anton Vorontsov, Alan Cox
Cc: linux-kernel, linux-serial, Wang Zhaolong, Qiliang Yuan
serial_unlink_irq_chain() holds hash_mutex and calls free_irq() + kfree(i)
when it sees an empty port list. serial_link_irq_chain() released
hash_mutex after serial_get_or_create_irq_info() but before acquiring
i->lock. This gap allowed a concurrent unlink to observe list_empty()
as true while a new port was still being added, free i, and trigger a
use-after-free.
Dropping hash_mutex before request_irq() completes also allows another
port sharing the same IRQ to join the chain and run the shared-IRQ THRE
test while IRQ startup is still in progress, which can also trigger the
"Unbalanced enable for IRQ" warning (kernel/irq/manage.c:774) because
irq_shutdown() in the premature free_irq() path hard-sets desc->depth
to 1, breaking the disable_irq/enable_irq pairing in
serial8250_THRE_test().
Fix by pulling hash_mutex into serial_link_irq_chain() and holding it
across the first request_irq() completion so that no concurrent
unlink or second-port join can race with IRQ setup.
serial_unlink_irq_chain() already holds hash_mutex throughout, so the
race window is closed.
Fixes: 768aec0b5bcc ("serial: 8250: fix shared interrupts issues with SMP and RT kernels")
Reported-by: Wang Zhaolong <wangzhaolong@fnnas.com>
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=221579
Signed-off-by: Qiliang Yuan <realwujing@gmail.com>
---
V2 -> V3:
- Hold hash_mutex across the first request_irq() completion to prevent a
second port from joining the chain and running the shared-IRQ THRE test
while IRQ startup is still in progress.
V1 -> V2:
- Add Reported-by tag from Wang Zhaolong.
v2: https://lore.kernel.org/r/20260528-bug-221579-8250-shared-irq-race-v2-1-06531202e54d@gmail.com
v1: https://lore.kernel.org/r/20260528-bug-221579-8250-shared-irq-race-v1-1-30980cca02f3@gmail.com
---
drivers/tty/serial/8250/8250_core.c | 49 ++++++++++++++++++++++++++++---------
1 file changed, 38 insertions(+), 11 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index a428e88938eb7..55a1a0515397f 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -134,7 +134,7 @@ static struct irq_info *serial_get_or_create_irq_info(const struct uart_8250_por
{
struct irq_info *i;
- guard(mutex)(&hash_mutex);
+ lockdep_assert_held(&hash_mutex);
hash_for_each_possible(irq_lists, i, node, up->port.irq)
if (i->irq == up->port.irq)
@@ -151,27 +151,54 @@ static struct irq_info *serial_get_or_create_irq_info(const struct uart_8250_por
return i;
}
+/*
+ * serial_link_irq_chain() hooks the given 8250 port into the IRQ chain.
+ *
+ * hash_mutex must be held from the hash lookup through the first
+ * request_irq() completion. Dropping it earlier allows a concurrent
+ * serial_unlink_irq_chain() to race in after i->head is published but
+ * before the IRQ is fully set up — another port sharing the IRQ can then
+ * join the chain and run the shared-IRQ THRE test while IRQ startup is
+ * still in progress, triggering an "Unbalanced enable for IRQ" warning
+ * in kernel/irq/manage.c.
+ */
static int serial_link_irq_chain(struct uart_8250_port *up)
{
struct irq_info *i;
int ret;
+ mutex_lock(&hash_mutex);
+
i = serial_get_or_create_irq_info(up);
- if (IS_ERR(i))
+ if (IS_ERR(i)) {
+ mutex_unlock(&hash_mutex);
return PTR_ERR(i);
+ }
- scoped_guard(spinlock_irq, &i->lock) {
- if (i->head) {
- list_add(&up->list, i->head);
-
- return 0;
- }
+ /*
+ * Serialise against the list manipulation in the interrupt handler
+ * and in serial_unlink_irq_chain(). hash_mutex is still held which
+ * prevents serial_unlink_irq_chain() from entering and freeing the
+ * irq_info until the first request_irq() completes.
+ */
+ spin_lock_irq(&i->lock);
+ if (i->head) {
+ list_add(&up->list, i->head);
+ spin_unlock_irq(&i->lock);
+ mutex_unlock(&hash_mutex);
- INIT_LIST_HEAD(&up->list);
- i->head = &up->list;
+ return 0;
}
- ret = request_irq(up->port.irq, serial8250_interrupt, up->port.irqflags, up->port.name, i);
+ INIT_LIST_HEAD(&up->list);
+ i->head = &up->list;
+ spin_unlock_irq(&i->lock);
+
+ ret = request_irq(up->port.irq, serial8250_interrupt,
+ up->port.irqflags, up->port.name, i);
+
+ mutex_unlock(&hash_mutex);
+
if (ret < 0)
serial_do_unlink(i, up);
---
base-commit: eb3f4b7426cfd2b79d65b7d37155480b32259a11
change-id: 20260528-bug-221579-8250-shared-irq-race-581e4900a178
Best regards,
--
Qiliang Yuan <realwujing@gmail.com>
^ permalink raw reply related
* Re: [PATCH v2] serial: 8250: fix use-after-free in IRQ chain handling
From: Wang Zhaolong @ 2026-05-29 2:39 UTC (permalink / raw)
To: Qiliang Yuan
Cc: Greg Kroah-Hartman, Jiri Slaby, Anton Vorontsov, Alan Cox,
linux-kernel, linux-serial, Qiliang Yuan
In-Reply-To: <5cf37150673ea4d5c28f94db91cdf68504b50522.22df93db.e9a5.40db.a812.14ba5b63f31b@feishu.cn>
My patch should cover that link/unlink lifetime race as well.
serial_unlink_irq_chain() already holds hash_mutex before walking irq_lists
and before calling serial_do_unlink(), which can kfree(i). My patch keeps
hash_mutex held in serial_link_irq_chain() from serial_get_or_create_irq_info()
through the list update and the first request_irq() completion.
So once serial_link_irq_chain() has found or allocated i, a concurrent unlink
cannot enter serial_unlink_irq_chain() and free that irq_info until the link
path is done.
The important difference is that the lock is also held across the first
request_irq(). That is required for the reported "Unbalanced enable for IRQ"
race, because publishing i->head before request_irq() completes lets another
port join the chain and run the shared-IRQ THRE test while IRQ startup is still
in progress.
Thanks,
Wang Zhaolong
^ permalink raw reply
* Re: [PATCH v2] serial: 8250: fix use-after-free in IRQ chain handling
From: 王曌龙 @ 2026-05-29 2:07 UTC (permalink / raw)
To: Qiliang Yuan
Cc: Greg Kroah-Hartman, Jiri Slaby, Anton Vorontsov, Alan Cox,
linux-kernel, linux-serial, Qiliang Yuan
In-Reply-To: <20260528-bug-221579-8250-shared-irq-race-v2-1-06531202e54d@gmail.com>
Hi Qiliang
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=221579
This patch does not fix the reported issue.
I tested it with the Bugzilla reproducer, and the "Unbalanced enable for IRQ"
warning is still reproducible.
```
chmod +x /tmp/run-tty-open-race.sh
/tmp/run-tty-open-race.sh
racing simultaneous open/close on /dev/ttyS1 and /dev/ttyS3, 100000 iterations
[ 61.937561][ T915] ------------[ cut here ]------------
[ 61.938276][ T915] Unbalanced enable for IRQ 3
[ 61.938828][ T915] WARNING: kernel/irq/manage.c:774 at __enable_irq+0x33/0x60, CPU#1: tty-open-race/915
[ 61.939978][ T915] Modules linked in: af_packet(E) kvm_amd(E) nfnetlink(E) ahci(E) libahci(E) libata(E) lpc_ich(E) virtio_net(E) net_failover(E) failover(E) sunrpc(E) dm_mod(E) raid1(E) binfmt_misc(E) md_mod(E) 9p(E) 9pnet_virtio(E) 9pnet(E) ext4(E) netfs(E) virtio_scsi(E) scsi_mod(E) crc16(E) mbcache(E) jbd2(E) virtio_blk(E) scsi_common(E) virtiofs(E) fuse(E) vmw_vsock_virtio_transport(E) vmw_vsock_virtio_transport_common(E) vsock(E)
[ 61.944542][ T915] CPU: 1 UID: 0 PID: 915 Comm: tty-open-race Tainted: G E 7.1.0-rc5-trim+ #14 PREEMPT(full) 43dd8c1eb10e193a5538796ee5a4e7f9cf3e86f3
[ 61.946304][ T915] Tainted: [E]=UNSIGNED_MODULE
[ 61.946866][ T915] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS Arch Linux 1.17.0-2-2 04/01/2014
[ 61.948045][ T915] RIP: 0010:__enable_irq+0x39/0x60
[ 61.948650][ T915] Code: 85 c0 74 19 83 f8 01 74 0e 83 e8 01 89 87 80 00 00 00 e9 d5 52 c8 ff f6 47 7d 08 74 17 48 8d 05 ed 01 2c 02 8b 77 2c 48 89 c7 <67> 48 0f b9 3a e9 b8 52 c8 ff 81 4f 78 00 04 00 00 ba 01 00 00 00
[ 61.950925][ T915] RSP: 0018:ffffc90000f67858 EFLAGS: 00010046
[ 61.951634][ T915] RAX: ffffffff8373f030 RBX: 0000000000000001 RCX: ffff888108912380
[ 61.952574][ T915] RDX: 0000000000000001 RSI: 0000000000000003 RDI: ffffffff8373f030
[ 61.953508][ T915] RBP: 0000000000000003 R08: ffff888108912300 R09: 0000000000000001
[ 61.954438][ T915] R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000246
[ 61.955367][ T915] R13: ffff8881242ef000 R14: 0000000000000001 R15: 0000000000000002
[ 61.956309][ T915] FS: 00007fba5a6106c0(0000) GS:ffff8881eec34000(0000) knlGS:0000000000000000
[ 61.957351][ T915] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 61.958122][ T915] CR2: 000000001d2b1258 CR3: 000000010e5cf000 CR4: 0000000000750ef0
[ 61.959056][ T915] PKRU: 55555554
[ 61.959501][ T915] Call Trace:
[ 61.959913][ T915] <TASK>
[ 61.960262][ T915] enable_irq+0x7e/0x100
[ 61.960769][ T915] serial8250_do_startup+0x7ce/0xa80
[ 61.961395][ T915] uart_port_startup+0x13d/0x440
[ 61.961981][ T915] uart_port_activate+0x5b/0xb0
[ 61.962551][ T915] tty_port_open+0x90/0x110
[ 61.963089][ T915] ? srso_alias_return_thunk+0x5/0xfbef5
[ 61.963747][ T915] uart_open+0x1e/0x30
```
The problem is that the first-port path still drops hash_mutex before
request_irq() completes:
i->head = &up->list;
mutex_unlock(&hash_mutex);
request_irq(..., i);
After i->head is published, another port sharing the IRQ can still join the
chain and run the shared-IRQ THRE test while the IRQ core is starting the
interrupt for the first port.
So the lock has to cover the first request_irq() completion. Covering only
the i->head check and list_add() is not sufficient.
Test result: still fails with the Bugzilla reproducer.
Thanks,
Wang Zhaolong
^ permalink raw reply
* [PATCH] serial: mxs-auart: fix probe error paths and clock handling
From: Rosen Penev @ 2026-05-28 23:06 UTC (permalink / raw)
To: linux-serial
Cc: Greg Kroah-Hartman, Jiri Slaby, Frank Li, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam,
open list:TTY LAYER AND SERIAL DRIVERS,
open list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
Sashiko reported three pre-existing bugs in the mxs-auart driver:
- For non-ASM9260 variants, mxs_get_clks() obtained the clock but never
prepared/enabled it, leaving register accesses in probe at risk of
faulting if the bootloader had gated the clock.
- The error path and remove function used pdev->id instead of
s->port.line to clear the auart_port[] slot. For DT-probed devices
pdev->id is -1, causing an out-of-bounds write and leaving a dangling
pointer in the array.
- The probe error path called iounmap() while the IRQ was still
registered. An interrupt during that window would dereference the
unmapped membase.
All of this is a consequence of using mixed devm and non devm. Instead
of working around these issues, go full devm so that everything can be
cleaned up properly.
devm_clk_get_enabled allows removing clk_prepare_enable.
devm_platform_get_and_ioremap_resource is a bit worrying as it calls
request_mem_region. Manual review of the dts files shows no overlapping
memory regions, so should be good to go.
Use devm in mxs_auart_request_gpio_irq as devm is used in probe.
Assisted-by: Opencode:Big-pickle
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
drivers/tty/serial/mxs-auart.c | 110 ++++++---------------------------
1 file changed, 20 insertions(+), 90 deletions(-)
diff --git a/drivers/tty/serial/mxs-auart.c b/drivers/tty/serial/mxs-auart.c
index 697318dbb146..8b6463df11b3 100644
--- a/drivers/tty/serial/mxs-auart.c
+++ b/drivers/tty/serial/mxs-auart.c
@@ -1457,48 +1457,24 @@ static void mxs_init_regs(struct mxs_auart_port *s)
static int mxs_get_clks(struct mxs_auart_port *s,
struct platform_device *pdev)
{
- int err;
-
if (!is_asm9260_auart(s)) {
- s->clk = devm_clk_get(&pdev->dev, NULL);
+ s->clk = devm_clk_get_enabled(&pdev->dev, NULL);
return PTR_ERR_OR_ZERO(s->clk);
}
- s->clk = devm_clk_get(s->dev, "mod");
+ s->clk = devm_clk_get_enabled(s->dev, "mod");
if (IS_ERR(s->clk)) {
dev_err(s->dev, "Failed to get \"mod\" clk\n");
return PTR_ERR(s->clk);
}
- s->clk_ahb = devm_clk_get(s->dev, "ahb");
+ s->clk_ahb = devm_clk_get_enabled(s->dev, "ahb");
if (IS_ERR(s->clk_ahb)) {
dev_err(s->dev, "Failed to get \"ahb\" clk\n");
return PTR_ERR(s->clk_ahb);
}
- err = clk_prepare_enable(s->clk_ahb);
- if (err) {
- dev_err(s->dev, "Failed to enable ahb_clk!\n");
- return err;
- }
-
- err = clk_set_rate(s->clk, clk_get_rate(s->clk_ahb));
- if (err) {
- dev_err(s->dev, "Failed to set rate!\n");
- goto disable_clk_ahb;
- }
-
- err = clk_prepare_enable(s->clk);
- if (err) {
- dev_err(s->dev, "Failed to enable clk!\n");
- goto disable_clk_ahb;
- }
-
- return 0;
-
-disable_clk_ahb:
- clk_disable_unprepare(s->clk_ahb);
- return err;
+ return clk_set_rate(s->clk, clk_get_rate(s->clk_ahb));
}
static int mxs_auart_init_gpios(struct mxs_auart_port *s, struct device *dev)
@@ -1529,15 +1505,6 @@ static int mxs_auart_init_gpios(struct mxs_auart_port *s, struct device *dev)
return 0;
}
-static void mxs_auart_free_gpio_irq(struct mxs_auart_port *s)
-{
- enum mctrl_gpio_idx i;
-
- for (i = 0; i < UART_GPIO_MAX; i++)
- if (s->gpio_irq[i] >= 0)
- free_irq(s->gpio_irq[i], s);
-}
-
static int mxs_auart_request_gpio_irq(struct mxs_auart_port *s)
{
int *irq = s->gpio_irq;
@@ -1549,21 +1516,13 @@ static int mxs_auart_request_gpio_irq(struct mxs_auart_port *s)
continue;
irq_set_status_flags(irq[i], IRQ_NOAUTOEN);
- err = request_irq(irq[i], mxs_auart_irq_handle,
- IRQ_TYPE_EDGE_BOTH, dev_name(s->dev), s);
+ err = devm_request_irq(s->dev, irq[i], mxs_auart_irq_handle, IRQ_TYPE_EDGE_BOTH,
+ dev_name(s->dev), s);
if (err)
dev_err(s->dev, "%s - Can't get %d irq\n",
__func__, irq[i]);
}
- /*
- * If something went wrong, rollback.
- * Be careful: i may be unsigned.
- */
- while (err && (i-- > 0))
- if (irq[i] >= 0)
- free_irq(irq[i], s);
-
return err;
}
@@ -1604,18 +1563,10 @@ static int mxs_auart_probe(struct platform_device *pdev)
if (ret)
return ret;
- r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- if (!r) {
- ret = -ENXIO;
- goto out_disable_clks;
- }
-
+ s->port.membase = devm_platform_get_and_ioremap_resource(pdev, 0, &r);
+ if (IS_ERR(s->port.membase))
+ return PTR_ERR(s->port.membase);
s->port.mapbase = r->start;
- s->port.membase = ioremap(r->start, resource_size(r));
- if (!s->port.membase) {
- ret = -ENOMEM;
- goto out_disable_clks;
- }
s->port.ops = &mxs_auart_ops;
s->port.iotype = UPIO_MEM;
s->port.fifosize = MXS_AUART_FIFO_SIZE;
@@ -1628,23 +1579,20 @@ static int mxs_auart_probe(struct platform_device *pdev)
s->mctrl_prev = 0;
irq = platform_get_irq(pdev, 0);
- if (irq < 0) {
- ret = irq;
- goto out_iounmap;
- }
+ if (irq < 0)
+ return irq;
s->port.irq = irq;
- ret = devm_request_irq(&pdev->dev, irq, mxs_auart_irq_handle, 0,
- dev_name(&pdev->dev), s);
+ ret = devm_request_irq(&pdev->dev, irq, mxs_auart_irq_handle, 0, dev_name(&pdev->dev), s);
if (ret)
- goto out_iounmap;
+ return ret;
platform_set_drvdata(pdev, s);
ret = mxs_auart_init_gpios(s, &pdev->dev);
if (ret) {
dev_err(&pdev->dev, "Failed to initialize GPIOs.\n");
- goto out_iounmap;
+ return ret;
}
/*
@@ -1652,15 +1600,17 @@ static int mxs_auart_probe(struct platform_device *pdev)
*/
ret = mxs_auart_request_gpio_irq(s);
if (ret)
- goto out_iounmap;
+ return ret;
auart_port[s->port.line] = s;
mxs_auart_reset_deassert(s);
ret = uart_add_one_port(&auart_driver, &s->port);
- if (ret)
- goto out_free_qpio_irq;
+ if (ret) {
+ auart_port[s->port.line] = NULL;
+ return ret;
+ }
/* ASM9260 don't have version reg */
if (is_asm9260_auart(s)) {
@@ -1673,20 +1623,6 @@ static int mxs_auart_probe(struct platform_device *pdev)
}
return 0;
-
-out_free_qpio_irq:
- mxs_auart_free_gpio_irq(s);
- auart_port[pdev->id] = NULL;
-
-out_iounmap:
- iounmap(s->port.membase);
-
-out_disable_clks:
- if (is_asm9260_auart(s)) {
- clk_disable_unprepare(s->clk);
- clk_disable_unprepare(s->clk_ahb);
- }
- return ret;
}
static void mxs_auart_remove(struct platform_device *pdev)
@@ -1694,13 +1630,7 @@ static void mxs_auart_remove(struct platform_device *pdev)
struct mxs_auart_port *s = platform_get_drvdata(pdev);
uart_remove_one_port(&auart_driver, &s->port);
- auart_port[pdev->id] = NULL;
- mxs_auart_free_gpio_irq(s);
- iounmap(s->port.membase);
- if (is_asm9260_auart(s)) {
- clk_disable_unprepare(s->clk);
- clk_disable_unprepare(s->clk_ahb);
- }
+ auart_port[s->port.line] = NULL;
}
static struct platform_driver mxs_auart_driver = {
--
2.54.0
^ permalink raw reply related
* [PATCH] serial: mxs-auart: fix 64-bit cast in probe
From: Rosen Penev @ 2026-05-28 20:30 UTC (permalink / raw)
To: linux-serial
Cc: Greg Kroah-Hartman, Jiri Slaby, Frank Li, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam,
open list:TTY LAYER AND SERIAL DRIVERS,
open list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
of_device_get_match_data() returns a pointer. Casting it directly to
enum truncates on 64-bit platforms. Cast to unsigned long instead.
Fixes compilation with W=1.
Assisted-by: Opencode:Big-pickle
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
drivers/tty/serial/mxs-auart.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tty/serial/mxs-auart.c b/drivers/tty/serial/mxs-auart.c
index 697318dbb146..de97c0f74e7d 100644
--- a/drivers/tty/serial/mxs-auart.c
+++ b/drivers/tty/serial/mxs-auart.c
@@ -1598,7 +1598,7 @@ static int mxs_auart_probe(struct platform_device *pdev)
return -EINVAL;
}
- s->devtype = (enum mxs_auart_type)of_device_get_match_data(&pdev->dev);
+ s->devtype = (unsigned long)of_device_get_match_data(&pdev->dev);
ret = mxs_get_clks(s, pdev);
if (ret)
--
2.54.0
^ permalink raw reply related
* [PATCH] serial: 8250_pci: fix -Winitializer-overrides for Brainboxes UC-260/271/701/756 entries
From: Rosen Penev @ 2026-05-28 20:12 UTC (permalink / raw)
To: linux-serial
Cc: Greg Kroah-Hartman, Jiri Slaby, Nathan Chancellor,
Nick Desaulniers, Bill Wendling, Justin Stitt,
open list:TTY LAYER AND SERIAL DRIVERS,
open list:CLANG/LLVM BUILD SUPPORT:Keyword:b(?i:clang|llvm)b
PCI_VDEVICE() expands to set .class=0 and .class_mask=0, but the Brainboxes
UC-260/271/701/756 entries immediately override those fields. This causes
a build error with clang -Werror,-Winitializer-overrides.
Fix by expanding PCI_VDEVICE() manually, omitting the trailing
.class/.class_mask zeroes so each field is set exactly once.
Found with W=1
Assisted-by: Opencode:Big-pickle
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
drivers/tty/serial/8250/8250_pci.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c
index 3e5bc9e8d269..0513f4b3c093 100644
--- a/drivers/tty/serial/8250/8250_pci.c
+++ b/drivers/tty/serial/8250/8250_pci.c
@@ -5394,12 +5394,14 @@ static const struct pci_device_id serial_pci_tbl[] = {
* Brainboxes UC-260/271/701/756
*/
{
- PCI_VDEVICE(INTASHIELD, 0x0D21),
+ .vendor = PCI_VENDOR_ID_INTASHIELD, .device = 0x0D21,
+ .subvendor = PCI_ANY_ID, .subdevice = PCI_ANY_ID,
.class = PCI_CLASS_COMMUNICATION_MULTISERIAL << 8,
.class_mask = 0xffff00,
.driver_data = pbn_b2_4_115200,
}, {
- PCI_VDEVICE(INTASHIELD, 0x0E34),
+ .vendor = PCI_VENDOR_ID_INTASHIELD, .device = 0x0E34,
+ .subvendor = PCI_ANY_ID, .subdevice = PCI_ANY_ID,
.class = PCI_CLASS_COMMUNICATION_MULTISERIAL << 8,
.class_mask = 0xffff00,
.driver_data = pbn_b2_4_115200,
--
2.54.0
^ permalink raw reply related
* [PATCH v2] serial: qcom_geni: Fix RX DMA stall when SE_DMA_RX_LEN_IN is zero
From: Viken Dadhaniya @ 2026-05-28 17:18 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby, Bartosz Golaszewski
Cc: linux-arm-msm, linux-kernel, linux-serial, stable,
Bartosz Golaszewski, Viken Dadhaniya
In qcom_geni_serial_handle_rx_dma(), geni_se_rx_dma_unprep() clears
port->rx_dma_addr before SE_DMA_RX_LEN_IN is read. If the register is zero,
for example when the RX stale counter fires on an idle line, the handler
returns without calling geni_se_rx_dma_prep().
The next RX DMA interrupt then hits the !port->rx_dma_addr guard and
returns immediately, so the RX DMA buffer is never rearmed and later input
is lost.
Keep the handler on the rearm path when rx_in is zero. Warn about the
unexpected zero-length DMA completion, skip received-data handling, and
always call geni_se_rx_dma_prep().
Fixes: 2aaa43c70778 ("tty: serial: qcom-geni-serial: add support for serial engine DMA")
Cc: stable@vger.kernel.org
Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
Signed-off-by: Viken Dadhaniya <viken.dadhaniya@oss.qualcomm.com>
---
Changes in v2:
- Add Cc: stable@vger.kernel.org tag (missed in v1)
- Link to v1: https://patch.msgid.link/20260528-serial-rx-0-byte-fix-v1-1-dc4e876c7368@oss.qualcomm.com
---
drivers/tty/serial/qcom_geni_serial.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
index d81b539cff7f..7ead87b4eb65 100644
--- a/drivers/tty/serial/qcom_geni_serial.c
+++ b/drivers/tty/serial/qcom_geni_serial.c
@@ -905,12 +905,9 @@ static void qcom_geni_serial_handle_rx_dma(struct uart_port *uport, bool drop)
port->rx_dma_addr = 0;
rx_in = readl(uport->membase + SE_DMA_RX_LEN_IN);
- if (!rx_in) {
- dev_warn(uport->dev, "serial engine reports 0 RX bytes in!\n");
- return;
- }
-
- if (!drop)
+ if (!rx_in)
+ dev_warn_ratelimited(uport->dev, "serial engine reports 0 RX bytes in!\n");
+ else if (!drop)
handle_rx_uart(uport, rx_in);
ret = geni_se_rx_dma_prep(&port->se, port->rx_buf,
---
base-commit: e7d700e14934e68f86338c5610cf2ae76798b663
change-id: 20260528-serial-rx-0-byte-fix-ec9d08cfe78e
Best regards,
--
Viken Dadhaniya <viken.dadhaniya@oss.qualcomm.com>
^ permalink raw reply related
* [PATCH v2] serial: 8250: fix use-after-free in IRQ chain handling
From: Qiliang Yuan @ 2026-05-28 15:50 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby, Anton Vorontsov, Alan Cox
Cc: linux-kernel, linux-serial, wangzhaolong, Qiliang Yuan
serial_unlink_irq_chain() holds hash_mutex and calls free_irq() + kfree(i)
when it sees an empty port list. serial_link_irq_chain() released
hash_mutex after serial_get_or_create_irq_info() but before acquiring
i->lock. This gap allowed a concurrent unlink to observe list_empty()
as true while a new port was still being added, free i, and trigger a
use-after-free.
The corrupted list/irq_info then causes an "Unbalanced enable for IRQ"
warning (kernel/irq/manage.c:774) because irq_shutdown() in the premature
free_irq() path hard-sets desc->depth to 1, breaking the disable_irq/
enable_irq pairing in serial8250_THRE_test().
Fix by pulling hash_mutex into serial_link_irq_chain() so that it covers
the i->head check and the list_add/INIT_LIST_HEAD under i->lock.
serial_unlink_irq_chain() already holds hash_mutex throughout, so the
race window is closed.
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=221579
Reported-by: Wang Zhaolong <wangzhaolong@fnnas.com>
Fixes: 768aec0b5bcc ("serial: 8250: fix shared interrupts issues with SMP and RT kernels")
Signed-off-by: Qiliang Yuan <realwujing@gmail.com>
---
Changes in v2:
- EDITME: describe what is new in this series revision.
- EDITME: use bulletpoints and terse descriptions.
- Link to v1: https://lore.kernel.org/r/20260528-bug-221579-8250-shared-irq-race-v1-1-30980cca02f3@gmail.com
---
drivers/tty/serial/8250/8250_core.c | 46 ++++++++++++++++++++++++++++---------
1 file changed, 35 insertions(+), 11 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index a428e88938eb7..dfe76223ce10c 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -134,7 +134,7 @@ static struct irq_info *serial_get_or_create_irq_info(const struct uart_8250_por
{
struct irq_info *i;
- guard(mutex)(&hash_mutex);
+ lockdep_assert_held(&hash_mutex);
hash_for_each_possible(irq_lists, i, node, up->port.irq)
if (i->irq == up->port.irq)
@@ -151,27 +151,51 @@ static struct irq_info *serial_get_or_create_irq_info(const struct uart_8250_por
return i;
}
+/*
+ * serial_link_irq_chain() hooks the given 8250 port into the IRQ chain.
+ *
+ * hash_mutex must be held across checking i->head and adding the port to
+ * the list. Without this, a concurrent serial_unlink_irq_chain() can race
+ * in after hash_mutex is dropped but before i->lock is acquired, observe
+ * list_empty(i->head) as true, call free_irq() and kfree(i) — triggering a
+ * use-after-free and ultimately an "Unbalanced enable for IRQ" warning in
+ * kernel/irq/manage.c.
+ */
static int serial_link_irq_chain(struct uart_8250_port *up)
{
struct irq_info *i;
int ret;
+ mutex_lock(&hash_mutex);
+
i = serial_get_or_create_irq_info(up);
- if (IS_ERR(i))
+ if (IS_ERR(i)) {
+ mutex_unlock(&hash_mutex);
return PTR_ERR(i);
+ }
- scoped_guard(spinlock_irq, &i->lock) {
- if (i->head) {
- list_add(&up->list, i->head);
-
- return 0;
- }
+ /*
+ * Serialise against the list manipulation in the interrupt handler
+ * and in serial_unlink_irq_chain(). hash_mutex is still held which
+ * prevents serial_unlink_irq_chain() from running concurrently.
+ */
+ spin_lock_irq(&i->lock);
+ if (i->head) {
+ list_add(&up->list, i->head);
+ spin_unlock_irq(&i->lock);
+ mutex_unlock(&hash_mutex);
- INIT_LIST_HEAD(&up->list);
- i->head = &up->list;
+ return 0;
}
- ret = request_irq(up->port.irq, serial8250_interrupt, up->port.irqflags, up->port.name, i);
+ INIT_LIST_HEAD(&up->list);
+ i->head = &up->list;
+ spin_unlock_irq(&i->lock);
+
+ mutex_unlock(&hash_mutex);
+
+ ret = request_irq(up->port.irq, serial8250_interrupt,
+ up->port.irqflags, up->port.name, i);
if (ret < 0)
serial_do_unlink(i, up);
---
base-commit: eb3f4b7426cfd2b79d65b7d37155480b32259a11
change-id: 20260528-bug-221579-8250-shared-irq-race-581e4900a178
Best regards,
--
Qiliang Yuan <realwujing@gmail.com>
^ permalink raw reply related
* [PATCH] serial: 8250: fix use-after-free in IRQ chain handling
From: Qiliang Yuan @ 2026-05-28 15:08 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby, Anton Vorontsov, Alan Cox
Cc: linux-kernel, linux-serial, Qiliang Yuan
serial_unlink_irq_chain() holds hash_mutex and calls free_irq() + kfree(i)
when it sees an empty port list. serial_link_irq_chain() released
hash_mutex after serial_get_or_create_irq_info() but before acquiring
i->lock. This gap allowed a concurrent unlink to observe list_empty()
as true while a new port was still being added, free i, and trigger a
use-after-free.
The corrupted list/irq_info then causes an "Unbalanced enable for IRQ"
warning (kernel/irq/manage.c:774) because irq_shutdown() in the premature
free_irq() path hard-sets desc->depth to 1, breaking the disable_irq/
enable_irq pairing in serial8250_THRE_test().
Fix by pulling hash_mutex into serial_link_irq_chain() so that it covers
the i->head check and the list_add/INIT_LIST_HEAD under i->lock.
serial_unlink_irq_chain() already holds hash_mutex throughout, so the
race window is closed.
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=221579
Fixes: 768aec0b5bcc ("serial: 8250: fix shared interrupts issues with SMP and RT kernels")
Signed-off-by: Qiliang Yuan <realwujing@gmail.com>
---
drivers/tty/serial/8250/8250_core.c | 46 ++++++++++++++++++++++++++++---------
1 file changed, 35 insertions(+), 11 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index a428e88938eb7..dfe76223ce10c 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -134,7 +134,7 @@ static struct irq_info *serial_get_or_create_irq_info(const struct uart_8250_por
{
struct irq_info *i;
- guard(mutex)(&hash_mutex);
+ lockdep_assert_held(&hash_mutex);
hash_for_each_possible(irq_lists, i, node, up->port.irq)
if (i->irq == up->port.irq)
@@ -151,27 +151,51 @@ static struct irq_info *serial_get_or_create_irq_info(const struct uart_8250_por
return i;
}
+/*
+ * serial_link_irq_chain() hooks the given 8250 port into the IRQ chain.
+ *
+ * hash_mutex must be held across checking i->head and adding the port to
+ * the list. Without this, a concurrent serial_unlink_irq_chain() can race
+ * in after hash_mutex is dropped but before i->lock is acquired, observe
+ * list_empty(i->head) as true, call free_irq() and kfree(i) — triggering a
+ * use-after-free and ultimately an "Unbalanced enable for IRQ" warning in
+ * kernel/irq/manage.c.
+ */
static int serial_link_irq_chain(struct uart_8250_port *up)
{
struct irq_info *i;
int ret;
+ mutex_lock(&hash_mutex);
+
i = serial_get_or_create_irq_info(up);
- if (IS_ERR(i))
+ if (IS_ERR(i)) {
+ mutex_unlock(&hash_mutex);
return PTR_ERR(i);
+ }
- scoped_guard(spinlock_irq, &i->lock) {
- if (i->head) {
- list_add(&up->list, i->head);
-
- return 0;
- }
+ /*
+ * Serialise against the list manipulation in the interrupt handler
+ * and in serial_unlink_irq_chain(). hash_mutex is still held which
+ * prevents serial_unlink_irq_chain() from running concurrently.
+ */
+ spin_lock_irq(&i->lock);
+ if (i->head) {
+ list_add(&up->list, i->head);
+ spin_unlock_irq(&i->lock);
+ mutex_unlock(&hash_mutex);
- INIT_LIST_HEAD(&up->list);
- i->head = &up->list;
+ return 0;
}
- ret = request_irq(up->port.irq, serial8250_interrupt, up->port.irqflags, up->port.name, i);
+ INIT_LIST_HEAD(&up->list);
+ i->head = &up->list;
+ spin_unlock_irq(&i->lock);
+
+ mutex_unlock(&hash_mutex);
+
+ ret = request_irq(up->port.irq, serial8250_interrupt,
+ up->port.irqflags, up->port.name, i);
if (ret < 0)
serial_do_unlink(i, up);
---
base-commit: eb3f4b7426cfd2b79d65b7d37155480b32259a11
change-id: 20260528-bug-221579-8250-shared-irq-race-581e4900a178
Best regards,
--
Qiliang Yuan <realwujing@gmail.com>
^ permalink raw reply related
* Re: [PATCH v1] serial: qcom_geni: Fix RX DMA stall when SE_DMA_RX_LEN_IN is zero
From: Bartosz Golaszewski @ 2026-05-28 11:43 UTC (permalink / raw)
To: Viken Dadhaniya
Cc: linux-arm-msm, linux-kernel, linux-serial, Greg Kroah-Hartman,
Jiri Slaby, Bartosz Golaszewski
In-Reply-To: <20260528-serial-rx-0-byte-fix-v1-1-dc4e876c7368@oss.qualcomm.com>
On Thu, 28 May 2026 09:05:43 +0200, Viken Dadhaniya
<viken.dadhaniya@oss.qualcomm.com> said:
> In qcom_geni_serial_handle_rx_dma(), geni_se_rx_dma_unprep() clears
> port->rx_dma_addr before SE_DMA_RX_LEN_IN is read. If the register is zero,
> for example when the RX stale counter fires on an idle line, the handler
> returns without calling geni_se_rx_dma_prep().
>
> The next RX DMA interrupt then hits the !port->rx_dma_addr guard and
> returns immediately, so the RX DMA buffer is never rearmed and later input
> is lost.
>
> Keep the handler on the rearm path when rx_in is zero. Warn about the
> unexpected zero-length DMA completion, skip received-data handling, and
> always call geni_se_rx_dma_prep().
>
> Fixes: 2aaa43c70778 ("tty: serial: qcom-geni-serial: add support for serial engine DMA")
> Signed-off-by: Viken Dadhaniya <viken.dadhaniya@oss.qualcomm.com>
> ---
Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
^ permalink raw reply
* [PATCH 4/4] vc_screen: replace __get_free_pages() with kmalloc()
From: Mike Rapoport (Microsoft) @ 2026-05-28 10:24 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby
Cc: Mike Rapoport, linux-kernel, linux-mm, linux-serial
In-Reply-To: <20260528-b4-tty-v1-0-9da9f7aec5f2@kernel.org>
vcs_read() and vcs_write() allocate staging buffers with
__get_free_pages().
These buffers can be allocated with kmalloc() as there's nothing special
about them to go directly to the page allocator.
kmalloc() provides a better API that does not require ugly casts and it's a
modern way of saying "I need a page-sized buffer"
Replace use of __get_free_page() with kmalloc() and drop unused now
DEFINE_FREE(free_page_ptr ...)
Link: https://lore.kernel.org/all/700c5a5f-3128-4671-99aa-827ca73f5cdf@kernel.org
Link: https://lore.kernel.org/all/635405e4-9423-4a25-a6e7-e03c8ea0bcbe@redhat.com
Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
drivers/tty/vt/vc_screen.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/tty/vt/vc_screen.c b/drivers/tty/vt/vc_screen.c
index 4d2d46c95fef..386c80efc672 100644
--- a/drivers/tty/vt/vc_screen.c
+++ b/drivers/tty/vt/vc_screen.c
@@ -53,8 +53,6 @@
#define HEADER_SIZE 4u
#define CON_BUF_SIZE (IS_ENABLED(CONFIG_BASE_SMALL) ? 256 : PAGE_SIZE)
-DEFINE_FREE(free_page_ptr, void *, if (_T) free_page((unsigned long)_T));
-
/*
* Our minor space:
*
@@ -371,7 +369,7 @@ vcs_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
loff_t pos;
bool viewed, attr, uni_mode;
- char *con_buf __free(free_page_ptr) = (char *)__get_free_page(GFP_KERNEL);
+ char *con_buf __free(kfree) = kmalloc(PAGE_SIZE, GFP_KERNEL);
if (!con_buf)
return -ENOMEM;
@@ -596,7 +594,7 @@ vcs_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos)
if (use_unicode(inode))
return -EOPNOTSUPP;
- char *con_buf __free(free_page_ptr) = (char *)__get_free_page(GFP_KERNEL);
+ char *con_buf __free(kfree) = kmalloc(PAGE_SIZE, GFP_KERNEL);
if (!con_buf)
return -ENOMEM;
--
2.53.0
^ permalink raw reply related
* [PATCH 3/4] tty: serial: men_z135_uart: replace __get_free_page() with kmalloc()
From: Mike Rapoport (Microsoft) @ 2026-05-28 10:24 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby
Cc: Mike Rapoport, linux-kernel, linux-mm, linux-serial
In-Reply-To: <20260528-b4-tty-v1-0-9da9f7aec5f2@kernel.org>
men_z135_probe() allocates a receive staging buffer filled by the
CPU via memcpy_fromio() from the device MMIO region.
This buffer can be allocated with kmalloc() as there's nothing special
about it to go directly to the page allocator.
kmalloc() provides a better API that does not require ugly casts and
kfree() does not need to know the size of the freed object.
Replace use of __get_free_page() with kmalloc() and free_page() with
kfree().
Link: https://lore.kernel.org/all/635405e4-9423-4a25-a6e7-e03c8ea0bcbe@redhat.com
Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
drivers/tty/serial/men_z135_uart.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/tty/serial/men_z135_uart.c b/drivers/tty/serial/men_z135_uart.c
index 6fad57fee912..9c32b01edc9e 100644
--- a/drivers/tty/serial/men_z135_uart.c
+++ b/drivers/tty/serial/men_z135_uart.c
@@ -17,6 +17,7 @@
#include <linux/bitops.h>
#include <linux/mcb.h>
+#include <linux/slab.h>
#define MEN_Z135_MAX_PORTS 12
#define MEN_Z135_BASECLK 29491200
#define MEN_Z135_FIFO_SIZE 1024
@@ -811,7 +812,7 @@ static int men_z135_probe(struct mcb_device *mdev,
if (!uart)
return -ENOMEM;
- uart->rxbuf = (unsigned char *)__get_free_page(GFP_KERNEL);
+ uart->rxbuf = kmalloc(PAGE_SIZE, GFP_KERNEL);
if (!uart->rxbuf)
return -ENOMEM;
@@ -841,7 +842,7 @@ static int men_z135_probe(struct mcb_device *mdev,
return 0;
err:
- free_page((unsigned long) uart->rxbuf);
+ kfree(uart->rxbuf);
dev_err(dev, "Failed to add UART: %d\n", err);
return err;
@@ -858,7 +859,7 @@ static void men_z135_remove(struct mcb_device *mdev)
line--;
uart_remove_one_port(&men_z135_driver, &uart->port);
- free_page((unsigned long) uart->rxbuf);
+ kfree(uart->rxbuf);
}
static const struct mcb_device_id men_z135_ids[] = {
--
2.53.0
^ permalink raw reply related
* [PATCH 2/4] tty: amiserial: replace get_zeroed_page() with kzalloc()
From: Mike Rapoport (Microsoft) @ 2026-05-28 10:24 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby
Cc: Mike Rapoport, linux-kernel, linux-mm, linux-serial
In-Reply-To: <20260528-b4-tty-v1-0-9da9f7aec5f2@kernel.org>
rs_startup() allocates a transmit ring buffer that is used to buffer reads
and writes from/to serial data register.
This buffer can be allocated with kmalloc() as there's nothing special
about it to go directly to the page allocator.
kmalloc() provides a better API that does not require ugly casts and
kfree() does not need to know the size of the freed object.
Replace use of get_zeroed_page() with kzalloc() and free_page() with
kfree().
Link: https://lore.kernel.org/all/635405e4-9423-4a25-a6e7-e03c8ea0bcbe@redhat.com
Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
drivers/tty/amiserial.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/tty/amiserial.c b/drivers/tty/amiserial.c
index 81eaca751541..28af0fd98181 100644
--- a/drivers/tty/amiserial.c
+++ b/drivers/tty/amiserial.c
@@ -443,23 +443,23 @@ static int rs_startup(struct tty_struct *tty, struct serial_state *info)
struct tty_port *port = &info->tport;
unsigned long flags;
int retval=0;
- unsigned long page;
+ void *buffer;
- page = get_zeroed_page(GFP_KERNEL);
- if (!page)
+ buffer = kzalloc(PAGE_SIZE, GFP_KERNEL);
+ if (!buffer)
return -ENOMEM;
local_irq_save(flags);
if (tty_port_initialized(port)) {
- free_page(page);
+ kfree(buffer);
goto errout;
}
if (info->xmit.buf)
- free_page(page);
+ kfree(buffer);
else
- info->xmit.buf = (unsigned char *) page;
+ info->xmit.buf = buffer;
#ifdef SERIAL_DEBUG_OPEN
printk("starting up ttys%d ...", info->line);
@@ -537,7 +537,7 @@ static void rs_shutdown(struct tty_struct *tty, struct serial_state *info)
*/
free_irq(IRQ_AMIGA_VERTB, info);
- free_page((unsigned long)info->xmit.buf);
+ kfree(info->xmit.buf);
info->xmit.buf = NULL;
info->IER = 0;
--
2.53.0
^ permalink raw reply related
* [PATCH 1/4] serial: pch: replace __get_free_page() with kmalloc()
From: Mike Rapoport (Microsoft) @ 2026-05-28 10:24 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby
Cc: Mike Rapoport, linux-kernel, linux-mm, linux-serial
In-Reply-To: <20260528-b4-tty-v1-0-9da9f7aec5f2@kernel.org>
pch_uart_init_port() allocates a staging buffer for non-DMA receive path
using __get_free_page().
This buffer can be allocated with kmalloc() as there's nothing special
about it to go directly to the page allocator.
kmalloc() provides a better API that does not require ugly casts and
kfree() does not need to know the size of the freed object.
Replace use of __get_free_page() with kmalloc() and free_page() with
kfree().
Link: https://lore.kernel.org/all/635405e4-9423-4a25-a6e7-e03c8ea0bcbe@redhat.com
Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
drivers/tty/serial/pch_uart.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/tty/serial/pch_uart.c b/drivers/tty/serial/pch_uart.c
index 6729d8e83c3c..07d8cdb58912 100644
--- a/drivers/tty/serial/pch_uart.c
+++ b/drivers/tty/serial/pch_uart.c
@@ -1655,7 +1655,7 @@ static struct eg20t_port *pch_uart_init_port(struct pci_dev *pdev,
if (priv == NULL)
goto init_port_alloc_err;
- rxbuf = (unsigned char *)__get_free_page(GFP_KERNEL);
+ rxbuf = kmalloc(PAGE_SIZE, GFP_KERNEL);
if (!rxbuf)
goto init_port_free_txbuf;
@@ -1728,7 +1728,7 @@ static struct eg20t_port *pch_uart_init_port(struct pci_dev *pdev,
#ifdef CONFIG_SERIAL_PCH_UART_CONSOLE
pch_uart_ports[board->line_no] = NULL;
#endif
- free_page((unsigned long)rxbuf);
+ kfree(rxbuf);
init_port_free_txbuf:
kfree(priv);
init_port_alloc_err:
@@ -1743,7 +1743,7 @@ static void pch_uart_exit_port(struct eg20t_port *priv)
snprintf(name, sizeof(name), "uart%d_regs", priv->port.line);
debugfs_lookup_and_remove(name, NULL);
uart_remove_one_port(&pch_uart_driver, &priv->port);
- free_page((unsigned long)priv->rxbuf.buf);
+ kfree(priv->rxbuf.buf);
}
static void pch_uart_pci_remove(struct pci_dev *pdev)
--
2.53.0
^ permalink raw reply related
* [PATCH 0/4] tty: replace __get_free_pages() with kmalloc()
From: Mike Rapoport (Microsoft) @ 2026-05-28 10:24 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby
Cc: Mike Rapoport, linux-kernel, linux-mm, linux-serial
This is a (tiny) part of larger work of replacing page allocator calls
with kmalloc.
Nowadays the right way to say "I need a buffer" is kmalloc() rather than
ancient and ugly __get_free_pages().
---
Mike Rapoport (Microsoft) (4):
serial: pch: replace __get_free_page() with kmalloc()
tty: amiserial: replace get_zeroed_page() with kzalloc()
tty: serial: men_z135_uart: replace __get_free_page() with kmalloc()
vc_screen: replace __get_free_pages() with kmalloc()
drivers/tty/amiserial.c | 14 +++++++-------
drivers/tty/serial/men_z135_uart.c | 7 ++++---
drivers/tty/serial/pch_uart.c | 6 +++---
drivers/tty/vt/vc_screen.c | 6 ++----
4 files changed, 16 insertions(+), 17 deletions(-)
---
base-commit: 5d6919055dec134de3c40167a490f33c74c12581
change-id: 20260528-b4-tty-7c6e90f41d13
Best regards,
--
Sincerely yours,
Mike.
^ permalink raw reply
* Re: [PATCH v1] serial: qcom_geni: Fix RX DMA stall when SE_DMA_RX_LEN_IN is zero
From: Greg Kroah-Hartman @ 2026-05-28 7:28 UTC (permalink / raw)
To: Viken Dadhaniya
Cc: Jiri Slaby, Bartosz Golaszewski, linux-arm-msm, linux-kernel,
linux-serial
In-Reply-To: <20260528-serial-rx-0-byte-fix-v1-1-dc4e876c7368@oss.qualcomm.com>
On Thu, May 28, 2026 at 12:35:43PM +0530, Viken Dadhaniya wrote:
> In qcom_geni_serial_handle_rx_dma(), geni_se_rx_dma_unprep() clears
> port->rx_dma_addr before SE_DMA_RX_LEN_IN is read. If the register is zero,
> for example when the RX stale counter fires on an idle line, the handler
> returns without calling geni_se_rx_dma_prep().
>
> The next RX DMA interrupt then hits the !port->rx_dma_addr guard and
> returns immediately, so the RX DMA buffer is never rearmed and later input
> is lost.
>
> Keep the handler on the rearm path when rx_in is zero. Warn about the
> unexpected zero-length DMA completion, skip received-data handling, and
> always call geni_se_rx_dma_prep().
>
> Fixes: 2aaa43c70778 ("tty: serial: qcom-geni-serial: add support for serial engine DMA")
> Signed-off-by: Viken Dadhaniya <viken.dadhaniya@oss.qualcomm.com>
> ---
> drivers/tty/serial/qcom_geni_serial.c | 9 +++------
> 1 file changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
> index d81b539cff7f..7ead87b4eb65 100644
> --- a/drivers/tty/serial/qcom_geni_serial.c
> +++ b/drivers/tty/serial/qcom_geni_serial.c
> @@ -905,12 +905,9 @@ static void qcom_geni_serial_handle_rx_dma(struct uart_port *uport, bool drop)
> port->rx_dma_addr = 0;
>
> rx_in = readl(uport->membase + SE_DMA_RX_LEN_IN);
> - if (!rx_in) {
> - dev_warn(uport->dev, "serial engine reports 0 RX bytes in!\n");
> - return;
> - }
> -
> - if (!drop)
> + if (!rx_in)
> + dev_warn_ratelimited(uport->dev, "serial engine reports 0 RX bytes in!\n");
> + else if (!drop)
> handle_rx_uart(uport, rx_in);
>
> ret = geni_se_rx_dma_prep(&port->se, port->rx_buf,
>
> ---
> base-commit: e7d700e14934e68f86338c5610cf2ae76798b663
> change-id: 20260528-serial-rx-0-byte-fix-ec9d08cfe78e
>
> Best regards,
> --
> Viken Dadhaniya <viken.dadhaniya@oss.qualcomm.com>
>
>
Hi,
This is the friendly patch-bot of Greg Kroah-Hartman. You have sent him
a patch that has triggered this response. He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created. Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.
You are receiving this message because of the following common error(s)
as indicated below:
- You have marked a patch with a "Fixes:" tag for a commit that is in an
older released kernel, yet you do not have a cc: stable line in the
signed-off-by area at all, which means that the patch will not be
applied to any older kernel releases. To properly fix this, please
follow the documented rules in the
Documentation/process/stable-kernel-rules.rst file for how to resolve
this.
If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.
thanks,
greg k-h's patch email bot
^ permalink raw reply
* [PATCH v1] serial: qcom_geni: Fix RX DMA stall when SE_DMA_RX_LEN_IN is zero
From: Viken Dadhaniya @ 2026-05-28 7:05 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby, Bartosz Golaszewski
Cc: linux-arm-msm, linux-kernel, linux-serial, Viken Dadhaniya
In qcom_geni_serial_handle_rx_dma(), geni_se_rx_dma_unprep() clears
port->rx_dma_addr before SE_DMA_RX_LEN_IN is read. If the register is zero,
for example when the RX stale counter fires on an idle line, the handler
returns without calling geni_se_rx_dma_prep().
The next RX DMA interrupt then hits the !port->rx_dma_addr guard and
returns immediately, so the RX DMA buffer is never rearmed and later input
is lost.
Keep the handler on the rearm path when rx_in is zero. Warn about the
unexpected zero-length DMA completion, skip received-data handling, and
always call geni_se_rx_dma_prep().
Fixes: 2aaa43c70778 ("tty: serial: qcom-geni-serial: add support for serial engine DMA")
Signed-off-by: Viken Dadhaniya <viken.dadhaniya@oss.qualcomm.com>
---
drivers/tty/serial/qcom_geni_serial.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
index d81b539cff7f..7ead87b4eb65 100644
--- a/drivers/tty/serial/qcom_geni_serial.c
+++ b/drivers/tty/serial/qcom_geni_serial.c
@@ -905,12 +905,9 @@ static void qcom_geni_serial_handle_rx_dma(struct uart_port *uport, bool drop)
port->rx_dma_addr = 0;
rx_in = readl(uport->membase + SE_DMA_RX_LEN_IN);
- if (!rx_in) {
- dev_warn(uport->dev, "serial engine reports 0 RX bytes in!\n");
- return;
- }
-
- if (!drop)
+ if (!rx_in)
+ dev_warn_ratelimited(uport->dev, "serial engine reports 0 RX bytes in!\n");
+ else if (!drop)
handle_rx_uart(uport, rx_in);
ret = geni_se_rx_dma_prep(&port->se, port->rx_buf,
---
base-commit: e7d700e14934e68f86338c5610cf2ae76798b663
change-id: 20260528-serial-rx-0-byte-fix-ec9d08cfe78e
Best regards,
--
Viken Dadhaniya <viken.dadhaniya@oss.qualcomm.com>
^ permalink raw reply related
* Re: [PATCH v3 2/2] serial: qcom-geni: Add tracepoints for Qualcomm GENI serial driver
From: Praveen Talari @ 2026-05-27 16:16 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers, Jiri Slaby,
Konrad Dybcio, linux-kernel, linux-trace-kernel, linux-arm-msm,
linux-serial, Mukesh Kumar Savaliya, Aniket Randive,
chandana.chiluveru, jyothi.seerapu
In-Reply-To: <2026052738-unexpired-diligence-10f8@gregkh>
Hi
On 27-05-2026 13:47, Greg Kroah-Hartman wrote:
> On Tue, May 26, 2026 at 10:36:18AM +0530, Praveen Talari wrote:
>> Hi
>>
>> On 22-05-2026 15:17, Greg Kroah-Hartman wrote:
>>> On Mon, May 18, 2026 at 11:26:56PM +0530, 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.
>>>>
>>>> Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>>>> Signed-off-by: Praveen Talari <praveen.talari@oss.qualcomm.com>
>>>> ---
>>>> v2->v3:
>>>> - Updated commit text(removed example as it was available on cover
>>>> letter).
>>>> ---
>>>> drivers/tty/serial/qcom_geni_serial.c | 27 +++++++++++++++++++++++----
>>>> 1 file changed, 23 insertions(+), 4 deletions(-)
>>> This patch did not apply to my tree :(
>> Do you mean these patches are not applied cleanly?
> Yes.
>
>> If yes, i will push on linux-next tip.
> You mean rebase, right?
Yes, i have already posted V4.
https://lore.kernel.org/all/20260526-add-tracepoints-for-qcom-geni-serial-v4-0-e94fbaec0232@oss.qualcomm.com/
Thanks,
Praveen Talari
>
> thanks,
>
> greg k-h
^ permalink raw reply
* [PATCH] serial: 8250: serialize shared IRQ startup
From: Wang Zhaolong @ 2026-05-27 9:20 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby, Ilpo Järvinen, Xin Zhao,
Andy Shevchenko, Kees Cook, Ingo Molnar, Bing Fan, Guanbing Huang,
linux-kernel, linux-serial
Cc: Wang Zhaolong, stable
Concurrent startup of two 8250 ports sharing the same IRQ can trigger an
IRQ core warning:
Unbalanced enable for IRQ 3
WARNING: CPU: 0 PID: 580 at kernel/irq/manage.c:774 __enable_irq+0x3b/0x60
Call Trace:
enable_irq+0x8d/0x120
serial8250_do_startup+0x80d/0xa80
uart_port_startup+0x13d/0x440
uart_port_activate+0x5b/0xb0
tty_port_open+0xa1/0x120
uart_open+0x1e/0x30
tty_open+0x140/0x7a0
The second port can then run the shared-IRQ startup test while the IRQ core
is still enabling the line for the first port. The local
disable_irq_nosync()/enable_irq() pair is balanced, but the interleaving can
still unbalance the IRQ core disable depth.
That makes the QEMU legacy serial ports enter the shared-IRQ THRE test path:
serial8250_do_startup()
if (port->irqflags & IRQF_SHARED)
disable_irq_nosync(port->irq)
...
if (port->irqflags & IRQF_SHARED)
enable_irq(port->irq)
One possible interleaving is:
CPU0, ttyS1 CPU1, ttyS3
serial_link_irq_chain()
hash_add(i)
i->head = &ttyS1
request_irq()
serial_link_irq_chain()
find i in irq_lists
list_add(&ttyS3, i->head)
serial8250_do_startup()
disable_irq_nosync(irq)
irq_startup()
desc->depth = 0
enable_irq(irq)
WARN: Unbalanced enable for IRQ 3
Keep hash_mutex held in serial_link_irq_chain() until the first request_irq()
has completed. This prevents another 8250 port sharing the IRQ from joining
the chain and running the THRE test while the IRQ core is still starting the
interrupt.
This was reproduced in QEMU with ttyS1 and ttyS3 sharing IRQ 3. With this
change, 100000 synchronized open/close iterations on /dev/ttyS1 and /dev/ttyS3
completed without the warning.
Fixes: 64c79dfbc458 ("serial: 8250_pnp: Support configurable reg shift property")
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=221579
Cc: stable@vger.kernel.org # 6.10+
Assisted-by: Codex:gpt-5
Signed-off-by: Wang Zhaolong <wangzhaolong@fnnas.com>
---
drivers/tty/serial/8250/8250_core.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index a428e88938eb..64eed4dc343f 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -132,12 +132,10 @@ static void serial_do_unlink(struct irq_info *i, struct uart_8250_port *up)
*/
static struct irq_info *serial_get_or_create_irq_info(const struct uart_8250_port *up)
{
struct irq_info *i;
- guard(mutex)(&hash_mutex);
-
hash_for_each_possible(irq_lists, i, node, up->port.irq)
if (i->irq == up->port.irq)
return i;
i = kzalloc_obj(*i);
@@ -154,10 +152,12 @@ static struct irq_info *serial_get_or_create_irq_info(const struct uart_8250_por
static int serial_link_irq_chain(struct uart_8250_port *up)
{
struct irq_info *i;
int ret;
+ guard(mutex)(&hash_mutex);
+
i = serial_get_or_create_irq_info(up);
if (IS_ERR(i))
return PTR_ERR(i);
scoped_guard(spinlock_irq, &i->lock) {
@@ -169,10 +169,15 @@ static int serial_link_irq_chain(struct uart_8250_port *up)
INIT_LIST_HEAD(&up->list);
i->head = &up->list;
}
+ /*
+ * Keep the shared-IRQ chain locked until the first handler is installed.
+ * Otherwise another UART can join early and run startup IRQ masking while
+ * the IRQ core is still enabling the line, unbalancing the disable depth.
+ */
ret = request_irq(up->port.irq, serial8250_interrupt, up->port.irqflags, up->port.name, i);
if (ret < 0)
serial_do_unlink(i, up);
return ret;
--
2.54.0
^ permalink raw reply related
* Re: [PATCH v3 2/2] serial: qcom-geni: Add tracepoints for Qualcomm GENI serial driver
From: Greg Kroah-Hartman @ 2026-05-27 8:17 UTC (permalink / raw)
To: Praveen Talari
Cc: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers, Jiri Slaby,
Konrad Dybcio, linux-kernel, linux-trace-kernel, linux-arm-msm,
linux-serial, Mukesh Kumar Savaliya, Aniket Randive,
chandana.chiluveru, jyothi.seerapu
In-Reply-To: <c0daeac3-1c13-4389-b48d-92f3c3a1643c@oss.qualcomm.com>
On Tue, May 26, 2026 at 10:36:18AM +0530, Praveen Talari wrote:
> Hi
>
> On 22-05-2026 15:17, Greg Kroah-Hartman wrote:
> > On Mon, May 18, 2026 at 11:26:56PM +0530, 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.
> > >
> > > Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> > > Signed-off-by: Praveen Talari <praveen.talari@oss.qualcomm.com>
> > > ---
> > > v2->v3:
> > > - Updated commit text(removed example as it was available on cover
> > > letter).
> > > ---
> > > drivers/tty/serial/qcom_geni_serial.c | 27 +++++++++++++++++++++++----
> > > 1 file changed, 23 insertions(+), 4 deletions(-)
> > This patch did not apply to my tree :(
> Do you mean these patches are not applied cleanly?
Yes.
> If yes, i will push on linux-next tip.
You mean rebase, right?
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH tty] tty: n_gsm: fix use-after-free in gsm_queue vs gsm_cleanup_mux race
From: Greg KH @ 2026-05-27 8:16 UTC (permalink / raw)
To: Zhenghang Xiao; +Cc: jirislaby, linux-serial
In-Reply-To: <20260526102924.3174-1-kipreyyy@gmail.com>
On Tue, May 26, 2026 at 06:29:24PM +0800, Zhenghang Xiao wrote:
> gsm_queue() reads gsm->dlci[address] into a local pointer in the
> flush_to_ldisc workqueue without any lock. Concurrently,
> gsm_cleanup_mux() (triggered by GSMIOC_SETCONF ioctl) frees DLCIs under
> gsm->mutex — which the receive path never holds. The cached pointer in
> gsm_queue() becomes dangling, and the subsequent dlci->data() call
> dereferences freed memory.
>
> Fix this by:
> 1. Checking gsm->dead at the start of gsmld_receive_buf() to reject
> frame processing after cleanup has begun.
> 2. Moving tty_ldisc_flush() before the DLCI release loop in
> gsm_cleanup_mux(). tty_ldisc_flush() acquires the tty buffer lock
> (buf->lock), which serializes against any in-flight flush_to_ldisc
> work. After it returns, in-flight receive processing has completed,
> and subsequent calls see gsm->dead and return early.
>
> Fixes: e1eaea46bb40 ("tty: n_gsm line discipline")
> Signed-off-by: Zhenghang Xiao <kipreyyy@gmail.com>
> ---
> drivers/tty/n_gsm.c | 13 +++++++++++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
Cool, how did you test this?
>
> diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
> index c13e050de83b..8322fffbaeba 100644
> --- a/drivers/tty/n_gsm.c
> +++ b/drivers/tty/n_gsm.c
> @@ -3156,12 +3156,18 @@ static void gsm_cleanup_mux(struct gsm_mux *gsm, bool disc)
> gsm_unregister_devices(gsm_tty_driver, gsm->num);
> gsm->has_devices = false;
> }
> + /*
> + * Flush the ldisc before releasing DLCIs. tty_ldisc_flush() waits
> + * for any in-flight flush_to_ldisc work to complete via buf->lock,
> + * and the gsm->dead check added to gsmld_receive_buf() rejects any
> + * future receive processing. This ensures gsm_queue() cannot access
> + * a DLCI being freed.
> + */
> + tty_ldisc_flush(gsm->tty);
> for (i = NUM_DLCI - 1; i >= 0; i--)
> if (gsm->dlci[i])
> gsm_dlci_release(gsm->dlci[i]);
> mutex_unlock(&gsm->mutex);
> - /* Now wipe the queues */
> - tty_ldisc_flush(gsm->tty);
>
> guard(spinlock_irqsave)(&gsm->tx_lock);
> list_for_each_entry_safe(txq, ntxq, &gsm->tx_ctrl_list, list)
> @@ -3604,6 +3610,9 @@ static void gsmld_receive_buf(struct tty_struct *tty, const u8 *cp,
> struct gsm_mux *gsm = tty->disc_data;
> u8 flags = TTY_NORMAL;
>
> + if (gsm->dead)
> + return;
> +
What prevents dead from changing right after you test this?
thanks,
greg k-h
^ permalink raw reply
* RE: [PATCH RFT] tty: serial: fsl_lpuart: Add register dump
From: Sherry Sun @ 2026-05-27 2:16 UTC (permalink / raw)
To: Stefan Wahren, Greg Kroah-Hartman, Jiri Slaby
Cc: Christophe JAILLET, Ingo Molnar, Frank Li,
linux-serial@vger.kernel.org,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <2008febf-0eb6-4956-bd38-644c196bfaf0@gmx.net>
> Hi Sherry,
>
> Am 26.05.26 um 17:21 schrieb Sherry Sun:
> >> Subject: [PATCH RFT] tty: serial: fsl_lpuart: Add register dump
> >>
> >> Dumping the registers from userspace for debug purposes isn't
> >> trivial, because runtime PM can gate the required clocks and any read
> >> access to these registers would result in a lockup.
> >>
> >> So implement a register dump via debugfs, which deals with the runtime
> PM.
> > No need, you can disable runtime pm from user space before dump the
> registers.
> > For example: echo on >
> > /sys/bus/platform/devices/29860000.serial/power/control
> so "comfort" wouldn't be an argument for this change?
> I'm asking, because this isn't the first driver with a regdump via debugfs.
I only see this method added in the pch_uart.c driver, and that was added 14 years ago and is obsolete.
Sorry, I don't think it's elegant to use this method just for "comfort" :(
Best Regards
Sherry
>
> Best regards
> >
> > Best Regards
> > Sherry
> >> Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
> >> ---
> >>
> >> Hi, I only have access to a board (i.MX93) with 32 bit registers, so
> >> it would be great if someone could test it for 8 bit registers.
> >>
> >> Thanks
> >>
> >> drivers/tty/serial/fsl_lpuart.c | 69
> +++++++++++++++++++++++++++++++++
> >> 1 file changed, 69 insertions(+)
> >>
> >> diff --git a/drivers/tty/serial/fsl_lpuart.c
> >> b/drivers/tty/serial/fsl_lpuart.c index 1bd7ec9c81ea..7e4b3e59414a
> >> 100644
> >> --- a/drivers/tty/serial/fsl_lpuart.c
> >> +++ b/drivers/tty/serial/fsl_lpuart.c
> >> @@ -10,6 +10,7 @@
> >> #include <linux/circ_buf.h>
> >> #include <linux/clk.h>
> >> #include <linux/console.h>
> >> +#include <linux/debugfs.h>
> >> #include <linux/delay.h>
> >> #include <linux/dma-mapping.h>
> >> #include <linux/dmaengine.h>
> >> @@ -23,6 +24,7 @@
> >> #include <linux/pinctrl/consumer.h>
> >> #include <linux/platform_device.h>
> >> #include <linux/pm_runtime.h>
> >> +#include <linux/seq_file.h>
> >> #include <linux/serial_core.h>
> >> #include <linux/slab.h>
> >> #include <linux/tty_flip.h>
> >> @@ -291,6 +293,8 @@ struct lpuart_port {
> >> bool is_cs7; /* Set to true when character size is 7
> >> */
> >> /* and the parity is enabled
> >> */
> >> bool dma_idle_int;
> >> +
> >> + struct dentry *debugfs_dir;
> >> };
> >>
> >> struct lpuart_soc_data {
> >> @@ -1031,6 +1035,7 @@ static void lpuart32_rxint(struct lpuart_port
> *sport)
> >> }
> >>
> >> out:
> >> +
> >> uart_unlock_and_check_sysrq(&sport->port);
> >>
> >> tty_flip_buffer_push(port);
> >> @@ -2860,6 +2865,67 @@ static int lpuart_global_reset(struct
> >> lpuart_port
> >> *sport)
> >> return 0;
> >> }
> >>
> >> +#ifdef CONFIG_DEBUG_FS
> >> +
> >> +#define dump_register_hex(_seq, _reg, _sport) \ seq_printf((_seq),
> >> +"%-12s: 0x%02x\n", #_reg, readb((_sport)->port.membase + (_reg)))
> >> +
> >> +#define dump_register32_hex(_seq, _reg, _sport) \ seq_printf((_seq),
> >> +"%-12s: 0x%08x\n", #_reg, lpuart32_read(&(_sport)->port, _reg))
> >> +
> >> +static int regs_show(struct seq_file *s, void *p) {
> >> + struct lpuart_port *sport = s->private;
> >> +
> >> + pm_runtime_get_sync(sport->port.dev);
> >> +
> >> + if (lpuart_is_32(sport)) {
> >> + dump_register32_hex(s, UARTBAUD, sport);
> >> + dump_register32_hex(s, UARTSTAT, sport);
> >> + dump_register32_hex(s, UARTCTRL, sport);
> >> + dump_register32_hex(s, UARTMATCH, sport);
> >> + dump_register32_hex(s, UARTMODIR, sport);
> >> + dump_register32_hex(s, UARTFIFO, sport);
> >> + dump_register32_hex(s, UARTWATER, sport);
> >> + } else {
> >> + dump_register_hex(s, UARTBDH, sport);
> >> + dump_register_hex(s, UARTBDL, sport);
> >> + dump_register_hex(s, UARTCR1, sport);
> >> + dump_register_hex(s, UARTCR2, sport);
> >> + dump_register_hex(s, UARTSR1, sport);
> >> + dump_register_hex(s, UARTCR3, sport);
> >> + dump_register_hex(s, UARTDR, sport);
> >> + dump_register_hex(s, UARTCR4, sport);
> >> + dump_register_hex(s, UARTCR5, sport);
> >> + dump_register_hex(s, UARTMODEM, sport);
> >> + dump_register_hex(s, UARTPFIFO, sport);
> >> + dump_register_hex(s, UARTCFIFO, sport);
> >> + dump_register_hex(s, UARTSFIFO, sport);
> >> + dump_register_hex(s, UARTTWFIFO, sport);
> >> + dump_register_hex(s, UARTTCFIFO, sport);
> >> + dump_register_hex(s, UARTRWFIFO, sport);
> >> + }
> >> +
> >> + pm_runtime_mark_last_busy(sport->port.dev);
> >> + pm_runtime_put_autosuspend(sport->port.dev);
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +DEFINE_SHOW_ATTRIBUTE(regs);
> >> +
> >> +static void lpuart_init_debugfs(struct lpuart_port *sport) {
> >> + sport->debugfs_dir = debugfs_create_dir(dev_name(sport-
> >>> port.dev),
> >> + NULL);
> >> +
> >> + debugfs_create_file("regs", 0400, sport->debugfs_dir, sport,
> >> +®s_fops); }
> >> +
> >> +#else
> >> +static inline void lpuart_init_debugfs(struct lpuart_port *sport) {}
> >> +#endif
> >> +
> >> static int lpuart_probe(struct platform_device *pdev) {
> >> const struct lpuart_soc_data *sdata =
> >> of_device_get_match_data(&pdev->dev);
> >> @@ -2969,6 +3035,8 @@ static int lpuart_probe(struct platform_device
> >> *pdev)
> >> if (ret)
> >> goto failed_irq_request;
> >>
> >> + lpuart_init_debugfs(sport);
> >> +
> >> return 0;
> >>
> >> failed_irq_request:
> >> @@ -2987,6 +3055,7 @@ static void lpuart_remove(struct
> >> platform_device
> >> *pdev) {
> >> struct lpuart_port *sport = platform_get_drvdata(pdev);
> >>
> >> + debugfs_remove_recursive(sport->debugfs_dir);
> >> uart_remove_one_port(&lpuart_reg, &sport->port);
> >>
> >> lpuart_disable_clks(sport);
> >> --
> >> 2.43.0
^ permalink raw reply
* Re: [PATCH RFT] tty: serial: fsl_lpuart: Add register dump
From: Stefan Wahren @ 2026-05-26 20:50 UTC (permalink / raw)
To: Sherry Sun, Greg Kroah-Hartman, Jiri Slaby
Cc: Christophe JAILLET, Ingo Molnar, Frank Li,
linux-serial@vger.kernel.org,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <VI0PR04MB1211492AAE8858FD14187E922920B2@VI0PR04MB12114.eurprd04.prod.outlook.com>
Hi Sherry,
Am 26.05.26 um 17:21 schrieb Sherry Sun:
>> Subject: [PATCH RFT] tty: serial: fsl_lpuart: Add register dump
>>
>> Dumping the registers from userspace for debug purposes isn't trivial,
>> because runtime PM can gate the required clocks and any read access to
>> these registers would result in a lockup.
>>
>> So implement a register dump via debugfs, which deals with the runtime PM.
> No need, you can disable runtime pm from user space before dump the registers.
> For example: echo on > /sys/bus/platform/devices/29860000.serial/power/control
so "comfort" wouldn't be an argument for this change?
I'm asking, because this isn't the first driver with a regdump via debugfs.
Best regards
>
> Best Regards
> Sherry
>> Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
>> ---
>>
>> Hi, I only have access to a board (i.MX93) with 32 bit registers, so it would be
>> great if someone could test it for 8 bit registers.
>>
>> Thanks
>>
>> drivers/tty/serial/fsl_lpuart.c | 69 +++++++++++++++++++++++++++++++++
>> 1 file changed, 69 insertions(+)
>>
>> diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c index
>> 1bd7ec9c81ea..7e4b3e59414a 100644
>> --- a/drivers/tty/serial/fsl_lpuart.c
>> +++ b/drivers/tty/serial/fsl_lpuart.c
>> @@ -10,6 +10,7 @@
>> #include <linux/circ_buf.h>
>> #include <linux/clk.h>
>> #include <linux/console.h>
>> +#include <linux/debugfs.h>
>> #include <linux/delay.h>
>> #include <linux/dma-mapping.h>
>> #include <linux/dmaengine.h>
>> @@ -23,6 +24,7 @@
>> #include <linux/pinctrl/consumer.h>
>> #include <linux/platform_device.h>
>> #include <linux/pm_runtime.h>
>> +#include <linux/seq_file.h>
>> #include <linux/serial_core.h>
>> #include <linux/slab.h>
>> #include <linux/tty_flip.h>
>> @@ -291,6 +293,8 @@ struct lpuart_port {
>> bool is_cs7; /* Set to true when character size is 7
>> */
>> /* and the parity is enabled
>> */
>> bool dma_idle_int;
>> +
>> + struct dentry *debugfs_dir;
>> };
>>
>> struct lpuart_soc_data {
>> @@ -1031,6 +1035,7 @@ static void lpuart32_rxint(struct lpuart_port *sport)
>> }
>>
>> out:
>> +
>> uart_unlock_and_check_sysrq(&sport->port);
>>
>> tty_flip_buffer_push(port);
>> @@ -2860,6 +2865,67 @@ static int lpuart_global_reset(struct lpuart_port
>> *sport)
>> return 0;
>> }
>>
>> +#ifdef CONFIG_DEBUG_FS
>> +
>> +#define dump_register_hex(_seq, _reg, _sport) \ seq_printf((_seq),
>> +"%-12s: 0x%02x\n", #_reg, readb((_sport)->port.membase + (_reg)))
>> +
>> +#define dump_register32_hex(_seq, _reg, _sport) \ seq_printf((_seq),
>> +"%-12s: 0x%08x\n", #_reg, lpuart32_read(&(_sport)->port, _reg))
>> +
>> +static int regs_show(struct seq_file *s, void *p) {
>> + struct lpuart_port *sport = s->private;
>> +
>> + pm_runtime_get_sync(sport->port.dev);
>> +
>> + if (lpuart_is_32(sport)) {
>> + dump_register32_hex(s, UARTBAUD, sport);
>> + dump_register32_hex(s, UARTSTAT, sport);
>> + dump_register32_hex(s, UARTCTRL, sport);
>> + dump_register32_hex(s, UARTMATCH, sport);
>> + dump_register32_hex(s, UARTMODIR, sport);
>> + dump_register32_hex(s, UARTFIFO, sport);
>> + dump_register32_hex(s, UARTWATER, sport);
>> + } else {
>> + dump_register_hex(s, UARTBDH, sport);
>> + dump_register_hex(s, UARTBDL, sport);
>> + dump_register_hex(s, UARTCR1, sport);
>> + dump_register_hex(s, UARTCR2, sport);
>> + dump_register_hex(s, UARTSR1, sport);
>> + dump_register_hex(s, UARTCR3, sport);
>> + dump_register_hex(s, UARTDR, sport);
>> + dump_register_hex(s, UARTCR4, sport);
>> + dump_register_hex(s, UARTCR5, sport);
>> + dump_register_hex(s, UARTMODEM, sport);
>> + dump_register_hex(s, UARTPFIFO, sport);
>> + dump_register_hex(s, UARTCFIFO, sport);
>> + dump_register_hex(s, UARTSFIFO, sport);
>> + dump_register_hex(s, UARTTWFIFO, sport);
>> + dump_register_hex(s, UARTTCFIFO, sport);
>> + dump_register_hex(s, UARTRWFIFO, sport);
>> + }
>> +
>> + pm_runtime_mark_last_busy(sport->port.dev);
>> + pm_runtime_put_autosuspend(sport->port.dev);
>> +
>> + return 0;
>> +}
>> +
>> +DEFINE_SHOW_ATTRIBUTE(regs);
>> +
>> +static void lpuart_init_debugfs(struct lpuart_port *sport) {
>> + sport->debugfs_dir = debugfs_create_dir(dev_name(sport-
>>> port.dev),
>> + NULL);
>> +
>> + debugfs_create_file("regs", 0400, sport->debugfs_dir, sport,
>> +®s_fops); }
>> +
>> +#else
>> +static inline void lpuart_init_debugfs(struct lpuart_port *sport) {}
>> +#endif
>> +
>> static int lpuart_probe(struct platform_device *pdev) {
>> const struct lpuart_soc_data *sdata =
>> of_device_get_match_data(&pdev->dev);
>> @@ -2969,6 +3035,8 @@ static int lpuart_probe(struct platform_device
>> *pdev)
>> if (ret)
>> goto failed_irq_request;
>>
>> + lpuart_init_debugfs(sport);
>> +
>> return 0;
>>
>> failed_irq_request:
>> @@ -2987,6 +3055,7 @@ static void lpuart_remove(struct platform_device
>> *pdev) {
>> struct lpuart_port *sport = platform_get_drvdata(pdev);
>>
>> + debugfs_remove_recursive(sport->debugfs_dir);
>> uart_remove_one_port(&lpuart_reg, &sport->port);
>>
>> lpuart_disable_clks(sport);
>> --
>> 2.43.0
^ permalink raw reply
* [PATCH v4 1/2] serial: qcom-geni: trace: Add tracepoint support for Qualcomm GENI serial
From: Praveen Talari @ 2026-05-26 17:37 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Greg Kroah-Hartman, Jiri Slaby, konrad.dybcio
Cc: Praveen Talari, linux-kernel, linux-trace-kernel, linux-arm-msm,
linux-serial, mukesh.savaliya, aniket.randive, chandana.chiluveru
In-Reply-To: <20260526-add-tracepoints-for-qcom-geni-serial-v4-0-e94fbaec0232@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
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox